[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:916
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.

sammccall wrote:
> If you like, `DEBUG_WITH_TYPE("DelayedTypos", ...)` would still let people 
> who want to fix these see them. In practice, I suspect nobody actually wants 
> to fix these, though :-(
I've left it out for now, not sure what's the best way to report those: crash? 
show a message in stderr? (probably a short message, indicating the number of 
uncorrected delayed typos in stderr is enough)
Also second the concerns that it's not very useful since no-one is actually 
looking at these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaeae71cd96c3: [Sema] Emit diagnostics for uncorrected 
delayed typos at the end of TU (authored by ilya-biryukov).

Changed prior to commit:
  https://reviews.llvm.org/D64799?vs=210338&id=224005#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/SemaObjC/typo-correction-subscript.m


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify -disable-free
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify
 
 @class Dictionary;
 
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 
'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -38,6 +38,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -383,8 +384,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 void Sema::warnStackExhausted(SourceLocation Loc) {
@@ -934,6 +933,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only -Wno-objc-root-class %s -verify -disable-free
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only -Wno-objc-root-class %s -verify
 
 @class Dictionary;
 
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -38,6 +38,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -383,8 +384,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 void Sema::warnStackExhausted(SourceLocation Loc) {
@@ -934,6 +933,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end o

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D64799#1688293 , @dgoldman wrote:

> In D64799#1605514 , @ilya-biryukov 
> wrote:
>
> > Tried the suggested approach and ran into the issue described above.
> >  Either we make the diagnostics less useful ('did you mean foo::bar?' turns 
> > into 'unresolved identifier bar'; without mentioning the correction)  or we 
> > even stop providing some corrections in addition to that.
> >
> > On the other hand, I agree that over time we will start emitting too many 
> > diagnostics at the end of TU if keep the patch as is. That is not a good 
> > way.
> >  There should be a better option for emitting the uncorrected diagnostics 
> > closer to where they are produced, but I don't have a good idea on what it 
> > should be off the top of my head. Ideas warmly welcome.
>
>
> I might be wrong here, but I thought that diagnostics were delayed until typo 
> correction has been completed on an expression. For example, you'll only get 
> something like `unresolved identifier bar` instead of `did you mean 
> foo::bar?` only when you call the `DiagHandler` with either a proper 
> `TypoCorrection` or an empty one. If so, I'm not sure how you'd get into this 
> case if you're calling `CorrectDelayedTyposInExpr` and tracking which typos 
> have been resolved already.


I couldn't find a place in the code that would make sure the typo expressions 
will not be corrected later.
Therefore, this approach led to worse diagnostics (`unresolved identifier bar` 
vs `did you mean baz?`) in some situations, i.e. when a correction would take 
place somewhere higher up the callstack, but we decided to emit the error 
before that.

In particular, I tried emitting uncorrected typo diagnostics when popping 
expression evaluation contexts, per Richard's suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.
Herald added a subscriber: usaxena95.

In D64799#1651908 , @vsapsai wrote:

> For the record, there was another change regarding the delayed typos in 
> `clang::Sema::~Sema()`: D62648  [Sema][Typo] 
> Fix assertion failure for expressions with multiple typos.


Note that D62648  only makes fixes to the typo 
correction application (e.g. when `CorrectDelayedTyposInExpr` is called), in 
particular caused by the recursive nature of typo correction (correcting typos 
might introduce more typos!). The assertion failure can still be caused by 
failing to correct Typos (e.g not calling `CorrectDelayedTyposInExpr` for a 
given Expression).

This patch seems like a good idea as currently it only impacts clangd/libclang 
in practice. But here a few thoughts about potentially fixing the root cause 
here:

- It looks as if when initially implemented 

 typos were meant to be propagated up through the `ExprEvalContext` (see the 
commit description) as opposed to automatically clearing for every context, 
perhaps due to the issues described above. It's unclear what the intended 
behavior is though - when should `CorrectDelayedTyposInExpr` be called? When 
are we in a `stable state` to do typo correction?
- From my limited understanding there is one main issue here: failure to call 
`CorrectDelayedTyposInExpr` for an expression. When fixing the typo correction 
handling mentioned above, this could happen during `TreeTransform` due to an 
Expression being created (along with a typo) and then being thrown away (due to 
an error). The problem here is that once it is discarded, it becomes 
unreachable. I'm assuming the proper handling here is to 
`CorrectDelayedTyposInExpr` more frequently, but I'm not quite sure.
  - Could `Sema` check if a `TypoExpr` becomes unreachable? If so, we can check 
for unreachable `TypoExprs`, diagnose them, and potentially if in `DEBUG` emit 
a warning (although we'd need to track where a `TypoExpr` was created to really 
understand why it was thrown away).



In D64799#1605514 , @ilya-biryukov 
wrote:

> Tried the suggested approach and ran into the issue described above.
>  Either we make the diagnostics less useful ('did you mean foo::bar?' turns 
> into 'unresolved identifier bar'; without mentioning the correction)  or we 
> even stop providing some corrections in addition to that.
>
> On the other hand, I agree that over time we will start emitting too many 
> diagnostics at the end of TU if keep the patch as is. That is not a good way.
>  There should be a better option for emitting the uncorrected diagnostics 
> closer to where they are produced, but I don't have a good idea on what it 
> should be off the top of my head. Ideas warmly welcome.


I might be wrong here, but I thought that diagnostics were delayed until typo 
correction has been completed on an expression. For example, you'll only get 
something like `unresolved identifier bar` instead of `did you mean foo::bar?` 
only when you call the `DiagHandler` with either a proper `TypoCorrection` or 
an empty one. If so, I'm not sure how you'd get into this case if you're 
calling `CorrectDelayedTyposInExpr` and tracking which typos have been resolved 
already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

For the record, there was another change regarding the delayed typos in 
`clang::Sema::~Sema()`: D62648  [Sema][Typo] 
Fix assertion failure for expressions with multiple typos.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM: while this isn't a solution to the root cause of the issues here, it puts 
us in a better situation than the status quo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D64799#1608085 , @ilya-biryukov 
wrote:

> @rsmith two options for this patch seem to be:
>
> - silently ignore the errors (behavior before this patch),
> - show them to the user at the end of TU (what happens in the current version 
> of the patch). This can result in bad UX, but we won't drop any errors on the 
> floor. Which one do you think we should prefer?


Let's take the second option. It seems better to diagnose an error late than 
not at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

An experiment with popping on expression evaluation context failed, I couldn't 
find a good way to fix the problems described above.
Typo correction can and will run after the evaluation context where expression 
created is popped and the diagnostic we produce depends on the results of typo 
correction.
Emitting diagnostics on some, but not all context pops could be an option, but 
classifying those seems to be hard, would require looking closely at every expr 
eval context push.

> Failing that, I can live with this landing as-is, but delaying these 
> diagnostics to the end of the file is going to result in a bad user 
> experience whenever it happens -- and I suspect it'll start happening 
> significantly more than the assert currently fails, because we'll no longer 
> have this assertion forcing people to call `CorrectDelayedTyposInExpr` when 
> discarding an expression.

In practice, this assertion is not even checked most of the time. As mentioned 
earlier, it only fires when running `clang -cc1` is triggered explicitly 
**without** `-disable-free` (and driver passes `-disable-free` by default).
We ended up in a situation when this assertion was not firing for awhile; 
`clangd` and `libclang` crash because of this assertion and nobody else notices 
because `clang` won't crash and won't produce the errors either.

I'm happy to either emit or leave out those errors, but I think we should 
absolutely get rid of assertion in the Sema destructor that only fires in 
`clang -cc1` and source-level tools.
Placing it at some other place that would help to discover all missing calls to 
`CorrectTypos` to fix typo correction seems ok. However, this change is aimed 
at unbreaking `libclang` and `clangd` and not fixing typo correction (it looks 
like a much harder problem).

@rsmith two options for this patch seem to be:

- silently ignore the errors (current behavior),
- show them to the user at the end of TU (can result in bad UX, but we won't 
drop any errors on the floor).

Which one do you think we should prefer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Tried the suggested approach and ran into the issue described above.
Either we make the diagnostics less useful ('did you mean foo::bar?' turns into 
'unresolved identifier bar'; without mentioning the correction)  or we even 
stop providing some corrections in addition to that.

On the other hand, I agree that over time we will start emitting too many 
diagnostics at the end of TU if keep the patch as is. That is not a good way.
There should be a better option for emitting the uncorrected diagnostics closer 
to where they are produced, but I don't have a good idea on what it should be 
off the top of my head. Ideas warmly welcome.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for the suggestion. Sounds reasonably simple, I'll try this and report 
back with the result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D64799#1605211 , @ilya-biryukov 
wrote:

> @rsmith, emitting the typos on pop expression evaluation context resulted in 
> a bunch of failures when trying to transform the resulting errors,  see a 
> typical stacktrace below.
>  It seems we can actually pop expression evaluation contexts between 
> producing and correcting a typo expression.


I think I see the problem: if we have expression `E1` as a subexpression of 
`E2`, but `E1` has its own expression evaluation context (eg, maybe it's in a 
`sizeof` expression or similar), we'll now diagnose `E1`'s typos when we leave 
that context. That by itself seems fine, but then when we run typo correction 
for `E2`, we rediscover the typo from `E1` and are surprised to find that the 
correction information has been discarded but the `TypoExpr` is still reachable 
in the AST. It seems like we could handle that by tracking the 
already-corrected `TypoExpr`s and teaching `TransformTypos` to substitute in 
the known correction in this case rather than trying to pick a correction for 
itself.

> Would you be ok with landing the workaround as is? Alternatively, any ideas 
> on how we can avoid this problem without extending the scope of the patch too 
> much?

As above, I think we can probably fix this without changing too much by 
tracking which typos we've already resolved rather than throwing away the 
information about them. Failing that, I can live with this landing as-is, but 
delaying these diagnostics to the end of the file is going to result in a bad 
user experience whenever it happens -- and I suspect it'll start happening 
significantly more than the assert currently fails, because we'll no longer 
have this assertion forcing people to call `CorrectDelayedTyposInExpr` when 
discarding an expression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

@rsmith, emitting the typos on pop expression evaluation context resulted in a 
bunch of failures when trying to transform the resulting errors,  see a typical 
stacktrace below.
It seems we can actually pop expression evaluation contexts between producing 
and correcting a typo expression.

I don't see how to workaround this without digging further into typo correction 
and changing its design.
Would you be ok with landing the workaround as is? Alternatively, any ideas on 
how we can avoid this problem without extending the scope of the patch too much?

  FAIL: Clang :: CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp (1595 of 
15306)
   TEST 'Clang :: 
CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp' FAILED 
  Script:
  --
  : 'RUN: at line 1';   
/usr/local/google/home/ibiryukov/projects/llvm/build-rel/bin/clang -cc1 
-internal-isystem 
/usr/local/google/home/ibiryukov/projects/llvm/build-rel/lib/clang/10.0.0/include
 -nostdsysteminc -std=c++11 -verify 
/usr/local/google/home/ibiryukov/projects/llvm/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp
  --
  Exit Code: 134
  
  Command Output (stderr):
  --
  clang: 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaLookup.cpp:5378:
 const Sema::TypoExprState &clang::Sema::getTypoExprState(clang::TypoExpr *) 
const: Assertion `Entry != DelayedTypos.end() && "Failed to get the state for a 
TypoExpr!"' failed.
  Stack dump:
  0.  Program arguments: 
/usr/local/google/home/ibiryukov/projects/llvm/build-rel/bin/clang -cc1 
-internal-isystem 
/usr/local/google/home/ibiryukov/projects/llvm/build-rel/lib/clang/10.0.0/include
 -nostdsysteminc -std=c++11 -verify 
/usr/local/google/home/ibiryukov/projects/llvm/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp
  1.  
/usr/local/google/home/ibiryukov/projects/llvm/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp:23:26:
 current parser token ';'
   #0 0x043e1ea4 PrintStackTrace 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/lib/Support/Unix/Signals.inc:533:13
   #1 0x043e1ea4 PrintStackTraceSignalHandler(void*) 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/lib/Support/Unix/Signals.inc:593:0
   #2 0x043dfa9e llvm::sys::RunSignalHandlers() 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/lib/Support/Signals.cpp:69:18
   #3 0x043e22b8 SignalHandler(int) 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/lib/Support/Unix/Signals.inc:385:1
   #4 0x7fedabb7d3a0 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0)
   #5 0x7fedaac0bcfb raise (/lib/x86_64-linux-gnu/libc.so.6+0x36cfb)
   #6 0x7fedaabf68ad abort (/lib/x86_64-linux-gnu/libc.so.6+0x218ad)
   #7 0x7fedaabf677f (/lib/x86_64-linux-gnu/libc.so.6+0x2177f)
   #8 0x7fedaac04542 (/lib/x86_64-linux-gnu/libc.so.6+0x2f542)
   #9 0x05f4ba2e LookupBucketFor 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/include/llvm/ADT/DenseMap.h:618:5
  #10 0x05f4ba2e find 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/include/llvm/ADT/DenseMap.h:184:0
  #11 0x05f4ba2e find 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/include/llvm/ADT/MapVector.h:154:0
  #12 0x05f4ba2e clang::Sema::getTypoExprState(clang::TypoExpr*) const 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaLookup.cpp:5376:0
  #13 0x05e94695 TransformTypoExpr 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaExprCXX.cpp:7740:27
  #14 0x05e94695 clang::TreeTransform<(anonymous 
namespace)::TransformTypos>::TransformExpr(clang::Expr*) 
/usr/local/google/home/ibiryukov/projects/llvm/build-rel/tools/clang/include/clang/AST/StmtNodes.inc:1277:0
  #15 0x05e708ea hasErrorOccurred 
/usr/local/google/home/ibiryukov/projects/llvm/clang/include/clang/Sema/Sema.h:7864:38
  #16 0x05e708ea TryTransform 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaExprCXX.cpp:7651:0
  #17 0x05e708ea Transform 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaExprCXX.cpp:7686:0
  #18 0x05e708ea clang::Sema::CorrectDelayedTyposInExpr(clang::Expr*, 
clang::VarDecl*, llvm::function_ref 
(clang::Expr*)>) 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaExprCXX.cpp:7784:0
  #19 0x05c5deae isInvalid 
/usr/local/google/home/ibiryukov/projects/llvm/clang/include/clang/Sema/Ownership.h:208:37
  #20 0x05c5deae clang::Sema::AddInitializerToDecl(clang::Decl*, 
clang::Expr*, bool) 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaDecl.cpp:11236:0
  #21 0x05a22997 
clang::Parser::ParseDeclarationAfterDeclaratorAndAttributes(clang::Declarator&, 
clang::Parser::ParsedTemplateInfo const&, clang::Parser::ForRangeInit*) 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Parse/ParseDecl.cpp:2456:3
  #22 0x05a2093c clang

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D64799#1592263 , @rnk wrote:

> In D64799#1591732 , @ilya-biryukov 
> wrote:
>
> > @rsmith, I'll look into emitting the typos when we pop expression 
> > evaluation context, but do we expect this to cover **all** cases where 
> > `TypoExpr`s are produced?
> >  (conservatively assuming that the answer is "no") should we land this 
> > patch and also emit at the end of TU in addition to expression evaluation 
> > context?
>
>
> I was going to pose the question this way: suppose clang already diagnosed 
> typos when leaving an expr evaluation context, when appropriate. Would it 
> still make sense to relax this assertion to diagnose any remaining ones at 
> end of TU? Are we confident that we can catch all the typos, always? I'm not 
> confident that everything will be handled, so I think we should take this 
> change as is.


There may still be uncorrected typos left behind in the outermost 
`ExprEvalContext` (created by the `Sema` constructor). In principle we should 
be able to get rid of that and parse all expressions in an evaluation context 
created for that expression (and anywhere we can't do that is a bug because 
we'll be parsing an expression without specifying whether it's 
potentially-evaluated, etc), but in practice it looks like there are still at 
least a few places where we parse expressions with no expression evaluation 
context, so cleaning up the typos in `ExprEvalContext[0]` from 
`ActOnEndOfTranslationUnitFragment` seems reasonable to me too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D64799#1591732 , @ilya-biryukov 
wrote:

> @rsmith, I'll look into emitting the typos when we pop expression evaluation 
> context, but do we expect this to cover **all** cases where `TypoExpr`s are 
> produced?
>  (conservatively assuming that the answer is "no") should we land this patch 
> and also emit at the end of TU in addition to expression evaluation context?


I was going to pose the question this way: suppose clang already diagnosed 
typos when leaving an expr evaluation context, when appropriate. Would it still 
make sense to relax this assertion to diagnose any remaining ones at end of TU? 
Are we confident that we can catch all the typos, always? I'm not confident 
that everything will be handled, so I think we should take this change as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

@rsmith, I'll look into emitting the typos when we pop expression evaluation 
context, but do we expect this to cover **all** cases where `TypoExpr`s are 
produced?
(conservatively assuming that the answer is "no") should we land this patch and 
also emit at the end of TU in addition to expression evaluation context?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D64799#1589557 , @ilya-biryukov 
wrote:

> I tried to find a good place to emit unresolved typos earlier (to make sure 
> CodeGen does not ever get `TypoExpr`), but couldn't find one.
>  Please let me know if there's some obvious place I'm missing.


The original plan when we were first designing the feature was to emit these 
diagnostics when we pop an expression evaluation context. Maybe we could try 
that? If there's a reason to defer typo correction across such contexts, it 
might probably be rare enough that we can explicitly handle that and manually 
move the delayed typos to the surrounding context.

> unless people object I would propose to land it even if it does not solve all 
> of the problems around delayed exprs.

Sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Big +1 from me, though I can't say I understand this well enough to substitute 
for Richard here.




Comment at: clang/lib/Sema/Sema.cpp:916
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.

If you like, `DEBUG_WITH_TYPE("DelayedTypos", ...)` would still let people who 
want to fix these see them. In practice, I suspect nobody actually wants to fix 
these, though :-(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 210338.
ilya-biryukov added a comment.

- Remove -disable-free from the test, it is no longer required to workaround 
the crash


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/SemaObjC/typo-correction-subscript.m


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify -disable-free
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify
 
 @class Dictionary;
 
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 
'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only -Wno-objc-root-class %s -verify -disable-free
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only -Wno-objc-root-class %s -verify
 
 @class Dictionary;
 
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the
___
cfe-commits mailing list
cfe-co

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I tried to find a good place to emit unresolved typos earlier (to make sure 
CodeGen does not ever get `TypoExpr`), but couldn't find one.
Please let me know if there's some obvious place I'm missing.

Also open to suggestions for putting assertions on whether codegen is being 
called on `TypoExpr`s.
From looking at the code, it seems that currently if the codegen eventually 
arrives at a `TypoExpr`, it will emit an error saying this expression is not 
yet supported.
There might be other failure modes (computing values of exprs containing 
`TypoExpr`, etc), but it feels like the current state should catch most of the 
potential bugs there.

This change seems to be an improvement over what we had before (clangd and 
libclang do not crash, no errors are lost) and it's very simple. So unless 
people object I would propose to land it even if it does not solve all of the 
problems around delayed exprs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: clang/test/SemaObjC/typo-correction-subscript.m:1
 // RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify -disable-free
 

I think you can even remove `-disable-free` in this test. It was there to avoid 
the assertion in `~Sema` (see D60848).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I'm in favor of this as is. We should loop in and get confirmation from @rsmith 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 210105.
ilya-biryukov added a comment.

Fix a typo (xD)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/SemaObjC/typo-correction-subscript.m


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 
'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: rnk, sammccall.
Herald added a subscriber: kadircet.
Herald added a project: clang.

Instead of asserting all typos are corrected in the sema destructor.

The sema destructor is not run in the common case of running the compiler
with the -disable-free cc1 flag (which is the default in the driver).

Having this assertion led to crashes in libclang and clangd, which are not
reproducible when running the compiler.

Asserting at the end of the TU could be an option, but finding all
missing typo correction cases is hard and having worse diagnostics instead
of a failing assertion is a better trade-off.

For more discussion on this, see:
https://lists.llvm.org/pipermail/cfe-dev/2019-July/062872.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64799

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/SemaObjC/typo-correction-subscript.m


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 
'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that point, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that point, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the
___
cfe-commits mailing list
cfe-commits@li