[Sema] Fixes an assertion failure while instantiation a template with an incomplete typo corrected type

[Mordante]

Summary

This fixes bug 35682. When a template in instantiated with an incomplete typo corrected type an assertion can trigger if the -ferror-limit is used to reduce the number of errors. The issue can be reproduced with the following code:

#include <utility>
        
using SetKeyType = String;                                                      
std::pair<SetKeyType, int> v;

and compiled with: clang -stdlib=libc++ -ferror-limit=1 -c bug.cc
This requires the stdlib=libc++ option, without it the assertion does not trigger. Neither does it trigger when the -ferror-limit=1 is not used. The test case is based on this sample but no longer requires the -stdlib=libc++ option.

[lebedev.ri]

Test?

clang/lib/Sema/SemaTemplate.cpp

726

no else after return

[Mordante]

Thanks for feedback. I'll whether I can find a way to generate a nice test case.

clang/lib/Sema/SemaTemplate.cpp

726

Will do in the next update.

[Mordante]

Fixes @lebedev.ri's remark and adds the requested test.
While working on the test I discovered the initial assumption in bug 35682 was incorrect. Updated the title and summary accordingly.

[lebedev.ri]

clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp

9–10 ↗

(On Diff #210128)

Please don't pull in any external headers - just mock what you need.

[Mordante]

Remove the includes from the test.
Changed the std::is_constructible to is_same since the latter is easier to mock.

[lebedev.ri]

Test looks good, adding a few more potential reviewers..

[Mordante]

Update bug number references:

• Removed from the source
• Use PR35682 in the test

Note: I tried to use -verify instead of FileCheck but that doesn't seem to work well since -ferror-limit 1 reports no line number. Other tests with -ferror-limit 1 also use FileCheck.

[aaron.ballman]

clang/lib/Sema/SemaTemplate.cpp

721–729

The call to std::move() shouldn't be necessary.

clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp

1 ↗

(On Diff #218221)

Do you intend to test %clang_cc1 instead? Can the test be rewritten to use -verify rather than FileCheck? If the test crashes or asserts, that will cause the test to fail anyway, so it doesn't seem like it needs to FileCheck.

[Mordante]

clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp

1 ↗

(On Diff #218221)

As discussed on IRC this is not possible due to the fact there is an error without line number.
(frontend): too many errors emitted, stopping now

[Mordante]

Removes the unnecessary std::move as suggested by @aaron.ballman.

[aaron.ballman]

LGTM!

[Mordante]

Thanks for the review.
Could you commit this patch? I don't have commit access.

[aaron.ballman]

In D64644#1661417, @Mordante wrote:

Thanks for the review.
Could you commit this patch? I don't have commit access.

Happy to do so -- I've commit in r371320.

clang/lib/Sema/SemaTemplate.cpp

Sema::ActOnDependentIdExpression(const CXXScopeSpec &SS,
   return BuildDependentDeclRefExpr(SS, TemplateKWLoc, NameInfo, TemplateArgs);
 }
 
 ExprResult
 Sema::BuildDependentDeclRefExpr(const CXXScopeSpec &SS,
                                 SourceLocation TemplateKWLoc,
                                 const DeclarationNameInfo &NameInfo,
                                 const TemplateArgumentListInfo *TemplateArgs) {
-  return DependentScopeDeclRefExpr::Create(
-      Context, SS.getWithLocInContext(Context), TemplateKWLoc, NameInfo,
+  // DependentScopeDeclRefExpr::Create requires a valid QualifierLoc
+  // See https://bugs.llvm.org/show_bug.cgi?id=35682
+  NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
+  if (!QualifierLoc)
+    return ExprError();
+  else
+    return DependentScopeDeclRefExpr::Create(Context, std::move(QualifierLoc),
+                                             TemplateKWLoc, NameInfo,
                                              TemplateArgs);
 }
 
 
 /// Determine whether we would be unable to instantiate this template (because
 /// it either has no definition, or is in the process of being instantiated).
 bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
                                           NamedDecl *Instantiation,
                                           bool InstantiatedFromMember,