Skip to content

[C++20][Modules][NFC] Some minor non-functional fixes #172524

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

Merged
mpark merged 3 commits into from
Dec 16, 2025
+4 −4
Merged

[C++20][Modules][NFC] Some minor non-functional fixes #172524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c30337d
[C++20][Modules][NFC] Fix typo in ASTWriter.cpp.
mpark Dec 16, 2025
28555c7
[C++20][Modules][NFC] Fix incorrect comment in modules reachability t…
mpark Dec 16, 2025
56089c7
[C++20][Modules][NFC] Pull the chain access out of the loop.
mpark Dec 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions clang/lib/Serialization/ASTWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4441,9 +4441,9 @@ class ASTDeclContextNameLookupTrait

DeclIDs.push_back(ID);
};
ASTReader *Chain = Writer.getChain();
Copy link
Member

@ChuanqiXu9 ChuanqiXu9 Dec 17, 2025

Choose a reason for hiding this comment

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

Not a blocker. But I remember we prefer to move the declaration to if-statement if possible.

for (NamedDecl *D : Decls) {
if (ASTReader *Chain = Writer.getChain();
Chain && isa<NamespaceDecl>(D) && D->isFromASTFile() &&
if (Chain && isa<NamespaceDecl>(D) && D->isFromASTFile() &&
Comment on lines +4444 to +4446
Copy link
Collaborator

@hubert-reinterpretcast hubert-reinterpretcast Dec 16, 2025

Choose a reason for hiding this comment

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

This part is NFC, but not a "minor comment fix".

Copy link
Member Author

@mpark mpark Dec 16, 2025

Choose a reason for hiding this comment

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

Yep. I added this commit after creating the PR and hadn't updated the title. yet. Updated to "non-functional fixes" now.

D == Chain->getKeyDeclaration(D)) {
// In ASTReader, we stored only the key declaration of a namespace decl
// for this TU rather than storing all of the key declarations from each
Expand Down Expand Up @@ -4641,7 +4641,7 @@ uint64_t ASTWriter::WriteSpecializationInfoLookupTable(
return Offset;
}

/// Returns ture if all of the lookup result are either external, not emitted or
/// Returns true if all of the lookup result are either external, not emitted or
/// predefined. In such cases, the lookup result is not interesting and we don't
/// need to record the result in the current being written module. Return false
/// otherwise.
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CXX/module/module.reach/p5.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ export using Y = X;
//--- B.cppm
export module B;
import A;
Y y; // OK, definition of X is reachable
Y y; // OK, definition of Y is reachable
X x; // expected-error {{unknown type name 'X'}}
Loading