Fix an assertion failure in setPointOfInstantiation.

[hokein]

Summary

Fixes PR25668.

[hokein]

Update.

[hokein]

friendly ping.

[bkramer]

This doesn't look right to me. Aborting tree transform on a bad source location seems just wrong. Where is the invalid source location coming from? Also the backtrace seems related to typo correction, maybe the bug is there?

[hokein]

Update.

[hokein]

In D17762#367182, @bkramer wrote:

This doesn't look right to me. Aborting tree transform on a bad source location seems just wrong. Where is the invalid source location coming from? Also the backtrace seems related to typo correction, maybe the bug is there?

I have post the whole stack trace in PR25668.
The invalid source location comes from TypoExpr which is returned by ArraySubscriptExpr->getLHS(), this is also why there is a FIXME in code line. After reading the code more throughly, I find that the invalid source location is expected when constructing a TypoExpr.

I updated the fixing patch now. We probably don't call RequireCompleteType with an invalid source location, which is the same as trunk@84793.

[bkramer]

I'm still not sure if this is the right way to fix it. Maybe @rsmith has an opinion?

[hokein]

friendly ping @rsmith.

[rsmith]

It seems to me that the bug is that we're producing an invalid source location, not how we handle that downstream. Typo correction should be able to preserve the source location of whatever expression it was correcting.

lib/Sema/SemaExpr.cpp

14047–14048 ↗

(On Diff #49939)

General rule: for a function that returns true to say "an error occurred", it is not acceptable to return true without either (a) producing an error or (b) observing some state that proves someone else emitted an error (such as a decl that has been set as invalid).

lib/Sema/TreeTransform.h

 }
 
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformArraySubscriptExpr(ArraySubscriptExpr *E) {
   ExprResult LHS = getDerived().TransformExpr(E->getLHS());
   if (LHS.isInvalid())
     return ExprError();
+  if (E->getLHS()->getLocStart().isInvalid()) {
+    return ExprError();
+  }
 
   ExprResult RHS = getDerived().TransformExpr(E->getRHS());
   if (RHS.isInvalid())
     return ExprError();
 
 
   if (!getDerived().AlwaysRebuild() &&
       LHS.get() == E->getLHS() &&
       RHS.get() == E->getRHS())
     return E;
 
   return getDerived().RebuildArraySubscriptExpr(LHS.get(),
-                                           /*FIXME:*/E->getLHS()->getLocStart(),
+                                                E->getLHS()->getLocStart(),
                                                 RHS.get(),
                                                 E->getRBracketLoc());
 }
 
 template <typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformOMPArraySectionExpr(OMPArraySectionExpr *E) {
   ExprResult Base = getDerived().TransformExpr(E->getBase());

test/Sema/PR25668.cpp

+// RUN: %clang_cc1 -verify -fsyntax-only -std=c++11 %s
+// expected-no-assertion-crash
+
+template <class> class A {};
+class {
+  A<int> operator[](int);
+} mask_ {
+  mask[2]; // expected-error{{use of undeclared identifier 'mask'}}
+};