[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Lex/PPDirectives.cpp:1257
   // If we reached here, the preprocessing token is not valid!
-  Diag(Result, diag::err_pp_invalid_directive);
+  Diag(Result, diag::err_pp_invalid_directive) << 0;
 
----------------
ken-matsui wrote:
> aaron.ballman wrote:
> > I think we should be attempting to suggest a typo for the error case as well e.g.,
> > ```
> > #fi WHATEVER
> > #endif
> > ```
> > we currently give no suggestion for that typo, just the error. However, this may require a fair amount of changes because of the various edge cases where we give better diagnostics than "unknown directive". e.g.,
> > ```
> > #if WHATEVER // error: unterminated conditional directive
> > #endfi // no diagnostic
> > ```
> > so if it looks like covering error cases is going to be involved, I'm fine doing it in a follow-up if you'd prefer.
> The former can be implemented easily, but the latter seems not easy.
> So what about doing the latter in another patch?
I'm fine doing the error cases entirely in another patch. The current approach here is a problem because it issues a warning and an error for the same line of code. So I'd go back to the way the code was before, and in a follow-up you can handle errors.

I think one approach to consider for that is having an argument to `SuggestTypoedDirective()` as to whether to err or warn so that we can reuse the same logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726