-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Generate AST when importing a cpp file #4790
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
Conversation
Ignore the AST, for now. Report cpp compilation errors and warnings. Part of carbon-language#4666
danakj
left a comment
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.
a couple suggestions and questions
toolchain/check/testdata/interop/cpp/no_prelude/file_not_found.carbon
Outdated
Show resolved
Hide resolved
jonmeow
left a comment
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.
This is great!
I'm looking at this, and thought of a lateral issue... We're fuzz testing, and as soon as the fuzzer discovers it can write C++ imports we'll end up fuzz testing Clang, and Clang is not crash-resilient. The driver already has a DriverEnv::fuzzing flag, can you have it lead to an error when set and importing from Cpp?
1. Change diagnostic ids. 2. Split file read error into a note. 3. Added bad import Cpp tests. 4. Remove `is_cpp` and move the condition higher.
…o import Cpp files.
I've added an error for that. |
toolchain/check/testdata/interop/cpp/no_prelude/bad_import.carbon
Outdated
Show resolved
Hide resolved
toolchain/check/testdata/interop/cpp/no_prelude/bad_import.carbon
Outdated
Show resolved
Hide resolved
toolchain/check/testdata/interop/cpp/no_prelude/bad_import.carbon
Outdated
Show resolved
Hide resolved
jonmeow
left a comment
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.
LGTM
Ignore the AST and support a single Cpp import, for now.
Report cpp compilation errors and warnings.
Part of #4666