Revert "[clang] Catch missing format attributes" (#98617)

AaronBallman · web-flow · commit 9d1017204a25 · 2024-07-12T06:58:43.000-04:00
Reverts #70024 It broke several post-commit bots: https://lab.llvm.org/buildbot/#/builders/193/builds/896 https://lab.llvm.org/buildbot/#/builders/23/builds/925 https://lab.llvm.org/buildbot/#/builders/13/builds/686 and others
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -716,9 +716,6 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses integer constant expressions that are folded to a constant value as an extension in more circumstances. Fixes #GH59863
 
-- Clang now diagnoses missing format attributes for non-template functions and
-  class/struct/union members. Fixes #GH60718
-
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -525,6 +525,7 @@ 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">;
 def MissingIncludeDirs : DiagGroup<"missing-include-dirs">;
 def MissingNoreturn : DiagGroup<"missing-noreturn">;
 def MultiChar : DiagGroup<"multichar">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1031,9 +1031,6 @@ def err_opencl_invalid_param : Error<
 def err_opencl_invalid_return : Error<
   "declaring function return value of type %0 is not allowed %select{; did you forget * ?|}1">;
 def warn_enum_value_overflow : Warning<"overflow in enumeration value">;
-def warn_missing_format_attribute : Warning<
-  "diagnostic behavior may be improved by adding the %0 format attribute to the declaration of %1">,
-  InGroup<DiagGroup<"missing-format-attribute">>, DefaultIgnore;
 def warn_pragma_options_align_reset_failed : Warning<
   "#pragma options align=reset failed: %0">,
   InGroup<IgnoredPragmas>;
diff --git a/clang/include/clang/Sema/Attr.h b/clang/include/clang/Sema/Attr.h
@@ -123,13 +123,6 @@ inline bool isInstanceMethod(const Decl *D) {
   return false;
 }
 
-inline bool checkIfMethodHasImplicitObjectParameter(const Decl *D) {
-  if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(D))
-    return MethodDecl->isInstance() &&
-           !MethodDecl->hasCXXExplicitFunctionObjectParameter();
-  return false;
-}
-
 /// Diagnose mutually exclusive attributes when present on a given
 /// declaration. Returns true if diagnosed.
 template <typename AttrTy>
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
@@ -4594,10 +4594,6 @@ class Sema final : public SemaBase {
 
   enum class RetainOwnershipKind { NS, CF, OS };
 
-  void DiagnoseMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl);
-  std::vector<FormatAttr *>
-  GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl);
-
   UuidAttr *mergeUuidAttr(Decl *D, const AttributeCommonInfo &CI,
                           StringRef UuidAsWritten, MSGuidDecl *GuidDecl);
 
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
@@ -15934,8 +15934,6 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
         }
       }
 
-      DiagnoseMissingFormatAttributes(Body, FD);
-
       // We might not have found a prototype because we didn't wish to warn on
       // the lack of a missing prototype. Try again without the checks for
       // whether we want to warn on the missing prototype.
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3508,7 +3508,7 @@ static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   // In C++ the implicit 'this' function parameter also counts, and they are
   // counted from one.
-  bool HasImplicitThisParam = checkIfMethodHasImplicitObjectParameter(D);
+  bool HasImplicitThisParam = isInstanceMethod(D);
   unsigned NumArgs = getFunctionOrMethodNumParams(D) + HasImplicitThisParam;
 
   IdentifierInfo *II = AL.getArgAsIdent(0)->Ident;
@@ -3621,7 +3621,7 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     return;
   }
 
-  bool HasImplicitThisParam = checkIfMethodHasImplicitObjectParameter(D);
+  bool HasImplicitThisParam = isInstanceMethod(D);
   int32_t NumArgs = getFunctionOrMethodNumParams(D);
 
   FunctionDecl *FD = D->getAsFunction();
@@ -5320,221 +5320,6 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   D->addAttr(::new (S.Context) PreferredTypeAttr(S.Context, AL, ParmTSI));
 }
 
