Fix a crash when an error occurs in Template and the initializer is a nullptr for C++17

[bviyer]

Summary

When the error has occurred for one of the variables inside a template, the Loc function will try to find the end-location of the final item. If this variable is default initialized to nullptr, then the templateArgumentLoc function is invoked. It has an assert to see if the Argument's kind is Expression. If the default initializer is a nullptr, then the argument's kind will be NullPtr, not Expression. This will cause a compiler crash. This patch will allow NullPtr as a possibility for expression's kind.

[erik.pilkington]

Hi Balaji,

I reduced your testcase a bit more, this looks like the can be a crash on valid. Can you use the more minimal version in the testcase?

template <typename a, int* = nullptr>
struct e {
    e(a) {}
};
e c(0);

Also: you should add cfe-commits as a subscriber when creating new phab revisions. IIRC there was some issue with adding cfe-commits after the fact.

include/clang/AST/TemplateBase.h

469 ↗

(On Diff #157156)

Please use // comments in C++ files!

I think this comment is pretty superfluous though. If a future reader came across this then they would probably not care too much about the details of this bug.

474–475 ↗

(On Diff #157156)

I think we should let this constructor work with any non type template argument. If you agree, could you add the extra cases? (Should just be TemplateArgument::Declaration and TemplateArgument::Integral)

[bviyer]

I have fixed all the changes requested by Erik along with shortening the test case.

[erik.pilkington]

LGTM, thanks! Do you have your commit rights yet?

include/clang/AST/TemplateBase.h

469 ↗

(On Diff #157721)

This comment sounds pretty strange, how about "Permit any kind of template argument that can be represented with an expression"?

[bviyer]

Fixed the comment as you suggested.

I do not have check in rights yet. Can you please check it in for me?

cfe/trunk/include/clang/AST/TemplateBase.h

public:
 
   TemplateArgumentLoc(const TemplateArgument &Argument, TypeSourceInfo *TInfo)
       : Argument(Argument), LocInfo(TInfo) {
     assert(Argument.getKind() == TemplateArgument::Type);
   }
 
   TemplateArgumentLoc(const TemplateArgument &Argument, Expr *E)
       : Argument(Argument), LocInfo(E) {
-    assert(Argument.getKind() == TemplateArgument::Expression);
+
+    // Permit any kind of template argument that can be represented with an
+    // expression
+    assert(Argument.getKind() == TemplateArgument::NullPtr ||
+           Argument.getKind() == TemplateArgument::Integral ||
+           Argument.getKind() == TemplateArgument::Declaration ||
+           Argument.getKind() == TemplateArgument::Expression);
   }
 
   TemplateArgumentLoc(const TemplateArgument &Argument,
                       NestedNameSpecifierLoc QualifierLoc,
                       SourceLocation TemplateNameLoc,
                       SourceLocation EllipsisLoc = SourceLocation())
       : Argument(Argument),
         LocInfo(QualifierLoc, TemplateNameLoc, EllipsisLoc) {

cfe/trunk/test/SemaObjCXX/class-templ-error-null-init.mm

+// RUN: %clang_cc1 -fsyntax-only -std=c++17  -verify %s
+// expected-no-diagnostics
+template <typename a, int* = nullptr>
+struct e {
+    e(a) {}
+};
+e c(0);