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

Wed Oct 9 02:47:27 PDT 2019

ilya-biryukov added a comment.

In D64799#1688293 <https://reviews.llvm.org/D64799#1688293>, @dgoldman wrote:

> In D64799#1605514 <https://reviews.llvm.org/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