hokein added inline comments.
================
Comment at: clang/lib/AST/TemplateName.cpp:80
TemplateName::TemplateName(DependentTemplateName *Dep) : Storage(Dep) {}
+TemplateName::TemplateName(UsingShadowDecl *Using) : Storage(Using) {}
----------------
sammccall wrote:
> assert(isa<TemplateDecl>(Using->getTargetDecl()))
>
> (better to crash upfront than in getAsTemplateDecl() or so)
There is already a stricter assertion `TD == Using->getUnderlyingDecl()` in
every construct side.
================
Comment at: clang/lib/AST/TemplateName.cpp:246
TemplateNameDependenceScope::DependentInstantiation)
Template->printQualifiedName(OS, Policy);
else
----------------
sammccall wrote:
> this is going to print the wrong qualifier: qualifier of the underlying
> rather than of the usingdecl
I think this is a good argument -- after the code `namespace ns { using
std::vector; }`, what's the fully-qualified name of the `vector`
UsingTemplateName within ns?
1) `ns::vector` -- the qualified name of using-decl.
2) or `std::vector`- the qualified name of the underlying template decl.
after a second thought, I'm leaning towards 2). AFAIK, the major place to print
fully-qualified name is `TemplateArgument::print` (for template template
parameters). 2) is providing more useful information for users to locate the
actual template class. Imaging a case where we have a `using
long::long::long::abc` in a global namespace, printing `abc` seems less useful.
This is also consistent with `UsingType`.
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:292
+ Context.getQualifiedTemplateName(Qualifier, hasTemplateKeyword, TD);
+ } else if (!R.isAmbiguous() && R.begin()->getKind() == Decl::UsingShadow) {
+ UsingShadowDecl *Using = cast<UsingShadowDecl>(*R.begin());
----------------
sammccall wrote:
> it would be really nice to have a test verifying that if we are ambiguous
> here and form an Overloaded TemplateName, then when it's later resolved it
> turns into a Using TemplateName where appropriate...
> then when it's later resolved it turns into a Using TemplateName where
> appropriate...
clang doesn't seem to work like this -- it does form an Overloaded TemplateName
during parsing, but a call to these overloaded function templates is a
UnresolvedLookupExpr which stores all function candidates. After the template
instantiation, the UnresolvedLookupExpr is resolved to a normal DeclRefExpr
(that said there is no Using TemplateName being built).
The reason I added the `isAmbiguous` guard here was to ensure the
`Using->getUnderlyingDecl() == TD` invariant, it is always true for correct
cases. However, we pickup an *arbitrary* template for error-recovery (see line
231). Now I have figured out a better way to handle it (by storing the
corresponding FoundUsingDecl) without needing this `isAmbiguous` guard. Removed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123127/new/
https://reviews.llvm.org/D123127
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
