diff --git a/build/clang-plugin/clang-plugin.cpp b/build/clang-plugin/clang-plugin.cpp index ed3a0ebb3c6b..75e4931146a8 100644 --- a/build/clang-plugin/clang-plugin.cpp +++ b/build/clang-plugin/clang-plugin.cpp @@ -69,17 +69,11 @@ private: virtual void run(const MatchFinder::MatchResult &Result); }; - class NaNExprChecker : public MatchFinder::MatchCallback { - public: - virtual void run(const MatchFinder::MatchResult &Result); - }; - ScopeChecker stackClassChecker; ScopeChecker globalClassChecker; NonHeapClassChecker nonheapClassChecker; ArithmeticArgChecker arithmeticArgChecker; TrivialCtorDtorChecker trivialCtorDtorChecker; - NaNExprChecker nanExprChecker; MatchFinder astMatcher; }; @@ -422,39 +416,6 @@ AST_MATCHER(UnaryOperator, unaryArithmeticOperator) { opcode == UO_Minus || opcode == UO_Not; } - -/// This matcher will match == and != binary operators. -AST_MATCHER(BinaryOperator, binaryEqualityOperator) { - BinaryOperatorKind opcode = Node.getOpcode(); - return opcode == BO_EQ || opcode == BO_NE; -} - -/// This matcher will match floating point types. -AST_MATCHER(QualType, isFloat) { - return Node->isRealFloatingType(); -} - -/// This matcher will match locations in system headers. This is copied from -/// isExpansionInSystemHeader in newer clangs. -AST_POLYMORPHIC_MATCHER(isInSystemHeader, - AST_POLYMORPHIC_SUPPORTED_TYPES_3(Decl, Stmt, TypeLoc)) { - auto &SourceManager = Finder->getASTContext().getSourceManager(); - auto ExpansionLoc = SourceManager.getExpansionLoc(Node.getLocStart()); - if (ExpansionLoc.isInvalid()) { - return false; - } - return SourceManager.isInSystemHeader(ExpansionLoc); -} - -/// This matcher will match locations in SkScalar.h. This header contains a -/// known NaN-testing expression which we would like to whitelist. -AST_MATCHER(BinaryOperator, isInSkScalarDotH) { - SourceLocation Loc = Node.getOperatorLoc(); - auto &SourceManager = Finder->getASTContext().getSourceManager(); - SmallString<1024> FileName = SourceManager.getFilename(Loc); - return llvm::sys::path::rbegin(FileName)->equals("SkScalar.h"); -} - } } @@ -538,13 +499,6 @@ DiagnosticsMatcher::DiagnosticsMatcher() astMatcher.addMatcher(recordDecl(hasTrivialCtorDtor()).bind("node"), &trivialCtorDtorChecker); - - astMatcher.addMatcher(binaryOperator(allOf(binaryEqualityOperator(), - hasLHS(has(declRefExpr(hasType(qualType((isFloat())))).bind("lhs"))), - hasRHS(has(declRefExpr(hasType(qualType((isFloat())))).bind("rhs"))), - unless(anyOf(isInSystemHeader(), isInSkScalarDotH())) - )).bind("node"), - &nanExprChecker); } void DiagnosticsMatcher::ScopeChecker::run( @@ -694,42 +648,6 @@ void DiagnosticsMatcher::TrivialCtorDtorChecker::run( Diag.Report(node->getLocStart(), errorID) << node; } -void DiagnosticsMatcher::NaNExprChecker::run( - const MatchFinder::MatchResult &Result) { - if (!Result.Context->getLangOpts().CPlusPlus) { - // mozilla::IsNaN is not usable in C, so there is no point in issuing these warnings. - return; - } - - DiagnosticsEngine &Diag = Result.Context->getDiagnostics(); - unsigned errorID = Diag.getDiagnosticIDs()->getCustomDiagID( - DiagnosticIDs::Error, "comparing a floating point value to itself for NaN checking can lead to incorrect results"); - unsigned noteID = Diag.getDiagnosticIDs()->getCustomDiagID( - DiagnosticIDs::Note, "consider using mozilla::IsNaN instead"); - const BinaryOperator *expr = Result.Nodes.getNodeAs("node"); - const DeclRefExpr *lhs = Result.Nodes.getNodeAs("lhs"); - const DeclRefExpr *rhs = Result.Nodes.getNodeAs("rhs"); - const ImplicitCastExpr *lhsExpr = dyn_cast(expr->getLHS()); - const ImplicitCastExpr *rhsExpr = dyn_cast(expr->getRHS()); - // The AST subtree that we are looking for will look like this: - // -BinaryOperator ==/!= - // |-ImplicitCastExpr LValueToRValue - // | |-DeclRefExpr - // |-ImplicitCastExpr LValueToRValue - // |-DeclRefExpr - // The check below ensures that we are dealing with the correct AST subtree shape, and - // also that both of the found DeclRefExpr's point to the same declaration. - if (lhs->getFoundDecl() == rhs->getFoundDecl() && - lhsExpr && rhsExpr && - std::distance(lhsExpr->child_begin(), lhsExpr->child_end()) == 1 && - std::distance(rhsExpr->child_begin(), rhsExpr->child_end()) == 1 && - *lhsExpr->child_begin() == lhs && - *rhsExpr->child_begin() == rhs) { - Diag.Report(expr->getLocStart(), errorID); - Diag.Report(expr->getLocStart(), noteID); - } -} - class MozCheckAction : public PluginASTAction { public: ASTConsumerPtr CreateASTConsumer(CompilerInstance &CI, StringRef fileName) override { diff --git a/build/clang-plugin/tests/Makefile.in b/build/clang-plugin/tests/Makefile.in index 7d67f8be67fe..bc4d435081db 100644 --- a/build/clang-plugin/tests/Makefile.in +++ b/build/clang-plugin/tests/Makefile.in @@ -4,7 +4,6 @@ # Build without any warning flags, and with clang verify flag for a # syntax-only build (no codegen). -OS_CFLAGS := $(filter-out -W%,$(OS_CFLAGS)) -fsyntax-only -Xclang -verify OS_CXXFLAGS := $(filter-out -W%,$(OS_CXXFLAGS)) -fsyntax-only -Xclang -verify include $(topsrcdir)/config/rules.mk diff --git a/build/clang-plugin/tests/TestNANTestingExpr.cpp b/build/clang-plugin/tests/TestNANTestingExpr.cpp deleted file mode 100644 index 943577d4ae85..000000000000 --- a/build/clang-plugin/tests/TestNANTestingExpr.cpp +++ /dev/null @@ -1,16 +0,0 @@ -void test(bool x); -void foo() { - float f, f2; - typedef double mydouble; - mydouble d; - double d2; - test(f == f); // expected-error{{comparing a floating point value to itself for NaN checking can lead to incorrect results}} expected-note{{consider using mozilla::IsNaN instead}} - test(d == d); // expected-error{{comparing a floating point value to itself for NaN checking can lead to incorrect results}} expected-note{{consider using mozilla::IsNaN instead}} - test(f != f); // expected-error{{comparing a floating point value to itself for NaN checking can lead to incorrect results}} expected-note{{consider using mozilla::IsNaN instead}} - test(d != d); // expected-error{{comparing a floating point value to itself for NaN checking can lead to incorrect results}} expected-note{{consider using mozilla::IsNaN instead}} - test(f != d); - test(d == (d - f)); - test(f == f2); - test(d == d2); - test(d + 1 == d); -} diff --git a/build/clang-plugin/tests/TestNANTestingExprC.c b/build/clang-plugin/tests/TestNANTestingExprC.c deleted file mode 100644 index ab2fead22a42..000000000000 --- a/build/clang-plugin/tests/TestNANTestingExprC.c +++ /dev/null @@ -1,17 +0,0 @@ -/* expected-no-diagnostics */ -void test(int x); -void foo() { - float f, f2; - typedef double mydouble; - mydouble d; - double d2; - test(f == f); - test(d == d); - test(f != f); - test(d != d); - test(f != d); - test(d == (d - f)); - test(f == f2); - test(d == d2); - test(d + 1 == d); -} diff --git a/build/clang-plugin/tests/moz.build b/build/clang-plugin/tests/moz.build index 0d7f35b6a6d3..41d8dc0a682c 100644 --- a/build/clang-plugin/tests/moz.build +++ b/build/clang-plugin/tests/moz.build @@ -9,8 +9,6 @@ SOURCES += [ 'TestCustomHeap.cpp', 'TestGlobalClass.cpp', 'TestMustOverride.cpp', - 'TestNANTestingExpr.cpp', - 'TestNANTestingExprC.c', 'TestNoArithmeticExprInArgument.cpp', 'TestNonHeapClass.cpp', 'TestStackClass.cpp',