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.
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
fix(cxx_extractor): don't crash when modules are on #5554
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a test for this.
It appears that the OSS Bazel rules don't support header-only modules (the feature this PR is intended to stop exploding). An internal test would probably work; otherwise I think we're looking at a sh test with the cxx_extractor binary (ugh) or a hand-edited xa (ugh). I would be happy to be proven incorrect. |
None of the existing extractor integration tests use the Bazel rules or shell, see, for example: https://github.com/kythe/kythe/blob/master/kythe/cxx/extractor/testdata/root_directory_test.cc Is there some reason that approach won't work here? |
Aha, I had been trying to tie everything together with Bazel ( |
In service of kythe#5543. This PR should only affect the behavior of the extractor where it would have either crashed because of a null dereference or due to a CHECK-fail. The conditions leading to these cases occur when modules are turned on.
OK, the new test hits the new code path. I expect to refine the set of flags as I dig deeper into this. (also ugh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
In service of #5543.
This PR should only affect the behavior of the extractor where it would have either crashed because of a null dereference or due to a CHECK-fail. The conditions leading to these cases occur when modules are turned on.