-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Delay finishing the C++ translation unit until we reach the real EOF. #6489
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
Delay finishing the C++ translation unit until we reach the real EOF. #6489
Conversation
This fixes some issues where we would produce duplicated or incorrect diagnostics at the end of the C++ translation unit, particularly for unused declarations. Don't allow a C++ module to be defined in an inline Cpp fragment.
Reuse the same parser for macro parsing.
toolchain/check/cpp/generate_ast.cpp
Outdated
| // Parse the imports and inline C++ fragments. This is notionally very | ||
| // similar to `ParseAST`, but doesn't parse C++20 modules and stops just | ||
| // before reaching the end of the translation unit. |
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.
Perhaps move this to be a function comment? I'm not sure if it's just understood what ExecuteAction is expected to do.
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.
Done.
Co-authored-by: Jon Ross-Perkins <[email protected]>
toolchain/check/context.h
Outdated
| "Already have a C++ context"); | ||
| cpp_context_ = std::move(cpp_context); |
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.
[diff] reported by reviewdog 🐶
| "Already have a C++ context"); | |
| cpp_context_ = std::move(cpp_context); | |
| CARBON_CHECK(!cpp_context_ || !cpp_context, "Already have a C++ context"); |
Instead of parsing a complete C++ translation unit and then interacting with the translation unit further after the fact, delay finishing the translation unit until we finish the Carbon check phase. This fixes some issues where we would produce duplicated or incorrect diagnostics at the end of the C++ translation unit, particularly for unused declarations. Now we're in control of how we parse the translation unit, also disable parsing of C++20 modules if the syntax appears within
import Cpp inlinecode.Keep the same clang parser alive throughout check, and use it instead of building a new one when parsing macros. This resolves issues where the translation unit scope was destroyed too early, resulting in unqualified lookup within macros being unable to find global scope entities.