<div dir="ltr"><div dir="ltr">On Mon, 20 Jul 2020 at 12:11, David Rector via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;"><div style="margin:0px;font-stretch:normal;line-height:normal">Good point.  At best implementing the second NNS as a dependent prefix + Identifier is redundant, since DependentNameType could already store this info, but you’re right that it would be more consistent with the first NNS you cite (the DependentTemplateSpecializationType) to implement the second NNS as a DependentNameType.</div><div style="margin:0px;font-stretch:normal;line-height:normal;min-height:14px"><br></div><div style="margin:0px;font-stretch:normal;line-height:normal">It seems like the Prefix field of NestedNameSpecifier is the root cause of this redundancy, and perhaps more general confusion.  I don’t think it is needed.  Perhaps an NNS should only store a simple PointerUnion of the various name kinds.  If we really want to keep the getPrefix() method we can define it as accessing the the properly cast pointer’s getQualifier() method.</div></div></blockquote><div><br></div><div>I think this is backwards from the intent of the design: the intent is that a NestedNameSpecifier is a singly-linked list of prefixes, each of which adds a single (unqualified) component. Following that design intent, using DependentTemplateSpecializationType here is strange, since it stores redundant qualifier information; all we really want to store for a dependent simple-template-id in a nested-name-specifier is an identifier plus a set of template arguments (and a flag to indicate if 'template' was specified), not even a type. In fact, the use of a type here causes minor problems (and slightly breaks the model) as described in this comment: <a href="https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/NestedNameSpecifier.cpp#L306">https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/NestedNameSpecifier.cpp#L306</a></div><div><br></div><div>However, perhaps the current design intent is not the best approach; it might improve the design if we stored the prefix as part of the representation whenever possible, instead of excluding it from the representation whenever possible. That would mean (as you note below) that the representation of namespaces would be different from that of any other NestedNameSpecifier. We would also need to consider how such a change would interact with the NestedNameSpecifierLoc representation -- presumably we'd end up with nested-name-specifier location data embedded in type location data embedded in nested-name-specifier location data recursively. That might be OK; I suspect that means we'd end up coupling NestedNameSpecifierLocBuilder and TypeLocBuilder, which is a little awkward because they're currently entirely independent and even live in different clang libraries right now (but there doesn't seem to be any good reason why TypeLocBuilder is in Sema rather than in AST). Another problem with the alternative design is that it would need a representation for dependent member accesses (eg, `x.A::b` where x is type-dependent): it's not meaningful (outside of nested-name-specifiers) to have a DependentNameType with no qualifier, but that's exactly what we'd need here. Note that we do permit a DependentTemplateSpecializationType to have no qualifier in order to handle the analogous case of `x.template A<T>::b`, and that special case has caused crashes in recent memory because people wrote code making the (reasonable) assumption that a DependentTemplateSpecializationType must always have a qualifier. Relatedly, the Dependent* types in NestedNameSpecifiers would violate uniformity because substitution into them would not proceed in the same way as regular substitution into a type, in part because of the different behavior of qualifiers in class member access (and also because there are different lookup rules for a name on the left-hand side of a ::).</div><div><br></div><div>I think either approach is workable. The current design intent has the advantage that it more directly follows the standard and its grammar rules, and so is better prepared for the possibility that nested-name-specifiers might one day diverge from the syntax of type-specifiers, and it uses a different representation for cases with different behaviors. The alternative design has the advantage that the type representation of a type nested-name-specifier models how that type would be written in the same context (including its qualifier).</div><div><br></div><div>On balance, if we fix the non-uniformity here, I'd suggest that we stop using DependentTemplateSpecializationType as a possible type representation for a NestedNameSpecifier and instead store only a "template" keyword flag, an identifier, and a list of template arguments.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;"><div style="margin:0px;font-stretch:normal;line-height:normal">DependentNameTypes/DependentTemplateSpecializationTypes each store their prefix on their own, so do ElaboratedTypes, so written prefixes for types would be covered, if I’m not mistaken.</div><div style="margin:0px;font-stretch:normal;line-height:normal;min-height:14px"><br></div><div style="margin:0px;font-stretch:normal;line-height:normal">The only non-trivial change I think needed would be for handling namespaces, for which we need to pair a written prefix with the NamespaceDecl or NamespaceAliasDecl.  In those cases, I think, we just define a "NamespaceName" class to handle the pairing of the decl and written qualifier, then allocate the NamespaceName as a trailing object of its NestedNameSpecifier in the applicable Create functions.</div><div style="margin:0px;font-stretch:normal;line-height:normal;min-height:14px"><br></div><div style="margin:0px;font-stretch:normal;line-height:normal">By getting rid of the Prefix field in this way we would be forced to define the second example you cite via a DependentNameType, making it more consistent with other the first example you cite, and would also save a little space in most cases.</div><div style="margin:0px;font-stretch:normal;line-height:normal;min-height:14px"><br></div><div style="margin:0px;font-stretch:normal;line-height:normal">I am probably missing something though.  In any case you raise a good point,</div><div style="margin:0px;font-stretch:normal;line-height:normal;min-height:14px"><br></div><div style="margin:0px;font-stretch:normal;line-height:normal">- Dave</div><div><br><blockquote type="cite"><div>On Jul 20, 2020, at 5:21 AM, Eduardo Caldas via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:</div><br><div><div dir="ltr"><div><font face="monospace">template <class T><br></font></div><div><font face="monospace">void f() {<br>  // Case 1:<br>  typename T::template U<int>::type x;<br>  // 'T::template U<int>' in the NNS is a DependentTemplateSpecializationType.<br><br>  // Case 2:<br>  typename T::U::type y;<br>  // 'T::U' in the NNS is an Identifier. Why not a DependentNameType?<br>}</font><br></div><br></div>
_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br></div></blockquote></div><br></div>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>