tahonermann added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:116-117 "source file is not valid UTF-8">; +def warn_invalid_utf8_in_comment : Warning< + "invalid UTF-8 in comment">, InGroup<DiagGroup<"invalid-utf8">>, DefaultIgnore; def err_character_not_allowed : Error< ---------------- aaron.ballman wrote: > What would it take to enable the diagnostic by default? We don't typically > add off-by-default diagnostics because there's evidence that not many folks > enable them. > > Alternatively, would this make sense as a pedantic warning? It's not really > an extension for us to accept this stuff, but it seems like we can still > pedantically diagnose the code? I agree with enabling this by default in at least some warning mode. Aaron's suggestion of making it a pedantic warning seems reasonable to me. ================ Comment at: clang/lib/Lex/Lexer.cpp:2405-2406 // Skip over characters in the fast loop. - while (C != 0 && // Potentially EOF. - C != '\n' && C != '\r') // Newline or DOS-style newline. + // Warn on invalid UTF-8 if the corresponding warning is enabled, emitting a + // diagnostic only once per sequence that cannot be decoded. + while ((!WarnOnInvalidUtf8 || isASCII(C)) && C != 0 && // Potentially EOF. ---------------- I think it would be helpful to include a link to [[ http://unicode.org/review/pr-121.html | Unicode PR issue 121 ]] here and note that the diagnostic follows policy option 1. Likewise for handling of '//' comment styles below. Alternatively, provide the link and a note in the release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128059/new/ https://reviews.llvm.org/D128059 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits