[clang] Add source range to 'use of undeclared identifier' diagnostics (#117671)

Author: tbaederr

clang/lib/Frontend/DiagnosticRenderer.cpp

@@ -454,62 +454,41 @@ void DiagnosticRenderer::emitSingleMacroExpansion(
                  SpellingRanges, {});
 }
 
-/// Check that the macro argument location of Loc starts with ArgumentLoc.
-/// The starting location of the macro expansions is used to differeniate
-/// different macro expansions.
-static bool checkLocForMacroArgExpansion(SourceLocation Loc,
-                                         const SourceManager &SM,
-                                         SourceLocation ArgumentLoc) {
-  SourceLocation MacroLoc;
-  if (SM.isMacroArgExpansion(Loc, &MacroLoc)) {
-    if (ArgumentLoc == MacroLoc) return true;
-  }
-
-  return false;
-}
-
-/// Check if all the locations in the range have the same macro argument
-/// expansion, and that the expansion starts with ArgumentLoc.
-static bool checkRangeForMacroArgExpansion(CharSourceRange Range,
-                                           const SourceManager &SM,
-                                           SourceLocation ArgumentLoc) {
-  SourceLocation BegLoc = Range.getBegin(), EndLoc = Range.getEnd();
-  while (BegLoc != EndLoc) {
-    if (!checkLocForMacroArgExpansion(BegLoc, SM, ArgumentLoc))
-      return false;
-    BegLoc.getLocWithOffset(1);
-  }
-
-  return checkLocForMacroArgExpansion(BegLoc, SM, ArgumentLoc);
-}
-
 /// A helper function to check if the current ranges are all inside the same
 /// macro argument expansion as Loc.
