Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][analyzer][NFC] Prepare for clang-format #82921

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

steakhal
Copy link
Contributor

This is the followup PR for #82599 to fix the cases where clang-format would regress the flow of the code.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:analysis labels Feb 25, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 25, 2024

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Balazs Benics (steakhal)

Changes

This is the followup PR for #82599 to fix the cases where clang-format would regress the flow of the code.


Full diff: https://github.com/llvm/llvm-project/pull/82921.diff

13 Files Affected:

  • (modified) clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h (+8-4)
  • (modified) clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h (+2)
  • (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (+4-5)
  • (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (+1-1)
  • (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h (+1-1)
  • (modified) clang/lib/Analysis/PathDiagnostic.cpp (+1-2)
  • (modified) clang/lib/Analysis/ThreadSafetyCommon.cpp (+2)
  • (modified) clang/lib/Analysis/ThreadSafetyTIL.cpp (+2)
  • (modified) clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp (+4-1)
  • (modified) clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp (+9-11)
  • (modified) clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp (+26-24)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp (+2)
  • (modified) clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp (+2)
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
index 65dd66ee093fe4..7d26d7439b8830 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
@@ -128,6 +128,7 @@ enum TIL_CastOpcode : unsigned char {
   CAST_objToPtr
 };
 
+// clang-format off
 const TIL_Opcode       COP_Min  = COP_Future;
 const TIL_Opcode       COP_Max  = COP_Branch;
 const TIL_UnaryOpcode  UOP_Min  = UOP_Minus;
@@ -136,6 +137,7 @@ const TIL_BinaryOpcode BOP_Min  = BOP_Add;
 const TIL_BinaryOpcode BOP_Max  = BOP_LogicOr;
 const TIL_CastOpcode   CAST_Min = CAST_none;
 const TIL_CastOpcode   CAST_Max = CAST_toInt;
+// clang-format on
 
 /// Return the name of a unary opcode.
 StringRef getUnaryOpcodeString(TIL_UnaryOpcode Op);
@@ -188,12 +190,14 @@ struct ValueType {
 
 inline ValueType::SizeType ValueType::getSizeType(unsigned nbytes) {
   switch (nbytes) {
-    case 1: return ST_8;
-    case 2: return ST_16;
-    case 4: return ST_32;
-    case 8: return ST_64;
+    // clang-format off
+    case  1: return ST_8;
+    case  2: return ST_16;
+    case  4: return ST_32;
+    case  8: return ST_64;
     case 16: return ST_128;
     default: return ST_0;
+    // clang-format on
   }
 }
 
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
index 6fc55130655a29..e4545106915b25 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
@@ -440,6 +440,7 @@ class PrettyPrinter {
 
   // Return the precedence of a given node, for use in pretty printing.
   unsigned precedence(const SExpr *E) {
+    // clang-format off
     switch (E->opcode()) {
       case COP_Future:     return Prec_Atom;
       case COP_Undefined:  return Prec_Atom;
@@ -479,6 +480,7 @@ class PrettyPrinter {
       case COP_IfThenElse: return Prec_Other;
       case COP_Let:        return Prec_Decl;
     }
+    // clang-format on
     return Prec_MAX;
   }
 
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index ed5c4adb5e3d56..8c28f4c3a61217 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -393,8 +393,8 @@ class ExprEngine {
   ProgramStateRef processAssume(ProgramStateRef state, SVal cond,
                                 bool assumption);
 
-  /// processRegionChanges - Called by ProgramStateManager whenever a change is made
-  ///  to the store. Used to update checkers that track region values.
+  /// It's called by ProgramStateManager whenever a change is made to the store.
+  /// Used to update checkers that track region values.
   ProgramStateRef
   processRegionChanges(ProgramStateRef state,
                        const InvalidatedSymbols *invalidated,
@@ -587,9 +587,8 @@ class ExprEngine {
                                 ExplodedNode *Pred,
                                 ExplodedNodeSet &Dst);
 
-  /// evalEagerlyAssumeBinOpBifurcation - Given the nodes in 'Src', eagerly assume symbolic
-  ///  expressions of the form 'x != 0' and generate new nodes (stored in Dst)
-  ///  with those assumptions.
+  /// Given the nodes in \p Src, eagerly assume symbolic expressions of the form
+  /// `x != 0` and generate new nodes (stored in \p Dst) with those assumptions.
   void evalEagerlyAssumeBinOpBifurcation(ExplodedNodeSet &Dst, ExplodedNodeSet &Src,
                          const Expr *Ex);
 
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index 151d3e57c1cb81..615d9e849da53c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -526,7 +526,7 @@ class TypedRegion : public SubRegion {
   }
 };
 
-/// TypedValueRegion - An abstract class representing regions having a typed value.
+/// An abstract class representing regions having a typed value.
 class TypedValueRegion : public TypedRegion {
   void anchor() override;
 
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
index 15bec97c5be8fc..ded0c551b7f257 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
@@ -1,4 +1,4 @@
-//ProgramStateTrait.h - Partial implementations of ProgramStateTrait -*- C++ -*-
+// ProgramStateTrait.h - Partial implementations of ProgramStateTrait
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
diff --git a/clang/lib/Analysis/PathDiagnostic.cpp b/clang/lib/Analysis/PathDiagnostic.cpp
index 79f337a91ec8fa..6210eef6b44d29 100644
--- a/clang/lib/Analysis/PathDiagnostic.cpp
+++ b/clang/lib/Analysis/PathDiagnostic.cpp
@@ -788,8 +788,7 @@ PathDiagnosticRange
           }
           break;
         }
-          // FIXME: Provide better range information for different
-          //  terminators.
+        // FIXME: Provide better range information for different terminators.
         case Stmt::IfStmtClass:
         case Stmt::WhileStmtClass:
         case Stmt::DoStmtClass:
diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp
index 2fe0f85897c3bc..cbb76d92e6ff4f 100644
--- a/clang/lib/Analysis/ThreadSafetyCommon.cpp
+++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp
@@ -560,6 +560,7 @@ til::SExpr *SExprBuilder::translateBinAssign(til::TIL_BinaryOpcode Op,
 
 til::SExpr *SExprBuilder::translateBinaryOperator(const BinaryOperator *BO,
                                                   CallingContext *Ctx) {
+  // clang-format off
   switch (BO->getOpcode()) {
   case BO_PtrMemD:
   case BO_PtrMemI:
@@ -601,6 +602,7 @@ til::SExpr *SExprBuilder::translateBinaryOperator(const BinaryOperator *BO,
     // The clang CFG should have already processed both sides.
     return translate(BO->getRHS(), Ctx);
   }
+  // clang-format on
   return new (Arena) til::Undefined(BO);
 }
 
diff --git a/clang/lib/Analysis/ThreadSafetyTIL.cpp b/clang/lib/Analysis/ThreadSafetyTIL.cpp
index 652f953d2a6db5..368bab52b41872 100644
--- a/clang/lib/Analysis/ThreadSafetyTIL.cpp
+++ b/clang/lib/Analysis/ThreadSafetyTIL.cpp
@@ -26,6 +26,7 @@ StringRef til::getUnaryOpcodeString(TIL_UnaryOpcode Op) {
 }
 
 StringRef til::getBinaryOpcodeString(TIL_BinaryOpcode Op) {
+  // clang-format off
   switch (Op) {
     case BOP_Mul:      return "*";
     case BOP_Div:      return "/";
@@ -45,6 +46,7 @@ StringRef til::getBinaryOpcodeString(TIL_BinaryOpcode Op) {
     case BOP_LogicAnd: return "&&";
     case BOP_LogicOr:  return "||";
   }
+  // clang-format on
   return {};
 }
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
index c72a97cc01e914..d8b8a3ae2a646a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
@@ -387,6 +387,7 @@ static std::optional<uint64_t> GetCFNumberSize(ASTContext &Ctx, uint64_t i) {
 
   QualType T;
 
+  // clang-format off
   switch (i) {
     case kCFNumberCharType:     T = Ctx.CharTy;     break;
     case kCFNumberShortType:    T = Ctx.ShortTy;    break;
@@ -402,6 +403,7 @@ static std::optional<uint64_t> GetCFNumberSize(ASTContext &Ctx, uint64_t i) {
     default:
       return std::nullopt;
   }
+  // clang-format on
 
   return Ctx.getTypeSize(T);
 }
@@ -535,7 +537,8 @@ void CFNumberChecker::checkPreStmt(const CallExpr *CE,
 }
 
 //===----------------------------------------------------------------------===//
-// CFRetain/CFRelease/CFMakeCollectable/CFAutorelease checking for null arguments.
+// checking for null arguments:
+//   CFRetain/CFRelease/CFMakeCollectable/CFAutorelease
 //===----------------------------------------------------------------------===//
 
 namespace {
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
index 888724f7ea3b29..9dcc8bce96d3a2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -11,18 +11,19 @@
 //    of bytes to copy.
 //
 //===----------------------------------------------------------------------===//
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TypeTraits.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
@@ -228,11 +229,11 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
       llvm::raw_svector_ostream os(S);
       os << "Potential buffer overflow. ";
       if (!DstName.empty()) {
-        os << "Replace with 'sizeof(" << DstName << ") "
-              "- strlen(" << DstName <<") - 1'";
+        os << formatv("Replace with 'sizeof({0}) - strlen({0}) - 1'", DstName);
         os << " or u";
-      } else
+      } else {
         os << "U";
+      }
       os << "se a safer 'strlcat' API";
 
       BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument",
@@ -251,13 +252,10 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
 
       SmallString<256> S;
       llvm::raw_svector_ostream os(S);
-      os << "The third argument allows to potentially copy more bytes than it should. ";
-      os << "Replace with the value ";
-      if (!DstName.empty())
-          os << "sizeof(" << DstName << ")";
-      else
-          os << "sizeof(<destination buffer>)";
-      os << " or lower";
+      os << "The third argument allows to potentially copy more bytes ";
+      os << "than it should. Replace with the value ";
+      os << formatv("sizeof({0}) or lower",
+                    !DstName.empty() ? DstName : "<destination buffer>");
 
       BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument",
               "C String API", os.str(), Loc,
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
index 3e5e2b9139149d..6de0f611fda78f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
@@ -33,30 +33,32 @@ namespace ento {
 /// checking.
 ///
 /// \sa CheckerContext
-class CheckerDocumentation : public Checker< check::PreStmt<ReturnStmt>,
-                                       check::PostStmt<DeclStmt>,
-                                       check::PreObjCMessage,
-                                       check::PostObjCMessage,
-                                       check::ObjCMessageNil,
-                                       check::PreCall,
-                                       check::PostCall,
-                                       check::BranchCondition,
-                                       check::NewAllocator,
-                                       check::Location,
-                                       check::Bind,
-                                       check::DeadSymbols,
-                                       check::BeginFunction,
-                                       check::EndFunction,
-                                       check::EndAnalysis,
-                                       check::EndOfTranslationUnit,
-                                       eval::Call,
-                                       eval::Assume,
-                                       check::LiveSymbols,
-                                       check::RegionChanges,
-                                       check::PointerEscape,
-                                       check::ConstPointerEscape,
-                                       check::Event<ImplicitNullDerefEvent>,
-                                       check::ASTDecl<FunctionDecl> > {
+class CheckerDocumentation : public Checker<                           //
+                                 check::PreStmt<ReturnStmt>,           //
+                                 check::PostStmt<DeclStmt>,            //
+                                 check::PreObjCMessage,                //
+                                 check::PostObjCMessage,               //
+                                 check::ObjCMessageNil,                //
+                                 check::PreCall,                       //
+                                 check::PostCall,                      //
+                                 check::BranchCondition,               //
+                                 check::NewAllocator,                  //
+                                 check::Location,                      //
+                                 check::Bind,                          //
+                                 check::DeadSymbols,                   //
+                                 check::BeginFunction,                 //
+                                 check::EndFunction,                   //
+                                 check::EndAnalysis,                   //
+                                 check::EndOfTranslationUnit,          //
+                                 eval::Call,                           //
+                                 eval::Assume,                         //
+                                 check::LiveSymbols,                   //
+                                 check::RegionChanges,                 //
+                                 check::PointerEscape,                 //
+                                 check::ConstPointerEscape,            //
+                                 check::Event<ImplicitNullDerefEvent>, //
+                                 check::ASTDecl<FunctionDecl>          //
+                                 > {
 public:
   /// Pre-visit the Statement.
   ///
diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
index 514f53b4804f50..4ef43e5f4d2fbc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
@@ -53,6 +53,7 @@ class ObjCAutoreleaseWriteChecker : public Checker<check::ASTCodeBody> {
                         AnalysisManager &AM,
                         BugReporter &BR) const;
 private:
+  // clang-format off
   std::vector<std::string> SelectorsWithAutoreleasingPool = {
       // Common to NSArray,  NSSet, NSOrderedSet
       "enumerateObjectsUsingBlock:",
@@ -92,6 +93,7 @@ class ObjCAutoreleaseWriteChecker : public Checker<check::ASTCodeBody> {
       "indexInRange:options:passingTest:",
       "indexesInRange:options:passingTest:"
   };
+  // clang-format on
 
   std::vector<std::string> FunctionsWithAutoreleasingPool = {
       "dispatch_async", "dispatch_group_async", "dispatch_barrier_async"};
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index c3fc56ac30ee9f..a7ffee7605c7bb 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -112,6 +112,7 @@ void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
 
     assert (B->isCompoundAssignmentOp());
 
+    // clang-format off
     switch (Op) {
       default:
         llvm_unreachable("Invalid opcode for compound assignment.");
@@ -126,6 +127,7 @@ void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
       case BO_XorAssign: Op = BO_Xor; break;
       case BO_OrAssign:  Op = BO_Or;  break;
     }
+    // clang-format on
 
     // Perform a load (the LHS).  This performs the checks for
     // null dereferences, and so on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:analysis clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants