ChuanqiXu added inline comments.
================ Comment at: clang/include/clang/Sema/Sema.h:2955-2960 + FirstDecl, + GlobalFragment, + ImportAllowed, + ImportFinished, + PrivateFragment, + NotACXX20Module ---------------- I guess it is worth to add comment for these fields. Also I could guess the meaning, I think it is better to comment the semantics accurately. ================ Comment at: clang/lib/Parse/ParseObjc.cpp:82 if (getLangOpts().Modules || getLangOpts().DebuggerSupport) { - SingleDecl = ParseModuleImport(AtLoc); + Sema::ModuleImportState IS = Sema::ModuleImportState::NotACXX20Module; + SingleDecl = ParseModuleImport(AtLoc, IS); ---------------- We could remove the IS variable ================ Comment at: clang/lib/Parse/Parser.cpp:913 + } + SingleDecl = ParseModuleImport(SourceLocation(), IS); + } break; ---------------- I think it is better to: ``` SingleDecl = ParseModuleImport(SourceLocation(), ModuleImportState::NotACXX20Module); ``` ================ Comment at: clang/lib/Parse/Parser.cpp:2466 + case Sema::ModuleImportState::NotACXX20Module: + // These cases will be an error when partitions are implemented. + SeenError = false; ---------------- Generally, we would add a `TODO` or `FIXME` for such comments. (This is not need to be addressed if the following patch would be landed quickly) ================ Comment at: clang/lib/Sema/SemaModule.cpp:144 GlobalModuleFragment = ModuleScopes.back().Module; // In C++20, the module-declaration must be the first declaration if there ---------------- I feel better to add an assertion here: ``` assert(SeenGMF == GlobalModuleFragment && "msg); ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118893/new/ https://reviews.llvm.org/D118893 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits