Backed out changeset 517de066cc6f (bug 1615826) for linting failures at CustomAttributes.cpp on a CLOSED TREE

build/clang-plugin/CanRunScriptChecker.cpp

@@ -55,10 +55,12 @@
 void CanRunScriptChecker::registerMatchers(MatchFinder *AstMatcher) {
   auto Refcounted = qualType(hasDeclaration(cxxRecordDecl(isRefCounted())));
   auto StackSmartPtr =
-      ignoreTrivials(declRefExpr(to(varDecl(hasAutomaticStorageDuration())),
+    ignoreTrivials(
+      declRefExpr(to(varDecl(hasAutomaticStorageDuration())),
                   hasType(isSmartPtrToRefCounted())));
   auto ConstMemberOfThisSmartPtr =
-      memberExpr(hasType(isSmartPtrToRefCounted()), hasType(isConstQualified()),
+    memberExpr(hasType(isSmartPtrToRefCounted()),
+               hasType(isConstQualified()),
                hasObjectExpression(cxxThisExpr()));
   // A smartptr can be known-live for three reasons:
   // 1) It's declared on the stack.
@@ -68,14 +70,15 @@ void CanRunScriptChecker::registerMatchers(MatchFinder *AstMatcher) {
   // 3) It's an immediate temporary being constructed at the point where the
   //    call is happening.
   auto KnownLiveSmartPtr = anyOf(
-      StackSmartPtr, ConstMemberOfThisSmartPtr,
+    StackSmartPtr,
+    ConstMemberOfThisSmartPtr,
     ignoreTrivials(cxxConstructExpr(hasType(isSmartPtrToRefCounted()))));
 
   auto MozKnownLiveCall =
     ignoreTrivials(callExpr(callee(functionDecl(hasName("MOZ_KnownLive")))));
 
-  // Params of the calling function are presumed live, because it itself should
+  // Params of the calling function are presumed live, because it itself should be
-  // be MOZ_CAN_RUN_SCRIPT.  Note that this is subject to
+  // MOZ_CAN_RUN_SCRIPT.  Note that this is subject to
   // https://bugzilla.mozilla.org/show_bug.cgi?id=1537656 a the moment.
   auto KnownLiveParam = anyOf(
       // "this" is OK
@@ -105,12 +108,15 @@ void CanRunScriptChecker::registerMatchers(MatchFinder *AstMatcher) {
       // return non-live objects (e.g. consider "live_pointer->foo()" as an
       // example).  For purposes of this analysis we are assuming the method
       // calls on smart ptrs all just return the pointer inside,
-      cxxMemberCallExpr(
+      cxxMemberCallExpr(on(
-          on(allOf(hasType(isSmartPtrToRefCounted()), KnownLiveBase))),
+          allOf(hasType(isSmartPtrToRefCounted()),
+                KnownLiveBase))),
       // operator* or operator-> on a thing that is already known to be live.
-      cxxOperatorCallExpr(anyOf(hasOverloadedOperatorName("*"),
+      cxxOperatorCallExpr(
+          anyOf(hasOverloadedOperatorName("*"),
                 hasOverloadedOperatorName("->")),
-                          hasAnyArgument(KnownLiveBase), argumentCountIs(1)),
+          hasAnyArgument(KnownLiveBase),
+          argumentCountIs(1)),
       // A dereference on a thing that is known to be live.  This is _not_
       // caught by the "operator* or operator->" clause above, because
       // cxxOperatorCallExpr() only catches cases when a class defines
@@ -133,27 +139,37 @@ void CanRunScriptChecker::registerMatchers(MatchFinder *AstMatcher) {
       // does not guarantee liveness; in fact the callee could modify those
       // things!  In practice they would be the wrong type anyway, though, so
       // it's hard to add a test for this.
-      unaryOperator(hasOperatorName("&"),
+      unaryOperator(
-                    hasUnaryOperand(allOf(anyOf(hasType(references(Refcounted)),
+          hasOperatorName("&"),
+          hasUnaryOperand(allOf(
+              anyOf(
+                  hasType(references(Refcounted)),
                   hasType(Refcounted)),
-                                          ignoreTrivials(KnownLiveBase)))));
+              ignoreTrivials(KnownLiveBase))))
+      );
 
   auto KnownLive = anyOf(
       // Anything above, of course.
       KnownLiveSimple,
       // Conditional operators where both arms are live.
-      conditionalOperator(hasFalseExpression(ignoreTrivials(KnownLiveSimple)),
+      conditionalOperator(
+          hasFalseExpression(ignoreTrivials(KnownLiveSimple)),
           hasTrueExpression(ignoreTrivials(KnownLiveSimple)))
       // We're not handling cases like a dereference of a conditional operator,
       // mostly because handling a dereference in general is so ugly.  I
       // _really_ wish I could just write a recursive matcher here easily.
       );
 
-  auto InvalidArg = ignoreTrivialsConditional(
+  auto InvalidArg =
+      ignoreTrivialsConditional(
         // We want to consider things if there is anything refcounted involved,
         // including in any of the trivials that we otherwise strip off.
-      anyOf(hasType(Refcounted), hasType(pointsTo(Refcounted)),
+        anyOf(
-            hasType(references(Refcounted)), hasType(isSmartPtrToRefCounted())),
+          hasType(Refcounted),
+          hasType(pointsTo(Refcounted)),
+          hasType(references(Refcounted)),
+          hasType(isSmartPtrToRefCounted())
+        ),
         // We want to find any expression,
         expr(
           // which is not known live,
@@ -162,7 +178,8 @@ void CanRunScriptChecker::registerMatchers(MatchFinder *AstMatcher) {
           // always safe,
           unless(cxxDefaultArgExpr(isNullDefaultArg())),
           // and which is not a literal nullptr,
-          unless(cxxNullPtrLiteralExpr()), expr().bind("invalidArg")));
+          unless(cxxNullPtrLiteralExpr()),
+          expr().bind("invalidArg")));
 
   // A matcher which will mark the first invalid argument it finds invalid, but
   // will always match, even if it finds no invalid arguments, so it doesn't
@@ -186,7 +203,11 @@ void CanRunScriptChecker::registerMatchers(MatchFinder *AstMatcher) {
                   // which optionally has an invalid arg,
                   OptionalInvalidExplicitArg,
                   // or which optionally has an invalid this argument,
-                  anyOf(on(InvalidArg), anything()), expr().bind("callExpr")),
+                  anyOf(
+                    on(InvalidArg),
+                    anything()
+                  ),
+                  expr().bind("callExpr")),
               // or a regular call expression,
               callExpr(
                   // which optionally has an invalid arg.
@@ -219,7 +240,8 @@ class FuncSetCallback : public MatchFinder::MatchCallback {
 public:
   FuncSetCallback(CanRunScriptChecker& Checker,
                   std::unordered_set<const FunctionDecl *> &FuncSet)
-      : CanRunScriptFuncs(FuncSet), Checker(Checker) {}
+      : CanRunScriptFuncs(FuncSet),
+        Checker(Checker) {}
 
   void run(const MatchFinder::MatchResult &Result) override;
 
@@ -261,7 +283,8 @@ void FuncSetCallback::checkOverriddenMethods(const CXXMethodDecl *Method) {
       Checker.diag(Method->getLocation(), ErrorNonCanRunScriptOverridden,
                    DiagnosticIDs::Error);
       Checker.diag(OverriddenMethod->getLocation(),
-                   NoteNonCanRunScriptOverridden, DiagnosticIDs::Note);
+                   NoteNonCanRunScriptOverridden,
+                   DiagnosticIDs::Note);
     }
   }
 }
@@ -277,7 +300,9 @@ void CanRunScriptChecker::buildFuncSet(ASTContext *Context) {
   Finder.addMatcher(
       functionDecl(hasCanRunScriptAnnotation()).bind("canRunScriptFunction"),
       &Callback);
-  Finder.addMatcher(lambdaExpr().bind("lambda"), &Callback);
+  Finder.addMatcher(
+      lambdaExpr().bind("lambda"),
+      &Callback);
   // We start the analysis, given the ASTContext our main checker is in.
   Finder.matchAST(*Context);
 }
@@ -358,9 +383,11 @@ void CanRunScriptChecker::check(const MatchFinder::MatchResult &Result) {
   // If we have an invalid argument in the call, we emit the diagnostic to
   // signal it.
   if (InvalidArg) {
-    const std::string invalidArgText = Lexer::getSourceText(
+    const std::string invalidArgText =
+        Lexer::getSourceText(
             CharSourceRange::getTokenRange(InvalidArg->getSourceRange()),
-        Result.Context->getSourceManager(), Result.Context->getLangOpts());
+            Result.Context->getSourceManager(),
+            Result.Context->getLangOpts());
     diag(InvalidArg->getExprLoc(), ErrorInvalidArg, DiagnosticIDs::Error)
         << InvalidArg->getSourceRange() << invalidArgText;
   }

build/clang-plugin/CustomAttributes.cpp

@@ -2,10 +2,10 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include <algorithm>
 #include "CustomAttributes.h"
 #include "plugin.h"
 #include "clang/Frontend/FrontendPluginRegistry.h"
-#include <algorithm>
 
 /* Having annotations in the AST unexpectedly impacts codegen.
  * Ideally, we'd avoid having annotations at all, by using an API such as
@@ -28,7 +28,8 @@ using namespace llvm;
 
 static DenseMap<const Decl*, CustomAttributesSet> AttributesCache;
 
-static CustomAttributesSet CacheAttributes(const Decl *D) {
+static CustomAttributesSet CacheAttributes(const Decl* D)
+{
   CustomAttributesSet attrs = {};
   for (auto Attr : D->specific_attrs<AnnotateAttr>()) {
     auto annotation = Attr->getAnnotation();
@@ -45,15 +46,17 @@ static CustomAttributesSet CacheAttributes(const Decl *D) {
   return attrs;
 }
 
-static void Report(const Decl *D, const char *message) {
+static void Report(const Decl* D, const char* message)
+{
   ASTContext& Context = D->getASTContext();
   DiagnosticsEngine& Diag = Context.getDiagnostics();
-  unsigned ID =
+  unsigned ID = Diag.getDiagnosticIDs()->getCustomDiagID(
-      Diag.getDiagnosticIDs()->getCustomDiagID(DiagnosticIDs::Warning, message);
+    DiagnosticIDs::Warning, message);
   Diag.Report(D->getBeginLoc(), ID);
 }
 
-CustomAttributesSet GetAttributes(const Decl *D) {
+CustomAttributesSet GetAttributes(const Decl* D)
+{
   CustomAttributesSet attrs = {};
   if (D->hasAttr<AnnotateAttr>()) {
     Report(D, "Declaration has unhandled annotations.");
@@ -67,22 +70,22 @@ CustomAttributesSet GetAttributes(const Decl *D) {
   return attrs;
 }
 
-bool hasCustomAttribute(const clang::Decl *D, CustomAttributes A) {
+bool hasCustomAttribute(const clang::Decl* D, CustomAttributes A)
+{
   CustomAttributesSet attrs = GetAttributes(D);
   switch (A) {
-#define ATTR(a)                                                                \
+#define ATTR(a) case a: return attrs.has_ ## a;
-  case a:                                                                      \
-    return attrs.has_##a;
 #include "CustomAttributes.inc"
 #undef ATTR
   }
   return false;
 }
 
-class CustomAttributesMatcher
+class CustomAttributesMatcher : public ast_matchers::MatchFinder::MatchCallback {
-    : public ast_matchers::MatchFinder::MatchCallback {
 public:
-  virtual void run(const ast_matchers::MatchFinder::MatchResult &Result) final {
+  virtual void
+  run(const ast_matchers::MatchFinder::MatchResult &Result) final
+  {
     if (auto D = Result.Nodes.getNodeAs<Decl>("decl")) {
       CacheAttributes(D);
     } else if (auto L = Result.Nodes.getNodeAs<LambdaExpr>("lambda")) {
@@ -98,8 +101,7 @@ public:
                                    StringRef FileName) override {
     auto& Context = CI.getASTContext();
     auto AstMatcher = new (Context.Allocate<MatchFinder>()) MatchFinder();
-    auto Matcher = new (Context.Allocate<CustomAttributesMatcher>())
+    auto Matcher = new (Context.Allocate<CustomAttributesMatcher>()) CustomAttributesMatcher();
-        CustomAttributesMatcher();
     AstMatcher->addMatcher(decl().bind("decl"), Matcher);
     AstMatcher->addMatcher(lambdaExpr().bind("lambda"), Matcher);
     return AstMatcher->newASTConsumer();
@@ -110,8 +112,11 @@ public:
     return true;
   }
 
-  ActionType getActionType() override { return AddBeforeMainAction; }
+  ActionType getActionType() override {
+    return AddBeforeMainAction;
+  }
 };
 
-static FrontendPluginRegistry::Add<CustomAttributesAction>
+static FrontendPluginRegistry::Add<CustomAttributesAction> X(
-    X("moz-custom-attributes", "prepare custom attributes for moz-check");
+  "moz-custom-attributes",
+  "prepare custom attributes for moz-check");

build/clang-plugin/CustomAttributes.h

@@ -20,14 +20,16 @@ struct CustomAttributesSet {
 #undef ATTR
 };
 
-template <CustomAttributes A> bool hasCustomAttribute(const clang::Decl *D) {
+template<CustomAttributes A>
+bool hasCustomAttribute(const clang::Decl* D) {
   return false;
 }
 
 extern CustomAttributesSet GetAttributes(const clang::Decl* D);
 
 #define ATTR(name) \
-  template <> inline bool hasCustomAttribute<name>(const clang::Decl *D) {     \
+  template<> \
+  inline bool hasCustomAttribute<name>(const clang::Decl* D) { \
     return GetAttributes(D).has_ ## name; \
   }
 #include "CustomAttributes.inc"

build/clang-plugin/CustomMatchers.h

@@ -224,17 +224,18 @@ AST_MATCHER_P(Expr, ignoreTrivials, internal::Matcher<Expr>, InnerMatcher) {
   return InnerMatcher.matches(*IgnoreTrivials(&Node), Finder, Builder);
 }
 
-// Takes two matchers: the first one is a condition; the second is a matcher to
+// Takes two matchers: the first one is a condition; the second is a matcher to be
-// be applied once we are done unwrapping trivials.  While the condition does
+// applied once we are done unwrapping trivials.  While the condition does not match
-// not match and we're looking at a trivial, will keep unwrapping the trivial
+// and we're looking at a trivial, will keep unwrapping the trivial and trying again.
-// and trying again. Once the condition matches, we will go ahead and unwrap all
+// Once the condition matches, we will go ahead and unwrap all trivials and apply the
-// trivials and apply the inner matcher to the result.
+// inner matcher to the result.
 //
-// The expected use here is if we want to condition a match on some typecheck
+// The expected use here is if we want to condition a match on some typecheck but
-// but apply the match to only non-trivials, because there are trivials (e.g.
+// apply the match to only non-trivials, because there are trivials (e.g. casts) that
-// casts) that can change types.
+// can change types.
-AST_MATCHER_P2(Expr, ignoreTrivialsConditional, internal::Matcher<Expr>,
+AST_MATCHER_P2(Expr, ignoreTrivialsConditional,
-               Condition, internal::Matcher<Expr>, InnerMatcher) {
+               internal::Matcher<Expr>, Condition,
+               internal::Matcher<Expr>, InnerMatcher) {
   const Expr *node = &Node;
   while (true) {
     if (Condition.matches(*node, Finder, Builder)) {
@@ -317,8 +318,7 @@ AST_MATCHER(QualType, isSmartPtrToRefCounted) {
 }
 
 AST_MATCHER(ClassTemplateSpecializationDecl, isSmartPtrToRefCountedDecl) {
-  auto *D = dyn_cast_or_null<CXXRecordDecl>(
+  auto *D = dyn_cast_or_null<CXXRecordDecl>(Node.getSpecializedTemplate()->getTemplatedDecl());
-      Node.getSpecializedTemplate()->getTemplatedDecl());
   if (!D) {
     return false;
   }
@@ -328,6 +328,7 @@ AST_MATCHER(ClassTemplateSpecializationDecl, isSmartPtrToRefCountedDecl) {
   return D && hasCustomAttribute<moz_is_smartptr_to_refcounted>(D);
 }
 
+
 AST_MATCHER(CXXRecordDecl, hasBaseClasses) {
   const CXXRecordDecl *Decl = Node.getCanonicalDecl();
 
@@ -347,8 +348,7 @@ AST_MATCHER(CXXMethodDecl, isNonVirtual) {
 
 AST_MATCHER(FunctionDecl, isMozMustReturnFromCaller) {
   const FunctionDecl *Decl = Node.getCanonicalDecl();
-  return Decl &&
+  return Decl && hasCustomAttribute<moz_must_return_from_caller_if_this_is_arg>(Decl);
-         hasCustomAttribute<moz_must_return_from_caller_if_this_is_arg>(Decl);
 }
 
 /// This matcher will select default args which have nullptr as the value.

build/clang-plugin/DanglingOnTemporaryChecker.cpp

@@ -42,12 +42,12 @@ void DanglingOnTemporaryChecker::registerMatchers(MatchFinder *AstMatcher) {
   // Main checker //
   //////////////////
 
-  auto hasParentCall = hasParent(
+  auto hasParentCall = hasParent(expr(
-      expr(anyOf(cxxOperatorCallExpr(
+      anyOf(cxxOperatorCallExpr(
-                     // If we're in a lamda, we may have an operator call
+                // If we're in a lamda, we may have an operator call expression
-                     // expression ancestor in the AST, but the temporary we're
+                // ancestor in the AST, but the temporary we're matching
-                     // matching against is not going to have the same lifetime
+                // against is not going to have the same lifetime as the
-                     // as the constructor call.
+                // constructor call.
                 unless(has(expr(ignoreTrivials(lambdaExpr())))),
                 expr().bind("parentOperatorCallExpr")),
             callExpr(
@@ -58,10 +58,10 @@ void DanglingOnTemporaryChecker::registerMatchers(MatchFinder *AstMatcher) {
                 unless(has(expr(ignoreTrivials(lambdaExpr())))),
                 expr().bind("parentCallExpr")),
             objcMessageExpr(
-                     // If we're in a lamda, we may have an objc message
+                // If we're in a lamda, we may have an objc message expression
-                     // expression ancestor in the AST, but the temporary we're
+                // ancestor in the AST, but the temporary we're matching
-                     // matching against is not going to have the same lifetime
+                // against is not going to have the same lifetime as the
-                     // as the function call.
+                // function call.
                 unless(has(expr(ignoreTrivials(lambdaExpr())))),
                 expr().bind("parentObjCMessageExpr")),
             cxxConstructExpr(

build/clang-plugin/ExplicitOperatorBoolChecker.cpp

@@ -21,7 +21,8 @@ void ExplicitOperatorBoolChecker::check(
       Result.Nodes.getNodeAs<CXXConversionDecl>("node");
   const CXXRecordDecl *Clazz = Method->getParent();
 
-  if (!Method->isExplicit() && !hasCustomAttribute<moz_implicit>(Method) &&
+  if (!Method->isExplicit() &&
+      !hasCustomAttribute<moz_implicit>(Method) &&
       !ASTIsInSystemHeader(Method->getASTContext(), *Method) &&
       isInterestingDeclForImplicitConversion(Method)) {
     diag(Method->getBeginLoc(), "bad implicit conversion operator for %0",

build/clang-plugin/FopenUsageChecker.h

@@ -9,7 +9,8 @@
 
 class FopenUsageChecker : public BaseCheck {
 public:
-  FopenUsageChecker(StringRef CheckName, ContextType *Context = nullptr)
+  FopenUsageChecker(StringRef CheckName,
+                     ContextType *Context = nullptr)
     : BaseCheck(CheckName, Context) {}
   void registerMatchers(MatchFinder* AstMatcher) override;
   void check(const MatchFinder::MatchResult &Result) override;

build/clang-plugin/KungFuDeathGripChecker.cpp

@@ -6,7 +6,8 @@
 #include "CustomMatchers.h"
 
 void KungFuDeathGripChecker::registerMatchers(MatchFinder *AstMatcher) {
-  AstMatcher->addMatcher(varDecl(allOf(hasType(isRefPtr()), hasLocalStorage(),
+  AstMatcher->addMatcher(varDecl(allOf(hasType(isRefPtr()),
+                                       hasLocalStorage(),
                                        hasInitializer(anything())))
                              .bind("decl"),
                          this);

build/clang-plugin/NoUsingNamespaceMozillaJavaChecker.cpp

@@ -5,8 +5,7 @@
 #include "NoUsingNamespaceMozillaJavaChecker.h"
 #include "CustomMatchers.h"
 
-void NoUsingNamespaceMozillaJavaChecker::registerMatchers(
+void NoUsingNamespaceMozillaJavaChecker::registerMatchers(MatchFinder *AstMatcher) {
-    MatchFinder *AstMatcher) {
   AstMatcher->addMatcher(
       usingDirectiveDecl(isUsingNamespaceMozillaJava()).bind("directive"),
       this);
@@ -18,7 +17,9 @@ void NoUsingNamespaceMozillaJavaChecker::check(
       Result.Nodes.getNodeAs<UsingDirectiveDecl>("directive");
   const NamespaceDecl *Namespace = Directive->getNominatedNamespace();
 
-  diag(Directive->getUsingLoc(), "using namespace %0 is forbidden",
+  diag(Directive->getUsingLoc(),
+       "using namespace %0 is forbidden",
        DiagnosticIDs::Error)
       << Namespace->getQualifiedNameAsString();
 }
+

build/clang-plugin/NoUsingNamespaceMozillaJavaChecker.h

@@ -17,3 +17,4 @@ public:
 };
 
 #endif
+

build/clang-plugin/ParamTraitsEnumChecker.cpp

@@ -7,8 +7,7 @@
 
 void ParamTraitsEnumChecker::registerMatchers(MatchFinder* AstMatcher) {
   AstMatcher->addMatcher(
-      classTemplateSpecializationDecl(hasName("ParamTraits")).bind("decl"),
+    classTemplateSpecializationDecl(hasName("ParamTraits")).bind("decl"), this);
-      this);
 }
 
 void ParamTraitsEnumChecker::check(const MatchFinder::MatchResult &Result) {

build/clang-plugin/ParamTraitsEnumChecker.h

@@ -9,7 +9,8 @@
 
 class ParamTraitsEnumChecker : public BaseCheck {
 public:
-  ParamTraitsEnumChecker(StringRef CheckName, ContextType *Context = nullptr)
+  ParamTraitsEnumChecker(StringRef CheckName,
+                         ContextType *Context = nullptr)
     : BaseCheck(CheckName, Context) {}
   void registerMatchers(MatchFinder* AstMatcher) override;
   void check(const MatchFinder::MatchResult &Result) override;

build/clang-plugin/ScopeChecker.cpp

@@ -10,9 +10,8 @@ void ScopeChecker::registerMatchers(MatchFinder *AstMatcher) {
   AstMatcher->addMatcher(cxxNewExpr().bind("node"), this);
   AstMatcher->addMatcher(
       materializeTemporaryExpr(
-          unless(hasDescendant(cxxConstructExpr(allowsTemporary()))))
+          unless(hasDescendant(cxxConstructExpr(allowsTemporary())))
-          .bind("node"),
+      ).bind("node"), this);
-      this);
   AstMatcher->addMatcher(
       callExpr(callee(functionDecl(heapAllocator()))).bind("node"), this);
 }
@@ -40,7 +39,8 @@ void ScopeChecker::check(const MatchFinder::MatchResult &Result) {
   QualType T;
   bool IsStaticLocal = false;
 
-  if (const ParmVarDecl *D = Result.Nodes.getNodeAs<ParmVarDecl>("node")) {
+  if (const ParmVarDecl *D =
+          Result.Nodes.getNodeAs<ParmVarDecl>("node")) {
     if (D->hasUnparsedDefaultArg() || D->hasUninstantiatedDefaultArg()) {
       return;
     }

build/clang-plugin/TrivialDtorChecker.cpp

@@ -6,7 +6,8 @@
 #include "CustomMatchers.h"
 
 void TrivialDtorChecker::registerMatchers(MatchFinder *AstMatcher) {
-  AstMatcher->addMatcher(cxxRecordDecl(hasTrivialDtor()).bind("node"), this);
+  AstMatcher->addMatcher(cxxRecordDecl(hasTrivialDtor()).bind("node"),
+                         this);
 }
 
 void TrivialDtorChecker::check(const MatchFinder::MatchResult &Result) {