Skip to content

[clang-tidy][NFCI] Simplify bugprone-sizeof-expression #93024

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 3 commits into from
May 22, 2024

Conversation

NagyDonat
Copy link
Contributor

This commit eliminates a redundant matcher subexpression from the implementation of the "sizeof-pointer-to-aggregate" part of the clang-tidy check bugprone-sizeof-expression.

I'm fairly certain that anything that was previously matched by the deleted matcher StructAddrOfExpr is also covered by the more general PointerToStructExpr (which remains in the same anyOf).

This commit is made to "prepare the ground" for a followup change that would merge the functionality of the Clang Static Analyzer checker alpha.core.SizeofPtr into this clang-tidy check.

This commit eliminates a redundant matcher subexpression from the
implementation of the "sizeof-pointer-to-aggregate" part of the
clang-tidy check `bugprone-sizeof-expression`.

I'm fairly certain that anything that was previously matched by the
deleted matcher `StructAddrOfExpr` is also covered by the more general
`PointerToStructExpr` (which remains in the same `anyOf`).

This commit is made to "prepare the ground" for a followup change that
would merge the functionality of the Clang Static Analyzer checker
`alpha.core.SizeofPtr` into this clang-tidy check.
@llvmbot
Copy link
Member

llvmbot commented May 22, 2024

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Donát Nagy (NagyDonat)

Changes

This commit eliminates a redundant matcher subexpression from the implementation of the "sizeof-pointer-to-aggregate" part of the clang-tidy check bugprone-sizeof-expression.

I'm fairly certain that anything that was previously matched by the deleted matcher StructAddrOfExpr is also covered by the more general PointerToStructExpr (which remains in the same anyOf).

This commit is made to "prepare the ground" for a followup change that would merge the functionality of the Clang Static Analyzer checker alpha.core.SizeofPtr into this clang-tidy check.


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

1 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp (+3-6)
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index a1cffbc666199..f119711638111 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -147,9 +147,6 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
     const auto PointerToArrayExpr = ignoringParenImpCasts(
         hasType(hasCanonicalType(pointerType(pointee(arrayType())))));
 
-    const auto StructAddrOfExpr = unaryOperator(
-        hasOperatorName("&"), hasUnaryOperand(ignoringParenImpCasts(
-                                  hasType(hasCanonicalType(recordType())))));
     const auto PointerToStructType =
         hasUnqualifiedDesugaredType(pointerType(pointee(recordType())));
     const auto PointerToStructExpr = ignoringParenImpCasts(expr(
@@ -171,9 +168,9 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
                                            has(ArrayOfPointersExpr)))))))),
              sizeOfExpr(has(ArrayOfSamePointersZeroSubscriptExpr)));
 
-    Finder->addMatcher(expr(anyOf(sizeOfExpr(has(ignoringParenImpCasts(anyOf(
-                                      ArrayCastExpr, PointerToArrayExpr,
-                                      StructAddrOfExpr, PointerToStructExpr)))),
+    Finder->addMatcher(expr(anyOf(sizeOfExpr(has(ignoringParenImpCasts(
+                                      anyOf(ArrayCastExpr, PointerToArrayExpr,
+                                            PointerToStructExpr)))),
                                   sizeOfExpr(has(PointerToStructType))),
                             unless(ArrayLengthExprDenom))
                            .bind("sizeof-pointer-to-aggregate"),

@NagyDonat NagyDonat requested a review from PiotrZSL May 22, 2024 11:59
NagyDonat added 2 commits May 22, 2024 14:10
Eliminate a few redundant `ignoreParenImpCasts` calls and simplify the
final matcher expression.
The traversal matcher `hasParent` moves a single layer outwards, so it
cannot skip over paren or implicit cast layers that could be ignored at
that point.
Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

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

LGTM (if tests pass)

@NagyDonat NagyDonat merged commit 5272768 into llvm:main May 22, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants