likeamahoney wrote:

> This needs more thought. The solution implemented here takes a quite unusual 
> approach, rebuilding a non-dependent type as a dependent type. This seems 
> fragile, because non-dependent types are in general allowed to lose 
> information which would not be otherwise lost as a dependent type.

Hi!

I guess you're right and the approach doesn't hold up. A `DependentNameType` 
qualifier is part of its canonical identity, whereas on a resolved type the 
keyword and qualifier are sugar that Clang is free to drop. So rebuilding one 
from the other reconstructs canonically-significant data out of information 
nothing promises to keep, which is exactly why the patch has to bail out 
silently whenever the identifier or the qualifier isn't recoverable. 

Digging further, I don't think the parser is at fault either: it is fine for 
`computeDeclContext` in entering-context mode to resolve `B` in `A<T>::B::C` 
against the pattern. Bug is that this member is never mapped to the 
instantiation. `FindInstantiatedDecl` can only map a pattern member by walking 
`CurContext`, so it succeeds when substitution happens lexically inside 
`A<int>` and silently returns the pattern declaration otherwise, leaving the 
type dependent forever. So I'd rather fix it in `TreeTransform`, when a 
member's qualifier has been substituted from dependent to non-dependent but 
`TransformDecl` handed back the pattern declaration unchanged, perform the 
mapping with the parent we now have in hand - look the name up in the 
substituted qualifier's `DeclContext` and take the declaration that 
`isInstantiationOf` the pattern one. If there is none, as for an explicit 
specialization, fall back to `RebuildDependentNameType`, i.e. ordinary lookup, 
exactly what explicit typename does. Trying the instantiation mapping first is 
what makes this safe: a plain lookup would break qualifiers in declarator 
position where the pattern member is hidden by an unrelated member of the same 
name (`ConfuseLookup` in `SemaCXX/MicrosoftCompatibility.cpp`).

https://github.com/llvm/llvm-project/pull/205762
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to