[AST] Record SourceLocation for TypoExpr.

[sammccall]

clang/include/clang/AST/Expr.h

6159–6160

can we have Start != End in practice?

Looking at DeclarationNameInfo::getEndLocPrivate(), it seems like this only happens in operators, literal-operators, conversions, constructors/destructors.

If these don't actually support typo correction maybe we should just use one location?

[hokein]

use a single location.

[hokein]

clang/include/clang/AST/Expr.h

6159–6160

ah, yes, I missed this.

typo correction only supports identifiers, so Start and End are always the same.

clang/include/clang/AST/Expr.h

public:
   std::unique_ptr<AtomicScopeModel> getScopeModel() const {
     return getScopeModel(getOp());
   }
 };
 
 /// TypoExpr - Internal placeholder for expressions where typo correction
 /// still needs to be performed and/or an error diagnostic emitted.
 class TypoExpr : public Expr {
+  // The location for the typo name.
+  SourceLocation TypoLoc;
+
 public:
-  TypoExpr(QualType T) : Expr(TypoExprClass, T, VK_LValue, OK_Ordinary) {
+  TypoExpr(QualType T, SourceLocation TypoLoc)
+      : Expr(TypoExprClass, T, VK_LValue, OK_Ordinary), TypoLoc(TypoLoc) {
     assert(T->isDependentType() && "TypoExpr given a non-dependent type");
     setDependence(ExprDependence::TypeValueInstantiation |
                   ExprDependence::Error);
   }
 
   child_range children() {
     return child_range(child_iterator(), child_iterator());
   }
   const_child_range children() const {
     return const_child_range(const_child_iterator(), const_child_iterator());
   }
 
-  SourceLocation getBeginLoc() const LLVM_READONLY { return SourceLocation(); }
-  SourceLocation getEndLoc() const LLVM_READONLY { return SourceLocation(); }
+  SourceLocation getBeginLoc() const LLVM_READONLY { return TypoLoc; }
+  SourceLocation getEndLoc() const LLVM_READONLY { return TypoLoc; }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == TypoExprClass;
   }
 
 };
 
 /// Frontend produces RecoveryExprs on semantic errors that prevent creating

clang/include/clang/Sema/Sema.h

private:
   };
 
   /// The set of unhandled TypoExprs and their associated state.
   llvm::MapVector<TypoExpr *, TypoExprState> DelayedTypos;
 
   /// Creates a new TypoExpr AST node.
   TypoExpr *createDelayedTypo(std::unique_ptr<TypoCorrectionConsumer> TCC,
                               TypoDiagnosticGenerator TDG,
-                              TypoRecoveryCallback TRC);
+                              TypoRecoveryCallback TRC, SourceLocation TypoLoc);
 
   // The set of known/encountered (unique, canonicalized) NamespaceDecls.
   //
   // The boolean value will be true to indicate that the namespace was loaded
   // from an AST/PCH file, or false otherwise.
   llvm::MapVector<NamespaceDecl*, bool> KnownNamespaces;
 
   /// Whether we have already loaded known namespaces from an extenal

clang/lib/Sema/SemaExprCXX.cpp

if (DiscardedValue) {
 
     DiagnoseUnusedExprResult(FullExpr.get());
   }
 
   FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
   if (FullExpr.isInvalid()) {
     // Typo-correction fails, we rebuild the broken AST with the typos degraded
     // to RecoveryExpr.
-    // FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
-    // track source locations.
     struct TyposReplace : TreeTransform<TyposReplace> {
       TyposReplace(Sema &SemaRef) : TreeTransform(SemaRef) {}
       ExprResult TransformTypoExpr(TypoExpr *E) {
         return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
                                                 E->getEndLoc(), {});
       }
     } TT(*this);
 

clang/lib/Sema/SemaLookup.cpp

if (!Consumer || Consumer->empty())
     return nullptr;
 
   // Make sure the best edit distance (prior to adding any namespace qualifiers)
   // is not more that about a third of the length of the typo's identifier.
   unsigned ED = Consumer->getBestEditDistance(true);
   IdentifierInfo *Typo = TypoName.getName().getAsIdentifierInfo();
   if (!ExternalTypo && ED > 0 && Typo->getName().size() / ED < 3)
     return nullptr;
-
   ExprEvalContexts.back().NumTypos++;
-  return createDelayedTypo(std::move(Consumer), std::move(TDG), std::move(TRC));
+  return createDelayedTypo(std::move(Consumer), std::move(TDG), std::move(TRC),
+                           TypoName.getLoc());
 }
 
 void TypoCorrection::addCorrectionDecl(NamedDecl *CDecl) {
   if (!CDecl) return;
 
   if (isKeyword())
     CorrectionDecls.clear();
 
void Sema::diagnoseTypo(const TypoCorrection &Correction,
 
   // Add any extra diagnostics.
   for (const PartialDiagnostic &PD : Correction.getExtraDiagnostics())
     Diag(Correction.getCorrectionRange().getBegin(), PD);
 }
 
 TypoExpr *Sema::createDelayedTypo(std::unique_ptr<TypoCorrectionConsumer> TCC,
                                   TypoDiagnosticGenerator TDG,
-                                  TypoRecoveryCallback TRC) {
+                                  TypoRecoveryCallback TRC,
+                                  SourceLocation TypoLoc) {
   assert(TCC && "createDelayedTypo requires a valid TypoCorrectionConsumer");
-  auto TE = new (Context) TypoExpr(Context.DependentTy);
+  auto TE = new (Context) TypoExpr(Context.DependentTy, TypoLoc);
   auto &State = DelayedTypos[TE];
   State.Consumer = std::move(TCC);
   State.DiagHandler = std::move(TDG);
   State.RecoveryHandler = std::move(TRC);
   if (TE)
     TypoExprs.push_back(TE);
   return TE;
 }

clang/test/AST/ast-dump-recovery.cpp

 // RUN: not %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -frecovery-ast -frecovery-ast-type -ast-dump %s | FileCheck -strict-whitespace %s
 // RUN: not %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -fno-recovery-ast -ast-dump %s | FileCheck --check-prefix=DISABLED -strict-whitespace %s
 
 int some_func(int *);
 
 // CHECK:     VarDecl {{.*}} invalid_call
 // CHECK-NEXT: `-ImplicitCastExpr {{.*}} 'int' contains-errors
 // CHECK-NEXT:  `-RecoveryExpr {{.*}} 'int' contains-errors
 // CHECK-NEXT:    |-UnresolvedLookupExpr {{.*}} 'some_func'
 // CHECK-NEXT:    `-IntegerLiteral {{.*}} 123
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int invalid_call = some_func(123);
 void test_invalid_call(int s) {
   // CHECK:      CallExpr {{.*}} '<dependent type>' contains-errors
   // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
-  // CHECK-NEXT: |-RecoveryExpr {{.*}} <<invalid sloc>>
-  // CHECK-NEXT: `-BinaryOperator {{.*}} <<invalid sloc>, col:28>
-  // CHECK-NEXT:   |-RecoveryExpr {{.*}} <<invalid sloc>>
+  // CHECK-NEXT: |-RecoveryExpr {{.*}} <col:13>
+  // CHECK-NEXT: `-BinaryOperator {{.*}}
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}}
   // CHECK-NEXT:   `-IntegerLiteral {{.*}} <col:28> 'int' 1
   some_func(undef1, undef2+1);
 
   // CHECK:      BinaryOperator {{.*}} '<dependent type>' contains-errors '='
   // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
   // CHECK-NEXT: `-CallExpr {{.*}} '<dependent type>' contains-errors
   // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors