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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
