Skip to content

[BoundsSafety][NFC] Remove the unused parameter 'Decls' from 'Sema::C… #102076

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

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

rapidsna
Copy link
Contributor

@rapidsna rapidsna commented Aug 5, 2024

…heckCountedByAttrOnField'

llvm::SmallVectorImpl &Decls is a vector of declarations referred to by the argument of 'counted_by' attributes and fields. 'BuildCountAttributedArrayOrPointerType' had been made self-contained to produce the 'Decls' within itself to allow 'TreeTransform' to invoke the function without having to call 'Sema::CheckCountedByAttrOnField' again. Thus, 'Decls' produced by Sema::CheckCountedByAttrOnField is never used.

…heckCountedByAttrOnField'

llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls is a vector
of declarations referred to by the argument of 'counted_by' attributes
and frields. Since 'BuildCountAttributedArrayOrPointerType' has been
made self-contained to produce the 'Decls' within itself to allow
'TreeTransform' to invoke the functinon without having to call
'Sema::CheckCountedByAttrOnField' again.
@rapidsna rapidsna added the clang:bounds-safety Issue/PR relating to the experimental -fbounds-safety feature in Clang label Aug 5, 2024
@rapidsna rapidsna requested a review from Endilll as a code owner August 5, 2024 22:45
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 5, 2024

@llvm/pr-subscribers-clang

Author: Yeoul Na (rapidsna)

Changes

…heckCountedByAttrOnField'

llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls is a vector of declarations referred to by the argument of 'counted_by' attributes and frields. Since 'BuildCountAttributedArrayOrPointerType' has been made self-contained to produce the 'Decls' within itself to allow 'TreeTransform' to invoke the functinon without having to call 'Sema::CheckCountedByAttrOnField' again.


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

3 Files Affected:

  • (modified) clang/include/clang/Sema/Sema.h (+2-4)
  • (modified) clang/lib/Sema/SemaBoundsSafety.cpp (+2-6)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (+1-2)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2ec6367eccea0..d00b3511059f7 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -15086,10 +15086,8 @@ class Sema final : public SemaBase {
   /// `counted_by_or_null` attribute.
   ///
   /// \returns false iff semantically valid.
-  bool CheckCountedByAttrOnField(
-      FieldDecl *FD, Expr *E,
-      llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls, bool CountInBytes,
-      bool OrNull);
+  bool CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
+                                 bool OrNull);
 
   ///@}
 };
diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp
index 290c820938898..d63a2389ea11d 100644
--- a/clang/lib/Sema/SemaBoundsSafety.cpp
+++ b/clang/lib/Sema/SemaBoundsSafety.cpp
@@ -48,10 +48,8 @@ enum class CountedByInvalidPointeeTypeKind {
   VALID,
 };
 
-bool Sema::CheckCountedByAttrOnField(
-    FieldDecl *FD, Expr *E,
-    llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls, bool CountInBytes,
-    bool OrNull) {
+bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
+                                     bool OrNull) {
   // Check the context the attribute is used in
 
   unsigned Kind = getCountAttrKind(CountInBytes, OrNull);
@@ -185,8 +183,6 @@ bool Sema::CheckCountedByAttrOnField(
       return true;
     }
   }
-
-  Decls.push_back(TypeCoupledDeclRefInfo(CountFD, /*IsDref*/ false));
   return false;
 }
 
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 9011fa547638e..bcb1424825df0 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5898,8 +5898,7 @@ static void handleCountedByAttrField(Sema &S, Decl *D, const ParsedAttr &AL) {
     llvm_unreachable("unexpected counted_by family attribute");
   }
 
-  llvm::SmallVector<TypeCoupledDeclRefInfo, 1> Decls;
-  if (S.CheckCountedByAttrOnField(FD, CountExpr, Decls, CountInBytes, OrNull))
+  if (S.CheckCountedByAttrOnField(FD, CountExpr, CountInBytes, OrNull))
     return;
 
   QualType CAT = S.BuildCountAttributedArrayOrPointerType(

Copy link
Contributor

@delcypher delcypher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM other than the comment in source that needs removing.

@delcypher
Copy link
Contributor

@rapidsna There are also a few typos in the commit message

llvm::SmallVectorImpl &Decls is a vector of declarations referred to by the argument of 'counted_by' attributes and frields. 'BuildCountAttributedArrayOrPointerType' had been made self-contained to produce the 'Decls' within itself to allow 'TreeTransform' to invoke the functinon without having to call 'Sema::CheckCountedByAttrOnField' again. Thus, 'Decls' produced by Sema::CheckCountedByAttrOnField is never used.
  • frields -> fields
  • functinon -> function

Copy link
Contributor

@delcypher delcypher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please fix the commit message when squashing.

@rapidsna rapidsna merged commit 1119a08 into llvm:main Aug 6, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:bounds-safety Issue/PR relating to the experimental -fbounds-safety feature in Clang clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants