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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
