Hahnfeld added a comment.

In D153003#4423926 <https://reviews.llvm.org/D153003#4423926>, @ChuanqiXu wrote:

> This looks not so good. In this way, we can't disambiguate other template 
> types. At least we added the kind and a TODO here.

Which template name types would we need to disambiguate? We could also 
normalize the `Kind`, for example from `QualifiedTemplate` to `Template` (which 
is the case I care about).

> BTW, the attached test case is in fact in correct. See 
> https://eel.is/c++draft/basic.def.odr#14.3, such mergeable declarations 
> shouldn't be attached to named modules. (And this is also a defect that now 
> we don't diagnose it correctly).

I'm not sure I understand, could you elaborate? "correct" as in "the merge 
error is right" or "should work as written". Also there are a number of `export 
struct` in the tests, but you are saying this should not be included in the 
modules? How would this work?

> Even if you put them into the global module fragment, I think we should try 
> to look other places like `isSameEntity` to solve this.

Sure, but this first requires the hash to be identical, no? My understanding is 
that two (structurally) identical Decls must always hash to the same value, but 
the same value does not imply that `isSameEntity` and friends eventually agree 
(due to hash collisions).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153003/new/

https://reviews.llvm.org/D153003

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to