r288732 - Warn on unsigned zero in call to std::max
Richard Trieu via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 5 15:41:47 PST 2016
Author: rtrieu
Date: Mon Dec 5 17:41:46 2016
New Revision: 288732
URL: http://llvm.org/viewvc/llvm-project?rev=288732&view=rev
Log:
Warn on unsigned zero in call to std::max
New default warning that triggers when an unsigned zero is used in a call to
std::max. For unsigned values, zero is the minimum value, so any call to
std::max is always equal to the other value. A common pattern was to take
the max of zero and the difference of two unsigned values, not taking into
account that unsigned values wrap around below zero. This warning also emits
a note with a fixit hint to remove the zero and call to std::max.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaChecking.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=288732&r1=288731&r2=288732&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Dec 5 17:41:46 2016
@@ -256,6 +256,7 @@ def LoopAnalysis : DiagGroup<"loop-analy
def MalformedWarningCheck : DiagGroup<"malformed-warning-check">;
def Main : DiagGroup<"main">;
def MainReturnType : DiagGroup<"main-return-type">;
+def MaxUnsignedZero : DiagGroup<"max-unsigned-zero">;
def MissingBraces : DiagGroup<"missing-braces">;
def MissingDeclarations: DiagGroup<"missing-declarations">;
def : DiagGroup<"missing-format-attribute">;
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=288732&r1=288731&r2=288732&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Dec 5 17:41:46 2016
@@ -53,6 +53,14 @@ def warn_pointer_abs : Warning<
"taking the absolute value of %select{pointer|function|array}0 type %1 is suspicious">,
InGroup<AbsoluteValue>;
+def warn_max_unsigned_zero : Warning<
+ "taking the max of "
+ "%select{a value and unsigned zero|unsigned zero and a value}0 "
+ "is always equal to the other value">,
+ InGroup<MaxUnsignedZero>;
+def note_remove_max_call : Note<
+ "remove call to max function and unsigned zero argument">;
+
def warn_infinite_recursive_function : Warning<
"all paths through this function will call itself">,
InGroup<InfiniteRecursion>, DefaultIgnore;
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=288732&r1=288731&r2=288732&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Dec 5 17:41:46 2016
@@ -9802,6 +9802,9 @@ private:
const FunctionDecl *FDecl,
IdentifierInfo *FnInfo);
+ void CheckMinZero(const CallExpr *Call, const FunctionDecl *FDecl,
+ IdentifierInfo *FnInfo);
+
void CheckMemaccessArguments(const CallExpr *Call,
unsigned BId,
IdentifierInfo *FnName);
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=288732&r1=288731&r2=288732&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Dec 5 17:41:46 2016
@@ -2518,6 +2518,8 @@ bool Sema::CheckFunctionCall(FunctionDec
return false;
CheckAbsoluteValueFunction(TheCall, FDecl, FnInfo);
+ CheckMinZero(TheCall, FDecl, FnInfo);
+
if (getLangOpts().ObjC1)
DiagnoseCStringFormatDirectiveInCFAPI(*this, FDecl, Args, NumArgs);
@@ -6767,6 +6769,91 @@ void Sema::CheckAbsoluteValueFunction(co
Call->getCallee()->getSourceRange(), NewAbsKind, ArgType);
}
+//===--- CHECK: Warn on use of std::max and unsigned zero. r---------------===//
+static bool IsFunctionStdMax(const FunctionDecl *FDecl) {
+ if (!FDecl)
+ return false;
+
+ if (!FDecl->getIdentifier() || !FDecl->getIdentifier()->isStr("max"))
+ return false;
+
+ const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(FDecl->getDeclContext());
+
+ while (ND && ND->isInlineNamespace()) {
+ ND = dyn_cast<NamespaceDecl>(ND->getDeclContext());
+ }
+
+ if (!ND || !ND->getIdentifier() || !ND->getIdentifier()->isStr("std"))
+ return false;
+
+ if (!isa<TranslationUnitDecl>(ND->getDeclContext()))
+ return false;
+
+ return true;
+}
+
+void Sema::CheckMinZero(const CallExpr *Call, const FunctionDecl *FDecl,
+ IdentifierInfo *FnInfo) {
+ if (!Call || !FDecl) return;
+
+ // Ignore template specializations and macros.
+ if (!ActiveTemplateInstantiations.empty()) return;
+ if (Call->getExprLoc().isMacroID()) return;
+
+ // Only care about the one template argument, two function parameter std::max
+ if (Call->getNumArgs() != 2) return;
+ if (!IsFunctionStdMax(FDecl)) return;
+ const auto * ArgList = FDecl->getTemplateSpecializationArgs();
+ if (!ArgList) return;
+ if (ArgList->size() != 1) return;
+
+ // Check that template type argument is unsigned integer.
+ const auto& TA = ArgList->get(0);
+ if (TA.getKind() != TemplateArgument::Type) return;
+ QualType ArgType = TA.getAsType();
+ if (!ArgType->isUnsignedIntegerType()) return;
+
+ // See if either argument is a literal zero.
+ auto IsLiteralZeroArg = [](const Expr* E) -> bool {
+ const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E);
+ if (!MTE) return false;
+ const auto *Num = dyn_cast<IntegerLiteral>(MTE->GetTemporaryExpr());
+ if (!Num) return false;
+ if (Num->getValue() != 0) return false;
+ return true;
+ };
+
+ const Expr *FirstArg = Call->getArg(0);
+ const Expr *SecondArg = Call->getArg(1);
+ const bool IsFirstArgZero = IsLiteralZeroArg(FirstArg);
+ const bool IsSecondArgZero = IsLiteralZeroArg(SecondArg);
+
+ // Only warn when exactly one argument is zero.
+ if (IsFirstArgZero == IsSecondArgZero) return;
+
+ SourceRange FirstRange = FirstArg->getSourceRange();
+ SourceRange SecondRange = SecondArg->getSourceRange();
+
+ SourceRange ZeroRange = IsFirstArgZero ? FirstRange : SecondRange;
+
+ Diag(Call->getExprLoc(), diag::warn_max_unsigned_zero)
+ << IsFirstArgZero << Call->getCallee()->getSourceRange() << ZeroRange;
+
+ // Deduce what parts to remove so that "std::max(0u, foo)" becomes "(foo)".
+ SourceRange RemovalRange;
+ if (IsFirstArgZero) {
+ RemovalRange = SourceRange(FirstRange.getBegin(),
+ SecondRange.getBegin().getLocWithOffset(-1));
+ } else {
+ RemovalRange = SourceRange(getLocForEndOfToken(FirstRange.getEnd()),
+ SecondRange.getEnd());
+ }
+
+ Diag(Call->getExprLoc(), diag::note_remove_max_call)
+ << FixItHint::CreateRemoval(Call->getCallee()->getSourceRange())
+ << FixItHint::CreateRemoval(RemovalRange);
+}
+
//===--- CHECK: Standard memory functions ---------------------------------===//
/// \brief Takes the expression passed to the size_t parameter of functions
More information about the cfe-commits
mailing list