[PATCH] D150191: [clang][Diagnostics] Provide a source range for 'use of undeclared identifier' diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:2187
     else
-      SemaRef.Diag(TypoLoc, DiagnosticID) << Typo;
+      SemaRef.Diag(TypoRange.getEnd(), DiagnosticID) << Typo;
     return;
----------------
tbaeder wrote:
> I'm not passing the `TypoRange` here now, which regresses the test case I posted. Apparently the handling of `-fmacro-backtrace-limit` depends on the range passed here? That seems weird.
Is it failing within `checkRangesForMacroArgExpansion()` in DiagnosticRenderer.cpp? It looks like this change effectively undoes the work from ecd36ee80b7a6ac73c84da19f8a75c4c025a7625


================
Comment at: clang/test/Misc/reduced-diags-macros-backtrace.cpp:30-38
+// ALL-NEXT: {{.*}}:6:20: note: expanded from macro 'ADD'
+// ALL-NEXT: #define ADD(x,y) G(x) + y
+// ALL-NEXT:                    ^
+// ALL-NEXT: {{.*}}:5:16: note: expanded from macro 'G'
+// ALL-NEXT: #define G(x) F(x) + 2
+// ALL-NEXT:                ^
+// ALL-NEXT: {{.*}}:4:14: note: expanded from macro 'F'
----------------
These notes make it harder to read the diagnostic instead of easier, I think they should remain suppressed.


================
Comment at: clang/test/Misc/reduced-diags-macros-backtrace.cpp:42-56
+// ALL-NEXT: {{.*}}:10:26: note: expanded from macro 'LEVEL1'
+// ALL-NEXT: #define LEVEL1(x) LEVEL2(x)
+// ALL-NEXT:                          ^
+// ALL-NEXT: {{.*}}:9:26: note: expanded from macro 'LEVEL2'
+// ALL-NEXT: #define LEVEL2(x) LEVEL3(x)
+// ALL-NEXT:                          ^
+// ALL-NEXT: {{.*}}:8:26: note: expanded from macro 'LEVEL3'
----------------
Same here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150191/new/

https://reviews.llvm.org/D150191