[PATCH] Warn on suspicious use of absolute value function

Richard Smith richard at metafoo.co.uk
Wed Feb 19 18:04:32 PST 2014


  LGTM, subject to some trivial things.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:38
@@ +37,3 @@
+def note_remove_abs : Note<
+  "remove absolute value function since unsigned values cannot be negative">;
+def warn_abs_too_small : Warning<
----------------
We're suggesting removing the call, not removing the function. This might be nicer as "remove call to %0 since [...]"

================
Comment at: lib/Sema/SemaChecking.cpp:3876-3877
@@ +3875,4 @@
+
+  // The argument and parameter are the same type.  Check if they are the right
+  // size.
+  if (ArgValueKind == ParamValueKind) {
----------------
This is misleading: they're the same kind, but not necessarily the same size.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:40
@@ +39,3 @@
+def warn_abs_too_small : Warning<
+  "absolute value function given an argument of type %0 but has parameter of "
+  "type %1 which may cause truncation of value">, InGroup<AbsoluteValue>;
----------------
Likewise, maybe say which function here instead of "absolute value function"?


http://llvm-reviews.chandlerc.com/D2224



More information about the cfe-commits mailing list