rsmith added a comment.

In D154324#4524235 <https://reviews.llvm.org/D154324#4524235>, @v.g.vassilev 
wrote:

> @rsmith, thanks for the suggestions! Could you go over 
> `ODRHash::AddTemplateName` suggest how to fix it to address 
> https://reviews.llvm.org/D153003 and https://reviews.llvm.org/D41416#4496451?

`AddTemplateName` looks fine as-is to me; I think the problem in D153003 
<https://reviews.llvm.org/D153003> is that we'd stepped outside of the entity 
we were odr-hashing and started hashing something else, which (legitimately) 
was different between translation units.

For D41416 <https://reviews.llvm.org/D41416>, ODR hashing may not be the best 
mechanism to hash the template arguments, unfortunately. ODR hashing is (or 
perhaps, should be) about determining whether two things are spelled the same 
way and have the same meaning (as required by the C++ ODR), whereas I think 
what you're looking for is whether they have the same meaning regardless of 
spelling. Maybe we can get away with reusing ODR hashing anyway, on the basis 
that any canonical, non-dependent template argument should have the same 
(invented) spelling in every translation unit, but I'm not certain that's true 
in all cases. There may still be cases where the canonical type includes some 
aspect of "whatever we saw first", in which case the ODR hash can differ across 
translation units for non-dependent, canonical template arguments that are 
spelled differently but have the same meaning, though I can't think of one 
off-hand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154324

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

Reply via email to