diff --git a/build/clang-plugin/clang-plugin.cpp b/build/clang-plugin/clang-plugin.cpp index ec9ece09689..0999789d44e 100644 --- a/build/clang-plugin/clang-plugin.cpp +++ b/build/clang-plugin/clang-plugin.cpp @@ -84,11 +84,6 @@ private: virtual void run(const MatchFinder::MatchResult &Result); }; - class ExplicitOperatorBoolChecker : public MatchFinder::MatchCallback { - public: - virtual void run(const MatchFinder::MatchResult &Result); - }; - ScopeChecker stackClassChecker; ScopeChecker globalClassChecker; NonHeapClassChecker nonheapClassChecker; @@ -97,17 +92,16 @@ private: NaNExprChecker nanExprChecker; NoAddRefReleaseOnReturnChecker noAddRefReleaseOnReturnChecker; RefCountedInsideLambdaChecker refCountedInsideLambdaChecker; - ExplicitOperatorBoolChecker explicitOperatorBoolChecker; MatchFinder astMatcher; }; namespace { -std::string getDeclarationNamespace(const Decl *decl) { +bool isInIgnoredNamespace(const Decl *decl) { const DeclContext *DC = decl->getDeclContext()->getEnclosingNamespaceContext(); const NamespaceDecl *ND = dyn_cast(DC); if (!ND) { - return ""; + return false; } while (const DeclContext *ParentDC = ND->getParent()) { @@ -118,15 +112,8 @@ std::string getDeclarationNamespace(const Decl *decl) { } const auto& name = ND->getName(); - return name; -} - -bool isInIgnoredNamespaceForImplicitCtor(const Decl *decl) { - std::string name = getDeclarationNamespace(decl); - if (name == "") { - return false; - } + // namespace std and icu are ignored for now return name == "std" || // standard C++ lib name == "__gnu_cxx" || // gnu C++ lib name == "boost" || // boost @@ -142,19 +129,7 @@ bool isInIgnoredNamespaceForImplicitCtor(const Decl *decl) { name == "testing"; // gtest } -bool isInIgnoredNamespaceForImplicitConversion(const Decl *decl) { - std::string name = getDeclarationNamespace(decl); - if (name == "") { - return false; - } - - return name == "std" || // standard C++ lib - name == "__gnu_cxx" || // gnu C++ lib - name == "google_breakpad" || // breakpad - name == "testing"; // gtest -} - -bool isIgnoredPathForImplicitCtor(const Decl *decl) { +bool isIgnoredPath(const Decl *decl) { decl = decl->getCanonicalDecl(); SourceLocation Loc = decl->getLocation(); const SourceManager &SM = decl->getASTContext().getSourceManager(); @@ -175,30 +150,9 @@ bool isIgnoredPathForImplicitCtor(const Decl *decl) { return false; } -bool isIgnoredPathForImplicitConversion(const Decl *decl) { - decl = decl->getCanonicalDecl(); - SourceLocation Loc = decl->getLocation(); - const SourceManager &SM = decl->getASTContext().getSourceManager(); - SmallString<1024> FileName = SM.getFilename(Loc); - llvm::sys::fs::make_absolute(FileName); - llvm::sys::path::reverse_iterator begin = llvm::sys::path::rbegin(FileName), - end = llvm::sys::path::rend(FileName); - for (; begin != end; ++begin) { - if (begin->compare_lower(StringRef("graphite2")) == 0) { - return true; - } - } - return false; -} - -bool isInterestingDeclForImplicitCtor(const Decl *decl) { - return !isInIgnoredNamespaceForImplicitCtor(decl) && - !isIgnoredPathForImplicitCtor(decl); -} - -bool isInterestingDeclForImplicitConversion(const Decl *decl) { - return !isInIgnoredNamespaceForImplicitConversion(decl) && - !isIgnoredPathForImplicitConversion(decl); +bool isInterestingDecl(const Decl *decl) { + return !isInIgnoredNamespace(decl) && + !isIgnoredPath(decl); } } @@ -278,7 +232,7 @@ public: } } - if (!d->isAbstract() && isInterestingDeclForImplicitCtor(d)) { + if (!d->isAbstract() && isInterestingDecl(d)) { for (CXXRecordDecl::ctor_iterator ctor = d->ctor_begin(), e = d->ctor_end(); ctor != e; ++ctor) { // Ignore non-converting ctors @@ -462,16 +416,6 @@ bool isClassRefCounted(QualType T) { return clazz ? isClassRefCounted(clazz) : RegularClass; } -template -bool IsInSystemHeader(const ASTContext &AC, const T &D) { - auto &SourceManager = AC.getSourceManager(); - auto ExpansionLoc = SourceManager.getExpansionLoc(D.getLocStart()); - if (ExpansionLoc.isInvalid()) { - return false; - } - return SourceManager.isInSystemHeader(ExpansionLoc); -} - } namespace clang { @@ -571,7 +515,12 @@ AST_MATCHER(QualType, isFloat) { /// isExpansionInSystemHeader in newer clangs, but modified in order to work /// with old clangs that we use on infra. AST_MATCHER(BinaryOperator, isInSystemHeader) { - return IsInSystemHeader(Finder->getASTContext(), Node); + 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 @@ -700,13 +649,6 @@ DiagnosticsMatcher::DiagnosticsMatcher() hasDescendant(declRefExpr(hasType(pointerType(pointee(isRefCounted())))).bind("node")) ), &refCountedInsideLambdaChecker); - - // Older clang versions such as the ones used on the infra recognize these - // conversions as 'operator _Bool', but newer clang versions recognize these - // as 'operator bool'. - astMatcher.addMatcher(methodDecl(anyOf(hasName("operator bool"), - hasName("operator _Bool"))).bind("node"), - &explicitOperatorBoolChecker); } void DiagnosticsMatcher::ScopeChecker::run( @@ -919,25 +861,6 @@ void DiagnosticsMatcher::RefCountedInsideLambdaChecker::run( Diag.Report(node->getLocStart(), noteID); } -void DiagnosticsMatcher::ExplicitOperatorBoolChecker::run( - const MatchFinder::MatchResult &Result) { - DiagnosticsEngine &Diag = Result.Context->getDiagnostics(); - unsigned errorID = Diag.getDiagnosticIDs()->getCustomDiagID( - DiagnosticIDs::Error, "bad implicit conversion operator for %0"); - unsigned noteID = Diag.getDiagnosticIDs()->getCustomDiagID( - DiagnosticIDs::Note, "consider adding the explicit keyword to %0"); - const CXXConversionDecl *method = Result.Nodes.getNodeAs("node"); - const CXXRecordDecl *clazz = method->getParent(); - - if (!method->isExplicitSpecified() && - !MozChecker::hasCustomAnnotation(method, "moz_implicit") && - !IsInSystemHeader(method->getASTContext(), *method) && - isInterestingDeclForImplicitConversion(method)) { - Diag.Report(method->getLocStart(), errorID) << clazz; - Diag.Report(method->getLocStart(), noteID) << "'operator bool'"; - } -} - class MozCheckAction : public PluginASTAction { public: ASTConsumerPtr CreateASTConsumer(CompilerInstance &CI, StringRef fileName) override { diff --git a/build/clang-plugin/tests/TestExplicitOperatorBool.cpp b/build/clang-plugin/tests/TestExplicitOperatorBool.cpp deleted file mode 100644 index bc4b43a7d06..00000000000 --- a/build/clang-plugin/tests/TestExplicitOperatorBool.cpp +++ /dev/null @@ -1,11 +0,0 @@ -#define MOZ_IMPLICIT __attribute__((annotate("moz_implicit"))) - -struct Bad { - operator bool(); // expected-error {{bad implicit conversion operator for 'Bad'}} expected-note {{consider adding the explicit keyword to 'operator bool'}} -}; -struct Good { - explicit operator bool(); -}; -struct Okay { - MOZ_IMPLICIT operator bool(); -}; diff --git a/build/clang-plugin/tests/moz.build b/build/clang-plugin/tests/moz.build index 0894046e112..5a6a26b5034 100644 --- a/build/clang-plugin/tests/moz.build +++ b/build/clang-plugin/tests/moz.build @@ -7,7 +7,6 @@ SOURCES += [ 'TestBadImplicitConversionCtor.cpp', 'TestCustomHeap.cpp', - 'TestExplicitOperatorBool.cpp', 'TestGlobalClass.cpp', 'TestMustOverride.cpp', 'TestNANTestingExpr.cpp', diff --git a/dom/bindings/CallbackObject.h b/dom/bindings/CallbackObject.h index a07279e96bc..5d8565e9c31 100644 --- a/dom/bindings/CallbackObject.h +++ b/dom/bindings/CallbackObject.h @@ -305,7 +305,7 @@ public: } // Boolean conversion operator so people can use this in boolean tests - explicit operator bool() const + operator bool() const { return GetISupports(); } diff --git a/dom/html/HTMLMediaElement.h b/dom/html/HTMLMediaElement.h index cec21baefa6..b5e6a66dd66 100644 --- a/dom/html/HTMLMediaElement.h +++ b/dom/html/HTMLMediaElement.h @@ -660,7 +660,7 @@ protected: void SetOuter(HTMLMediaElement* outer) { mOuter = outer; } void SetCanPlay(bool aCanPlay); - MOZ_IMPLICIT operator bool() const { return mValue; } + operator bool() const { return mValue; } WakeLockBoolWrapper& operator=(bool val); diff --git a/dom/media/SelfRef.h b/dom/media/SelfRef.h index 301d986fa0d..866b6de9953 100644 --- a/dom/media/SelfRef.h +++ b/dom/media/SelfRef.h @@ -35,7 +35,7 @@ public: } } - MOZ_IMPLICIT operator bool() const { return mHeld; } + operator bool() const { return mHeld; } SelfReference(const SelfReference& aOther) = delete; void operator=(const SelfReference& aOther) = delete; diff --git a/dom/plugins/base/nsNPAPIPluginInstance.cpp b/dom/plugins/base/nsNPAPIPluginInstance.cpp index efcc353682e..5f601403b16 100644 --- a/dom/plugins/base/nsNPAPIPluginInstance.cpp +++ b/dom/plugins/base/nsNPAPIPluginInstance.cpp @@ -1167,7 +1167,7 @@ public: if (plugin) mLibrary = plugin->GetLibrary(); } - explicit operator bool() { return !!mLibrary; } + operator bool() { return !!mLibrary; } PluginLibrary* operator->() { return mLibrary; } private: diff --git a/dom/plugins/ipc/PluginScriptableObjectUtils.h b/dom/plugins/ipc/PluginScriptableObjectUtils.h index bef2113c79f..432c53b7777 100644 --- a/dom/plugins/ipc/PluginScriptableObjectUtils.h +++ b/dom/plugins/ipc/PluginScriptableObjectUtils.h @@ -277,7 +277,7 @@ public: return mActor; } - explicit operator bool() + operator bool() { return !!mActor; } diff --git a/hal/linux/udev.h b/hal/linux/udev.h index 4c717560c03..9f28d95c827 100644 --- a/hal/linux/udev.h +++ b/hal/linux/udev.h @@ -63,7 +63,7 @@ class udev_lib { } } - explicit operator bool() { + operator bool() { return udev; } diff --git a/memory/replace/logalloc/replay/Replay.cpp b/memory/replace/logalloc/replay/Replay.cpp index a047d4152bf..75db8fb1e76 100644 --- a/memory/replace/logalloc/replay/Replay.cpp +++ b/memory/replace/logalloc/replay/Replay.cpp @@ -176,7 +176,7 @@ public: } /* Returns whether the buffer is empty. */ - explicit operator bool() { return mLength; } + operator bool() { return mLength; } /* Returns the memory location of the buffer. */ const char* get() { return mBuf; } diff --git a/mfbt/Atomics.h b/mfbt/Atomics.h index 33610144eda..0afee7f2df5 100644 --- a/mfbt/Atomics.h +++ b/mfbt/Atomics.h @@ -742,7 +742,7 @@ public: explicit MOZ_CONSTEXPR Atomic(bool aInit) : Base(aInit) {} // We provide boolean wrappers for the underlying AtomicBase methods. - MOZ_IMPLICIT operator bool() const + operator bool() const { return Base::Intrinsics::load(Base::mValue); } diff --git a/netwerk/base/AutoClose.h b/netwerk/base/AutoClose.h index f8521144a9d..ba82614f69d 100644 --- a/netwerk/base/AutoClose.h +++ b/netwerk/base/AutoClose.h @@ -23,7 +23,7 @@ public: Close(); } - explicit operator bool() const + operator bool() const { return mPtr; } diff --git a/netwerk/socket/nsSOCKSIOLayer.cpp b/netwerk/socket/nsSOCKSIOLayer.cpp index d07df13ecf2..50232f0f590 100644 --- a/netwerk/socket/nsSOCKSIOLayer.cpp +++ b/netwerk/socket/nsSOCKSIOLayer.cpp @@ -242,7 +242,7 @@ public: return mLength; } - explicit operator bool() { return !!mBuf; } + operator bool() { return !!mBuf; } private: template friend class Buffer; diff --git a/xpcom/build/FileLocation.h b/xpcom/build/FileLocation.h index 17f24be6f33..967718b0ee6 100644 --- a/xpcom/build/FileLocation.h +++ b/xpcom/build/FileLocation.h @@ -90,9 +90,9 @@ public: * or not. */ #if defined(MOZILLA_XPCOMRT_API) - explicit operator bool() const { return mBaseFile; } + operator bool() const { return mBaseFile; } #else - explicit operator bool() const { return mBaseFile || mBaseZip; } + operator bool() const { return mBaseFile || mBaseZip; } #endif // defined(MOZILLA_XPCOMRT_API) /** diff --git a/xpcom/glue/nsTArray.h b/xpcom/glue/nsTArray.h index 9d654d54bcb..c7f56614a8e 100644 --- a/xpcom/glue/nsTArray.h +++ b/xpcom/glue/nsTArray.h @@ -112,7 +112,7 @@ struct nsTArrayFallibleResult // Note: allows implicit conversions from and to bool MOZ_IMPLICIT nsTArrayFallibleResult(bool aResult) : mResult(aResult) {} - MOZ_IMPLICIT operator bool() { return mResult; } + operator bool() { return mResult; } private: bool mResult;