-static bool checkRangesForMacroArgExpansion(FullSourceLoc Loc,
-                                            ArrayRef<CharSourceRange> Ranges) {
+static bool
+rangesInsideSameMacroArgExpansion(FullSourceLoc Loc,
+                                  ArrayRef<CharSourceRange> Ranges) {
   assert(Loc.isMacroID() && "Must be a macro expansion!");
 
-  SmallVector<CharSourceRange, 4> SpellingRanges;
+  SmallVector<CharSourceRange> SpellingRanges;
   mapDiagnosticRanges(Loc, Ranges, SpellingRanges);
 
-  // Count all valid ranges.
   unsigned ValidCount =
       llvm::count_if(Ranges, [](const auto &R) { return R.isValid(); });
-
   if (ValidCount > SpellingRanges.size())
     return false;
 
-  // To store the source location of the argument location.
-  FullSourceLoc ArgumentLoc;
+  const SourceManager &SM = Loc.getManager();
+  for (const auto &R : Ranges) {
+    // All positions in the range need to point to Loc.
+    SourceLocation Begin = R.getBegin();
+    if (Begin == R.getEnd()) {
+      if (!SM.isMacroArgExpansion(Begin))
+        return false;
+      continue;
+    }
 
-  // Set the ArgumentLoc to the beginning location of the expansion of Loc
-  // so to check if the ranges expands to the same beginning location.
-  if (!Loc.isMacroArgExpansion(&ArgumentLoc))
-    return false;
+    while (Begin != R.getEnd()) {
+      SourceLocation MacroLoc;
+      if (!SM.isMacroArgExpansion(Begin, &MacroLoc))
+        return false;
+      if (MacroLoc != Loc)
+        return false;
 
-  for (const auto &Range : SpellingRanges)
-    if (!checkRangeForMacroArgExpansion(Range, Loc.getManager(), ArgumentLoc))
-      return false;
+      Begin = Begin.getLocWithOffset(1);
+    }
+  }
 
   return true;
 }
@@ -539,13 +518,13 @@ void DiagnosticRenderer::emitMacroExpansions(FullSourceLoc Loc,
   while (L.isMacroID()) {
     // If this is the expansion of a macro argument, point the caret at the
     // use of the argument in the definition of the macro, not the expansion.
-    if (SM.isMacroArgExpansion(L))
+    if (SM.isMacroArgExpansion(L)) {
       LocationStack.push_back(SM.getImmediateExpansionRange(L).getBegin());
-    else
-      LocationStack.push_back(L);
 
-    if (checkRangesForMacroArgExpansion(FullSourceLoc(L, SM), Ranges))
-      IgnoredEnd = LocationStack.size();
+      if (rangesInsideSameMacroArgExpansion(FullSourceLoc(L, SM), Ranges))
+        IgnoredEnd = LocationStack.size();
+    } else
+      LocationStack.push_back(L);
 
     L = SM.getImmediateMacroCallerLoc(L);
 

clang/lib/Sema/SemaExpr.cpp

@@ -2376,20 +2376,22 @@ Sema::DecomposeUnqualifiedId(const UnqualifiedId &Id,
   }
 }
 
-static void emitEmptyLookupTypoDiagnostic(
-    const TypoCorrection &TC, Sema &SemaRef, const CXXScopeSpec &SS,
-    DeclarationName Typo, SourceLocation TypoLoc, ArrayRef<Expr *> Args,
-    unsigned DiagnosticID, unsigned DiagnosticSuggestID) {
+static void emitEmptyLookupTypoDiagnostic(const TypoCorrection &TC,
+                                          Sema &SemaRef, const CXXScopeSpec &SS,
+                                          DeclarationName Typo,
+                                          SourceRange TypoRange,
+                                          unsigned DiagnosticID,
+                                          unsigned DiagnosticSuggestID) {
   DeclContext *Ctx =
       SS.isEmpty() ? nullptr : SemaRef.computeDeclContext(SS, false);
   if (!TC) {
     // Emit a special diagnostic for failed member lookups.
     // FIXME: computing the declaration context might fail here (?)
     if (Ctx)
-      SemaRef.Diag(TypoLoc, diag::err_no_member) << Typo << Ctx
-                                                 << SS.getRange();
+      SemaRef.Diag(TypoRange.getBegin(), diag::err_no_member)
+          << Typo << Ctx << TypoRange;
     else
-      SemaRef.Diag(TypoLoc, DiagnosticID) << Typo;
+      SemaRef.Diag(TypoRange.getBegin(), DiagnosticID) << Typo << TypoRange;
     return;
   }
 
@@ -2400,12 +2402,13 @@ static void emitEmptyLookupTypoDiagnostic(
                         ? diag::note_implicit_param_decl
                         : diag::note_previous_decl;
   if (!Ctx)
-    SemaRef.diagnoseTypo(TC, SemaRef.PDiag(DiagnosticSuggestID) << Typo,
-                         SemaRef.PDiag(NoteID));
+    SemaRef.diagnoseTypo(
+        TC, SemaRef.PDiag(DiagnosticSuggestID) << Typo << TypoRange,
+        SemaRef.PDiag(NoteID));
   else
-    SemaRef.diagnoseTypo(TC, SemaRef.PDiag(diag::err_no_member_suggest)
-                                 << Typo << Ctx << DroppedSpecifier
-                                 << SS.getRange(),
+    SemaRef.diagnoseTypo(TC,
+                         SemaRef.PDiag(diag::err_no_member_suggest)
+                             << Typo << Ctx << DroppedSpecifier << TypoRange,
                          SemaRef.PDiag(NoteID));
 }
 
@@ -2474,6 +2477,7 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
                                ArrayRef<Expr *> Args, DeclContext *LookupCtx,
                                TypoExpr **Out) {
   DeclarationName Name = R.getLookupName();
+  SourceRange NameRange = R.getLookupNameInfo().getSourceRange();
 
   unsigned diagnostic = diag::err_undeclared_var_use;
   unsigned diagnostic_suggest = diag::err_undeclared_var_use_suggest;
@@ -2531,13 +2535,12 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
   // We didn't find anything, so try to correct for a typo.
   TypoCorrection Corrected;
   if (S && Out) {
-    SourceLocation TypoLoc = R.getNameLoc();
     assert(!ExplicitTemplateArgs &&
            "Diagnosing an empty lookup with explicit template args!");
     *Out = CorrectTypoDelayed(
         R.getLookupNameInfo(), R.getLookupKind(), S, &SS, CCC,
         [=](const TypoCorrection &TC) {
-          emitEmptyLookupTypoDiagnostic(TC, *this, SS, Name, TypoLoc, Args,
+          emitEmptyLookupTypoDiagnostic(TC, *this, SS, Name, NameRange,
                                         diagnostic, diagnostic_suggest);
         },
         nullptr, CTK_ErrorRecovery, LookupCtx);
@@ -2616,12 +2619,13 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
                             ? diag::note_implicit_param_decl
                             : diag::note_previous_decl;
       if (SS.isEmpty())
-        diagnoseTypo(Corrected, PDiag(diagnostic_suggest) << Name,
+        diagnoseTypo(Corrected, PDiag(diagnostic_suggest) << Name << NameRange,
                      PDiag(NoteID), AcceptableWithRecovery);
       else
-        diagnoseTypo(Corrected, PDiag(diag::err_no_member_suggest)
-                                  << Name << computeDeclContext(SS, false)
-                                  << DroppedSpecifier << SS.getRange(),
+        diagnoseTypo(Corrected,
+                     PDiag(diag::err_no_member_suggest)
+                         << Name << computeDeclContext(SS, false)
+                         << DroppedSpecifier << NameRange,
                      PDiag(NoteID), AcceptableWithRecovery);
 
       // Tell the callee whether to try to recover.
@@ -2634,13 +2638,12 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
   // FIXME: computing the declaration context might fail here (?)
   if (!SS.isEmpty()) {
     Diag(R.getNameLoc(), diag::err_no_member)
-      << Name << computeDeclContext(SS, false)
-      << SS.getRange();
+        << Name << computeDeclContext(SS, false) << NameRange;
     return true;
   }
 
   // Give up, we can't recover.
-  Diag(R.getNameLoc(), diagnostic) << Name;
+  Diag(R.getNameLoc(), diagnostic) << Name << NameRange;
   return true;
 }