-// This function is called only if function call is not inside template body.
-// TODO: Add call for function calls inside template body.
-// Emit warnings if parent function misses format attributes.
-void Sema::DiagnoseMissingFormatAttributes(Stmt *Body,
-                                           const FunctionDecl *FDecl) {
-  assert(FDecl);
-
-  // If there are no function body, exit.
-  if (!Body)
-    return;
-
-  // Get missing format attributes
-  std::vector<FormatAttr *> MissingFormatAttributes =
-      GetMissingFormatAttributes(Body, FDecl);
-  if (MissingFormatAttributes.empty())
-    return;
-
-  // Check if there are more than one format type found. In that case do not
-  // emit diagnostic.
-  const FormatAttr *FirstAttr = MissingFormatAttributes[0];
-  if (llvm::any_of(MissingFormatAttributes, [&](const FormatAttr *Attr) {
-        return FirstAttr->getType() != Attr->getType();
-      }))
-    return;
-
-  for (const FormatAttr *FA : MissingFormatAttributes) {
-    // If format index and first-to-check argument index are negative, it means
-    // that this attribute is only saved for multiple format types checking.
-    if (FA->getFormatIdx() < 0 || FA->getFirstArg() < 0)
-      continue;
-
-    // Emit diagnostic
-    SourceLocation Loc = FDecl->getLocation();
-    Diag(Loc, diag::warn_missing_format_attribute)
-        << FA->getType() << FDecl
-        << FixItHint::CreateInsertion(Loc,
-                                      (llvm::Twine("__attribute__((format(") +
-                                       FA->getType()->getName() + ", " +
-                                       llvm::Twine(FA->getFormatIdx()) + ", " +
-                                       llvm::Twine(FA->getFirstArg()) + ")))")
-                                          .str());
-  }
-}
-
-// Returns vector of format attributes. There are no two attributes with same
-// arguments in returning vector. There can be attributes that effectivelly only
-// store information about format type.
-std::vector<FormatAttr *>
-Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
-  unsigned int FunctionFormatArgumentIndexOffset =
-      checkIfMethodHasImplicitObjectParameter(FDecl) ? 2 : 1;
-
-  std::vector<FormatAttr *> MissingAttributes;
-
-  // Iterate over body statements.
-  for (auto *Child : Body->children()) {
-    // If child statement is compound statement, recursively get missing
-    // attributes.
-    if (dyn_cast_or_null<CompoundStmt>(Child)) {
-      std::vector<FormatAttr *> CompoundStmtMissingAttributes =
-          GetMissingFormatAttributes(Child, FDecl);
-
-      // If there are already missing attributes with same arguments, do not add
-      // duplicates.
-      for (FormatAttr *FA : CompoundStmtMissingAttributes) {
-        if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
-              return FA->getType() == Attr->getType() &&
-                     FA->getFormatIdx() == Attr->getFormatIdx() &&
-                     FA->getFirstArg() == Attr->getFirstArg();
-            }))
-          MissingAttributes.push_back(FA);
-      }
-
-      continue;
-    }
-
-    ValueStmt *VS = dyn_cast_or_null<ValueStmt>(Child);
-    if (!VS)
-      continue;
-    Expr *TheExpr = VS->getExprStmt();
-    if (!TheExpr)
-      continue;
-    CallExpr *TheCall = dyn_cast_or_null<CallExpr>(TheExpr);
-    if (!TheCall)
-      continue;
-    const FunctionDecl *ChildFunction =
-        dyn_cast_or_null<FunctionDecl>(TheCall->getCalleeDecl());
-    if (!ChildFunction)
-      continue;
-
-    Expr **Args = TheCall->getArgs();
-    unsigned int NumArgs = TheCall->getNumArgs();
-
-    // If child expression is function, check if it is format function.
-    // If it is, check if parent function misses format attributes.
-
-    // If child function is format function and format arguments are not
-    // relevant to emit diagnostic, save only information about format type
-    // (format index and first-to-check argument index are set to -1).
-    // Information about format type is later used to determine if there are
-    // more than one format type found.
-
-    unsigned int ChildFunctionFormatArgumentIndexOffset =
-        checkIfMethodHasImplicitObjectParameter(ChildFunction) ? 2 : 1;
-
-    // Check if function has format attribute with forwarded format string.
-    IdentifierInfo *AttrType;
-    const ParmVarDecl *FormatArg;
-    if (!llvm::any_of(ChildFunction->specific_attrs<FormatAttr>(),
-                      [&](const FormatAttr *Attr) {
-                        AttrType = Attr->getType();
-
-                        int OffsetFormatIndex =
-                            Attr->getFormatIdx() -
-                            ChildFunctionFormatArgumentIndexOffset;
-                        if (OffsetFormatIndex < 0 ||
-                            (unsigned)OffsetFormatIndex >= NumArgs)
-                          return false;
-
-                        const auto *FormatArgExpr = dyn_cast<DeclRefExpr>(
-                            Args[OffsetFormatIndex]->IgnoreParenCasts());
-                        if (!FormatArgExpr)
-                          return false;
-
-                        FormatArg = dyn_cast_or_null<ParmVarDecl>(
-                            FormatArgExpr->getReferencedDeclOfCallee());
-                        if (!FormatArg)
-                          return false;
-
-                        return true;
-                      })) {
-      MissingAttributes.push_back(
-          FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
-      continue;
-    }
-
-    // Do not add in a vector format attributes whose type is different than
-    // parent function attribute type.
-    if (llvm::any_of(FDecl->specific_attrs<FormatAttr>(),
-                     [&](const FormatAttr *FunctionAttr) {
-                       return AttrType != FunctionAttr->getType();
-                     }))
-      continue;
-
-    // Check if format string argument is parent function parameter.
-    unsigned int StringIndex = 0;
-    if (!llvm::any_of(FDecl->parameters(), [&](const ParmVarDecl *Param) {
-          if (Param != FormatArg)
-            return false;
-
-          StringIndex = Param->getFunctionScopeIndex() +
-                        FunctionFormatArgumentIndexOffset;
-
-          return true;
-        })) {
-      MissingAttributes.push_back(
-          FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
-      continue;
-    }
-
-    unsigned NumOfParentFunctionParams = FDecl->getNumParams();
-
-    // Compare parent and calling function format attribute arguments (archetype
-    // and format string).
-    if (llvm::any_of(
-            FDecl->specific_attrs<FormatAttr>(), [&](const FormatAttr *Attr) {
-              if (Attr->getType() != AttrType)
-                return false;
-              int OffsetFormatIndex =
-                  Attr->getFormatIdx() - FunctionFormatArgumentIndexOffset;
-
-              if (OffsetFormatIndex < 0 ||
-                  (unsigned)OffsetFormatIndex >= NumOfParentFunctionParams)
-                return false;
-
-              if (FDecl->parameters()[OffsetFormatIndex] != FormatArg)
-                return false;
-
-              return true;
-            })) {
-      MissingAttributes.push_back(
-          FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
-      continue;
-    }
-
-    // Get first argument index
-    unsigned FirstToCheck = [&]() -> unsigned {
-      if (!FDecl->isVariadic())
-        return 0;
-      const auto *FirstToCheckArg =
-          dyn_cast<DeclRefExpr>(Args[NumArgs - 1]->IgnoreParenCasts());
-      if (!FirstToCheckArg)
-        return 0;
-
-      if (FirstToCheckArg->getType().getCanonicalType() !=
-          Context.getBuiltinVaListType().getCanonicalType())
-        return 0;
-      return NumOfParentFunctionParams + FunctionFormatArgumentIndexOffset;
-    }();
-
-    // If there are already attributes which arguments matches arguments
-    // detected in this iteration, do not add new attribute as it would be
-    // duplicate.
-    if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
-          return Attr->getType() == AttrType &&
-                 Attr->getFormatIdx() == StringIndex &&
-                 Attr->getFirstArg() == FirstToCheck;
-        }))
-      MissingAttributes.push_back(FormatAttr::CreateImplicit(
-          getASTContext(), AttrType, StringIndex, FirstToCheck));
-  }
-
-  return MissingAttributes;
-}
-
 //===----------------------------------------------------------------------===//
 // Microsoft specific attribute handlers.
 //===----------------------------------------------------------------------===//
diff --git a/clang/test/Sema/attr-format-missing.c b/clang/test/Sema/attr-format-missing.c
diff --git a/clang/test/Sema/attr-format-missing.cpp b/clang/test/Sema/attr-format-missing.cpp

Original file line number	Diff line number	Diff line change
`@@ -15934,8 +15934,6 @@ Decl Sema::ActOnFinishFunctionBody(Decl dcl, Stmt *Body,`
`15934`	`15934`	`}`
`15935`	`15935`	`}`
`15936`	`15936`
`15937`		`- DiagnoseMissingFormatAttributes(Body, FD);`
`15938`		`-`
`15939`	`15937`	`// We might not have found a prototype because we didn't wish to warn on`
`15940`	`15938`	`// the lack of a missing prototype. Try again without the checks for`
`15941`	`15939`	`// whether we want to warn on the missing prototype.`