[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

You should also come up with a release note for the changes.



================
Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:431-432
+def err_pp_invalid_directive : Error<
+  "invalid preprocessing directive%select{|, did you mean '#%1'?}0"
+>;
+def warn_pp_invalid_directive : Warning<
----------------



================
Comment at: clang/lib/Lex/PPDirectives.cpp:488-490
+  if (LangOpts.C2x || LangOpts.CPlusPlus2b) {
+    Candidates.insert(Candidates.end(), {"elifdef", "elifndef"});
+  }
----------------



================
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;
 
----------------
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.


================
Comment at: clang/test/Preprocessor/suggest-typoed-directive.c:36
+// elfidef:   c2x-cpp2b-warning at -12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning at -12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning at -12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
----------------
ken-matsui wrote:
> Here, `#elfindef` is suggested to `#elifdef`, not `#elifndef`, but I believe it will help many users to realize that they have typo'd `#elifndef`, or maybe they might have meant actually `#elifdef`.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726