PR10405 Missing actual type (aka) in error message when using decltype as a template parameter

[nikola]

Summary

Desugar TemplateSpecializationType if any of the template arguments is a decltype.

'aka Foo<int>' is much better than 'Foo<decltype(arbitrary expression of type int)>' :)

[rsmith]

This seems like a very special case; I wonder if there's some more general rule we should be using here. (Perhaps we should provide an 'aka' if there's a decltype anywhere in the type?) In any case, the same rule should also apply to GNU __typeof.

[nikola]

What cases would we need to handle to make this more generic? I'm thinking FunctionType (return type and parameters) and Template specialization (but at arbitrary nesting level, vector<vector<decltype(something)>>). Have I missed any (plain arrays), and more importantly is there a way to get this info at the moment?

[rsmith]

I think the best approach for a template specialization type would probably be to walk the explicitly-specified template arguments, and desugar each one as if it were a top-level type. If you desugar any, rebuild the template specialization type with the desugared type. Doing the same thing for the parameter and return types of function types might also make some sense.

[nikola]

This should handle templates in a generic way for both decltype and __typeof.

[nikola]

In what situation would FunctionType show up? In this case x is a PointerType which is not sugared.

int (*x)();
int t = x;

[rsmith]

Here's one way to get a diagnostic with a FunctionType in it:

void (&f)(decltype(1+1)) = 0;

This gives:

<stdin>:1:8: error: non-const lvalue reference to type 'void (decltype(1 + 1))' cannot bind to a temporary of type 'int'

lib/AST/ASTDiagnostic.cpp

68–70

Better would be something like "Desugar template specializations if any template argument should be desugared."

70–83

It'd be better here to rebuild a TemplateSpecializationType that has the desugared template arguments in it.

72

Please don't use a TemplateSpecializationType as a container like this; it's not notionally a container of template arguments. Instead, add an arguments member function to TemplateSpecializationType to return the arguments list.

73

Please use 2-space indent, not 4.

[nikola]

lib/AST/ASTDiagnostic.cpp

72

The class already has getArgs, getNumArgs and getArg methods. But why does it have begin and end if not to be used as a container?

[nikola]

lib/AST/ASTDiagnostic.cpp

70–83

What exactly do you mean by rebuild?

[rsmith]

lib/AST/ASTDiagnostic.cpp

70–83

I mean, ask the ASTContext to create a new TemplateSpecializationType with the desugared template arguments.

[nikola]

lib/AST/ASTDiagnostic.cpp

70–83

I thought it'd be obvious why I was doing this after I did it but I guess I should have asked. At first I though this was an optimization, create this QualType now as it will certainly be created later on. But I've noticed that getting the canonical type from sugared type returns a different type object with same 'spelling'. I know that we're supposed to have only one QualType per type and I'm lost... Is this what you had in mind, and more importantly why?

if (const TemplateSpecializationType *TST =
        dyn_cast<TemplateSpecializationType>(Ty)) {
  SmallVector<TemplateArgument, 4> Args;
  for (TemplateSpecializationType::iterator I = TST->args_begin(),
                                            E = TST->args_end();
       I != E; ++I) {
    if (I->getKind() == TemplateArgument::Type)
      Args.push_back(Desugar(Context, I->getAsType(), ShouldAKA));
    else
      Args.push_back(*I);
  }

  if (ShouldAKA || TST->isTypeAlias()) {
    QT = Context.getTemplateSpecializationType(
        TST->getTemplateName(), Args.data(), Args.size(), QT);
    break;
  }
}

[rsmith]

lib/AST/ASTDiagnostic.cpp

70–83

Yes, that's approximately what I had in mind. This way, we should perform an appropriate amount of desugaring on template arguments, and not just flatten them straight to their canonical types.

Comments on that code:

• ShouldAKA might have been set by something already; track your own variable to determine if you desugared any arguments
• It would be useful to also desugar non-type template arguments; maybe pick up the canonical TST's template argument if the non-type template argument is in Expression form?

[nikola]

It's been some time since we looked at this.

I'm having issues addressing your last comment, non type template parameters. What's the idea here, to turn A<1+2> into A<int>? While tinkering with this I managed to produce desugar A<1+2> as A<3> by accident so I'm not really sure what you meant.

I was surprised to see that both A<1> and A<1+2> have a TemplateArgument with Expression kind. Having 'aka A<int>' for each A<1> seems too spammy but I couldn't figure out how to differentiate them.

[nikola]

Ping.

[nikola]

Pong.

[nikola]

Ding.

[rsmith]

Looks great, thanks!

[nikola]

Committed in r242371 with minor changes to preserve nullability attribute.

lib/AST/ASTDiagnostic.cpp

       continue;
     }
 
-    // Don't desugar template specializations, unless it's an alias template.
+    // Don't desugar template specializations, unless it's an alias template
+    // or one of the template arguments is a decltype.
     if (const TemplateSpecializationType *TST
-          = dyn_cast<TemplateSpecializationType>(Ty))
-      if (!TST->isTypeAlias())
+        = dyn_cast<TemplateSpecializationType>(Ty)) {
+      bool AnyDecltypeArgs = false;
+      for (auto Arg : *TST)
+        if (Arg.getKind() == TemplateArgument::Type &&
+            Arg.getAsType()->getAs<DecltypeType>()) {
+            AnyDecltypeArgs = true;
+          break;
+        }
+
+      if (!AnyDecltypeArgs && !TST->isTypeAlias())
         break;
+    }
 
     // Don't desugar magic Objective-C types.
     if (QualType(Ty,0) == Context.getObjCIdType() ||

test/Misc/diag-aka-types.cpp

 void test() {
  helper(&ns::str::method); // expected-error{{no matching function for call to 'helper'}}
 }
+
+// PR10405
+template <typename T>
+class Temp {};
+
+int t = Temp<decltype(1 + 2)>(); // expected-error{{no viable conversion from 'Temp<decltype(1 + 2)>' (aka 'Temp<int>') to 'int'}}