[Lldb-commits] [PATCH] D146041: Fix weirdly apologetic diagnostic messages

[Richard Smith - zygoloid via Phabricator via lldb-commits]

rsmith added a comment.

I have no strong opinions on the merits of this patch in either direction; I think the "sorry"s in the Sema diagnostics for regrettable non-conformance make Clang marginally friendlier, but they do nothing to actually help people who encounter the diagnostic.

FWIW, the relevant guidance on Clang diagnostics is https://clang.llvm.org/docs/InternalsManual.html#the-format-string, and that would override the LLVM coding guidelines' rules in places where they conflict (though in this case there's no conflict).



================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:338
 def err_file_too_large : Error<
-  "sorry, unsupported: file '%0' is too large for Clang to process">;
+  "unsupported: file '%0' is too large for Clang to process">;
 def err_include_too_large : Error<
----------------
I think we could drop the "unsupported: " here too. (We're allowed to have implementation limits; this isn't a conformance issue.)


================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:341
+  "this include generates a translation unit too large for"
   " Clang to process.">, DefaultFatal;
 def err_unsupported_bom : Error<"%0 byte order mark detected in '%1', but "
----------------
Please remove the trailing period while you're fixing the style of this diagnostic.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9669
   "by aggregate initialization using default member initializer "
   "is not supported; lifetime of %select{temporary|backing array}0 "
   "will end at the end of the full-expression">, InGroup<Dangling>;
----------------
While we're fixing style: in this file we have "is not yet supported", "is not supported yet", and "is not supported". We should pick one and use it consistently; "is not supported yet" would be my preference.


================
Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:11-12
 template<float> struct Float {};
-using F1 = Float<1.0f>; // FIXME expected-error {{sorry}}
-using F1 = Float<2.0f / 2>; // FIXME expected-error {{sorry}}
+using F1 = Float<1.0f>; // FIXME expected-error
+using F1 = Float<2.0f / 2>; // FIXME expected-error
 
----------------
You should retain a `{{...}}` here with some text from the diagnostic for `-verify` to match against. Likewise below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146041