ken-matsui 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; ---------------- aaron.ballman wrote: > 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. OK, reverting :+1: I'll work on it. Thank you. 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