ChuanqiXu added inline comments.
================
Comment at: clang/lib/Sema/SemaModule.cpp:530-531
+ CurContext->addDecl(D);
+ PushDeclContext(S, D);
+
----------------
aaron.ballman wrote:
> Am I understanding properly that this moved up here so that the for loop on
> line 553 can traverse the new context?
>
> If so, can it be moved down to immediately before the for loop?
The intention to move these up is to make sure D could be put in the context
even error detected. It wouldn't affect the traverse on line 556 since an
ExportDecl wouldn't be a NamespaceDecl.
================
Comment at: clang/lib/Sema/SemaModule.cpp:539-550
+ return D;
} else if (!ModuleScopes.back().ModuleInterface) {
Diag(ExportLoc, diag::err_export_not_in_module_interface) << 1;
Diag(ModuleScopes.back().BeginLoc,
diag::note_not_module_interface_add_export)
<< FixItHint::CreateInsertion(ModuleScopes.back().BeginLoc, "export ");
+ return D;
----------------
aaron.ballman wrote:
> It seems a bit suspicious to me that we call `D->getInvalidDecl()` below
> within the for loop, but all the other places we leave it as a valid
> declaration despite it causing error diagnostics.
Yeah, it is intentional to call `D->setInvalidDecl()` this loop. It would
suppress more diagnostic messages. But I feel it is good to call
`D->setInvalidDecl()` in other places.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117093/new/
https://reviews.llvm.org/D117093
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
