[clang] Fix a nullptr dereference bug on invalid code

[adamcz]

Summary

When working with invalid code, we would try to dereference a nullptr
while deducing template arguments in some dependend code operating on a
lambda with invalid return type.

[hokein]

clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp

3

A common practice is to avoid depending on STL in tests. I think we need to pull out (even simplify?) std::result_of implementation if it is needed for reproducing the crash.

[adamcz]

clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp

3

I've noticed that pattern, but can't tell why this is. Is it documented somewhere?

[adamcz]

addressed review comments

[adamcz]

PTAL

clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp

3

Anyway, I updated the change to not require type_traits. Turned out to be simpler than expected ;-)

[hokein]

Thanks.

clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp

3

I've noticed that pattern, but can't tell why this is. Is it documented somewhere?

I did try to find some documentation about this before making the previous comment, but didn't find any.

Depending on the clang builtin includes is legitimate, but depending on the stand library is more complicated, it would hurt hermeticity of the test -- you may not always get the exact behavior you intend to test (as it may depend on the stand library versions, host environment etc).

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

if (FunctionProtoTypeLoc OldProtoLoc = OldTL.getAs<FunctionProtoTypeLoc>()) {
     if (NewTInfo != OldTInfo) {
       // Get parameters from the new type info.
       TypeLoc NewTL = NewTInfo->getTypeLoc().IgnoreParens();
       FunctionProtoTypeLoc NewProtoLoc = NewTL.castAs<FunctionProtoTypeLoc>();
       unsigned NewIdx = 0;
       for (unsigned OldIdx = 0, NumOldParams = OldProtoLoc.getNumParams();
            OldIdx != NumOldParams; ++OldIdx) {
         ParmVarDecl *OldParam = OldProtoLoc.getParam(OldIdx);
+        if (!OldParam)
+          return nullptr;
+
         LocalInstantiationScope *Scope = SemaRef.CurrentInstantiationScope;
 
         Optional<unsigned> NumArgumentsInExpansion;
         if (OldParam->isParameterPack())
           NumArgumentsInExpansion =
               SemaRef.getNumArgumentsInExpansion(OldParam->getType(),
                                                  TemplateArgs);
         if (!NumArgumentsInExpansion) {

clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp

+// RUN: %clang -fsyntax-only -std=c++17 %s -Xclang -verify
+
+// The important part is that we do not crash.
+
+template<typename T> T declval();
+
+template <typename T>
+auto Call(T x) -> decltype(declval<T>()(0)) {} // expected-note{{candidate template ignored}}
+
+class Status {};
+
+void fun() {
+  // The Status() (instead of Status) here used to cause a crash.
+  Call([](auto x) -> Status() {}); // expected-error{{function cannot return function type 'Status ()}}
+  // expected-error@-1{{no matching function for call to 'Call'}}
+}