ilya-biryukov added a comment.
Thanks for the thorough explanation and testing.
LGTM.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:1738
+// Return true if the redefinition is not allowed. Return false otherwise.
+bool Sema::CheckRedefinitionInModule(const NamedDecl *New,
+ const NamedDecl *Old) const {
----------------
NIT: maybe rename to `IsRedefinitionInModule`?
I would expect `Check...` to produce a diagnostic itself. Having `Is...` in the
name also removes any confusion about whether the `true` as a return value
indicates an error or not.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:1747
+
+ Module *NewM = New->getOwningModule();
+ Module *OldM = Old->getOwningModule();
----------------
NIT: maybe accept the modules directly?
================
Comment at: clang/test/Modules/merge-concepts.cppm:22
+// Testing with module map modules. It is unclear about the relation ship
between Clang modules and
+// C++20 Named Modules. Will them co_exist? Or will them be mutual from each
other?
+// The test here is for primarily coverity.
----------------
NIT on wording
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130614/new/
https://reviews.llvm.org/D130614
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
