[PATCH] D146041: Fix weirdly apologetic diagnostic messages

David Spickett via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 14 07:51:27 PDT 2023


DavidSpickett added a comment.

Is there some standard for writing warning messages? For llvm that is, it would be worth looking through the getting started guides to see. I think the majority of warnings are "formal" in that sense so this seems fine.

Personally I agree with making the warnings more succinct but aside from that I don't see the need to change comments or testing scripts.

You may consider splitting this change into 2. One that only changes warnings and errors (a less controversial change) and the rest (that is up to the reviewers of each bit).



================
Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:51
 template<_Complex int> struct ComplexInt {};
-using CI = ComplexInt<1 + 3i>; // FIXME: expected-error {{sorry}}
-using CI = ComplexInt<1 + 3i>; // FIXME: expected-error {{sorry}}
----------------
Please find out what the {{sorry}} here was for. Did someone literally write it as an apology, or is it indicating some warning that should be emitted here, if the FIXME is fixed?

You could git blame this file and find the commit that introduced it, as a starting point.


================
Comment at: polly/lib/External/isl/imath/tests/test.sh:12
 if [ ! -f ../imtest ] ; then
-  echo "I can't find the imath test driver 'imtest', did you build it?"
-  echo "I can't proceed with the unit tests until you do so, sorry."
+  echo "The imath test driver 'imtest' was not found."
+  echo "It needs to be build before proceeding with the unit tests."
----------------
Given that this is a developer facing script, I think the "did you build it" is in fact quite useful here. Perhaps it could say:
"The imath test driver 'imtest' was not found. Did you build it? It is required for running the tests."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146041



More information about the cfe-commits mailing list