Skip to content

Fix Interpreter::getMacro returning 0 when using modules. #558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
vgvassilev merged 1 commit into from
May 11, 2017
Merged

Fix Interpreter::getMacro returning 0 when using modules. #558

vgvassilev merged 1 commit into from
May 11, 2017

Conversation

@Teemperor
Copy link
Contributor

@Teemperor Teemperor commented May 9, 2017

The old code didn't query the modules if the identifier is updated. This caused some checks to fail such as CheckABICompatibility().

@phsft-bot
Copy link

phsft-bot commented May 9, 2017

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with CMake flags -Dvc=OFF -Dimt=OFF -Dccache=ON

@Teemperor
Copy link
Contributor Author

Teemperor commented May 9, 2017

Note, this handles now the modules an the non-modules case correctly. So no if(getenv("ROOT_MODULES")) ... here.

@Teemperor Teemperor force-pushed the runtime-cxxmodules branch from 6ff673a to 2f398d8 Compare May 9, 2017 09:01
@phsft-bot
Copy link

phsft-bot commented May 9, 2017

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with CMake flags -Dvc=OFF -Dimt=OFF -Dccache=ON

@vgvassilev
Copy link
Member

vgvassilev commented May 11, 2017

Shall I merge this? If yes, could you add a FIXME saying that this can go away when we update clang (presumably after our patch lands there).

@Teemperor
Fix Interpreter::getMacro returning 0 when using modules.
8f7a223 
The old code didn't query the modules if the identifier
is updated. This caused some checks to fail such as
CheckABICompatibility().
@Teemperor Teemperor force-pushed the runtime-cxxmodules branch from 2f398d8 to 8f7a223 Compare May 11, 2017 13:57
@phsft-bot
Copy link

phsft-bot commented May 11, 2017

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with CMake flags -Dvc=OFF -Dimt=ON -Dccache=ON

@Teemperor
Copy link
Contributor Author

Teemperor commented May 11, 2017

Sure, let's merge this. It won't bite us when we update clang and I added a FIXME for now.

@vgvassilev vgvassilev merged commit d5678be into root-project:runtime-cxxmodules May 11, 2017
@phsft-bot phsft-bot mentioned this pull request Oct 23, 2022
Closed
guitargeek pushed a commit to guitargeek/root that referenced this pull request Apr 28, 2025
@pcanal
Follow improvement in TKey's ls (root-project#558)
092c7e3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Teemperor @phsft-bot @vgvassilev