ChuanqiXu added inline comments.
================ Comment at: clang/lib/Sema/SemaTemplate.cpp:8679 + Previous.isSingleResult() ? Previous.getAsSingle<ConceptDecl>() : nullptr; + if (PrevDef && !hasVisibleDefinition(PrevDef) && + Context.isSameEntity(NewDecl, PrevDef)) { ---------------- rsmith wrote: > ChuanqiXu wrote: > > > I don't think that's right: [the C++20 modules > rule](http://eel.is/c++draft/basic.def.odr#2) is that at most one definition > is permitted per translation unit, and the Clang header modules rule is that > a definition is only permissible if no definition is visible (Clang header > modules effectively behave as if they're part of the same translation unit > for the purposes of this check). For now, checking for a visible definition > seems better than checking for a reachable one, until we implement the C++20 > rule in general. For clang modules, the reachability should be the visibility: https://github.com/llvm/llvm-project/blob/9cb00e7133ea3da6a49ade95e3708240c2aaae39/clang/lib/Sema/SemaLookup.cpp#L1903-L1905. So I think it is better to check for a reachable definition here. Otherwise we might not be able to handle the following case: ``` import M; export template <class T> concept ConflictingConcept = true; // There is another reachable but not visible definition for ConflictingConcept. ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128921/new/ https://reviews.llvm.org/D128921 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits