[PATCH] Warn on suspicious use of absolute value function

Richard Smith richard at metafoo.co.uk
Thu Jan 16 13:15:45 PST 2014



================
Comment at: lib/AST/Decl.cpp:3026-3045
@@ +3025,22 @@
+  default:
+    if (isExternC()) {
+      if (FnInfo->isStr("abs"))
+        return Builtin::BIabs;
+      else if (FnInfo->isStr("labs"))
+        return Builtin::BIlabs;
+      else if (FnInfo->isStr("llabs"))
+        return Builtin::BIllabs;
+      else if (FnInfo->isStr("fabs"))
+        return Builtin::BIfabs;
+      else if (FnInfo->isStr("fabsf"))
+        return Builtin::BIfabsf;
+      else if (FnInfo->isStr("fabsl"))
+        return Builtin::BIfabsl;
+      else if (FnInfo->isStr("cabs"))
+        return Builtin::BIcabs;
+      else if (FnInfo->isStr("cabsf"))
+        return Builtin::BIcabsf;
+      else if (FnInfo->isStr("cabsl"))
+        return Builtin::BIcabsl;
+    }
+    break;
----------------
This looks suspicious; I think the existing `getBuiltinID` code should handle this, except for the case where the user has used `-fno-builtin` or similar, in which case this is incorrect.

================
Comment at: lib/AST/Decl.cpp:2998-2999
@@ -2997,1 +2997,4 @@
 
+unsigned FunctionDecl::getAbsoluteValueFunctionKind() const {
+  IdentifierInfo *FnInfo = getIdentifier();
+
----------------
Please add an `isAbsoluteValueFunction(unsigned BuiltinID)` next to your check rather than adding this (rather specific) member to `FunctionDecl`.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:44
@@ +43,3 @@
+  "using %select{integer|floating point|complex}0 absolute value function "
+  "when argument is of type %select{integer|floating point|complex}1">,
+  InGroup<AbsoluteValue>;
----------------
`"[...] when argument is of type integer"` doesn't sound right. `"when argument is of %select{integer|floating point|complex}1 type %2"`, maybe?


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



More information about the cfe-commits mailing list