Skip to content

Conversation

@zygoloid
Copy link
Contributor

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 inline code.

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.

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.
@zygoloid zygoloid requested a review from a team as a code owner December 11, 2025 01:13
@zygoloid zygoloid requested review from jonmeow and removed request for a team December 11, 2025 01:13
Comment on lines 375 to 377
// 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.
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@zygoloid zygoloid enabled auto-merge December 12, 2025 21:55
Comment on lines 93 to 94
"Already have a C++ context");
cpp_context_ = std::move(cpp_context);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[diff] reported by reviewdog 🐶

Suggested change
"Already have a C++ context");
cpp_context_ = std::move(cpp_context);
CARBON_CHECK(!cpp_context_ || !cpp_context, "Already have a C++ context");

@zygoloid zygoloid added this pull request to the merge queue Dec 13, 2025
Merged via the queue into carbon-language:trunk with commit a8eca2e Dec 13, 2025
8 checks passed
@zygoloid zygoloid deleted the toolchain-interop-delay-end-of-tu branch December 13, 2025 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants