Fix warning about mismatches between function parameter and call-site args names #89294

Troy-Butler · 2024-04-18T19:50:20Z

No description provided.

Signed-off-by: Troy-Butler <[email protected]>

…m-project_tb into fix-redundancies

Signed-off-by: Troy-Butler <[email protected]>

github-actions · 2024-04-18T19:50:39Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-04-18T19:51:09Z

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-mlir-sparse

@llvm/pr-subscribers-llvm-transforms

Author: Troy Butler (Troy-Butler)

Changes

Addresses issue #88716

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

5 Files Affected:

(modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h (+2-2)
(modified) llvm/lib/TextAPI/TextStub.cpp (+2-2)
(modified) llvm/lib/Transforms/InstCombine/InstCombineInternal.h (+1-1)
(modified) llvm/lib/Transforms/Vectorize/VPlan.h (+2-2)
(modified) mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorLevel.h (+3-3)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
index fac0c04ae2caab..e60a49f68b7a0d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -227,12 +227,12 @@ class StoreManager {
   ///   information will not be used.
   virtual StoreRef invalidateRegions(Store store,
                                   ArrayRef<SVal> Values,
-                                  const Expr *E, unsigned Count,
+                                  const Expr *Ex, unsigned Count,
                                   const LocationContext *LCtx,
                                   const CallEvent *Call,
                                   InvalidatedSymbols &IS,
                                   RegionAndSymbolInvalidationTraits &ITraits,
-                                  InvalidatedRegions *InvalidatedTopLevel,
+                                  InvalidatedRegions *TopLevelRegions,
                                   InvalidatedRegions *Invalidated) = 0;
 
   /// enterStackFrame - Let the StoreManager to do something when execution
diff --git a/llvm/lib/TextAPI/TextStub.cpp b/llvm/lib/TextAPI/TextStub.cpp
index 0f742523f8207c..d903ba409360d6 100644
--- a/llvm/lib/TextAPI/TextStub.cpp
+++ b/llvm/lib/TextAPI/TextStub.cpp
@@ -276,7 +276,7 @@ namespace yaml {
 template <> struct MappingTraits<ExportSection> {
   static void mapping(IO &IO, ExportSection &Section) {
     const auto *Ctx = reinterpret_cast<TextAPIContext *>(IO.getContext());
-    assert((!Ctx || (Ctx && Ctx->FileKind != FileType::Invalid)) &&
+    assert((!Ctx || Ctx->FileKind != FileType::Invalid) &&
            "File type is not set in YAML context");
 
     IO.mapRequired("archs", Section.Architectures);
@@ -298,7 +298,7 @@ template <> struct MappingTraits<ExportSection> {
 template <> struct MappingTraits<UndefinedSection> {
   static void mapping(IO &IO, UndefinedSection &Section) {
     const auto *Ctx = reinterpret_cast<TextAPIContext *>(IO.getContext());
-    assert((!Ctx || (Ctx && Ctx->FileKind != FileType::Invalid)) &&
+    assert((!Ctx || Ctx->FileKind != FileType::Invalid) &&
            "File type is not set in YAML context");
 
     IO.mapRequired("archs", Section.Architectures);
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index b9ad3a74007929..90293feb9c1b82 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -433,7 +433,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
   Value *foldAndOrOfICmpsOfAndWithPow2(ICmpInst *LHS, ICmpInst *RHS,
                                        Instruction *CxtI, bool IsAnd,
                                        bool IsLogical = false);
-  Value *matchSelectFromAndOr(Value *A, Value *B, Value *C, Value *D,
+  Value *matchSelectFromAndOr(Value *A, Value *C, Value *B, Value *D,
                               bool InvertFalseVal = false);
   Value *getSelectCondition(Value *A, Value *B, bool ABIsTheSame);
 
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 22173954f7cec0..bd327d5fb8b605 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -3070,8 +3070,8 @@ class VPlan {
 private:
   /// Add to the given dominator tree the header block and every new basic block
   /// that was created between it and the latch block, inclusive.
-  static void updateDominatorTree(DominatorTree *DT, BasicBlock *LoopLatchBB,
-                                  BasicBlock *LoopPreHeaderBB,
+  static void updateDominatorTree(DominatorTree *DT, BasicBlock *LoopHeaderBB,
+                                  BasicBlock *LoopLatchBB,
                                   BasicBlock *LoopExitBB);
 };
 
diff --git a/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorLevel.h b/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorLevel.h
index 9f92eecdf75cb6..85ca22e937b457 100644
--- a/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorLevel.h
+++ b/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorLevel.h
@@ -283,9 +283,9 @@ class SparseIterator {
 };
 
 /// Helper function to create a TensorLevel object from given `tensor`.
-std::unique_ptr<SparseTensorLevel> makeSparseTensorLevel(OpBuilder &builder,
-                                                         Location loc, Value t,
-                                                         unsigned tid, Level l);
+std::unique_ptr<SparseTensorLevel> makeSparseTensorLevel(OpBuilder &b,
+                                                         Location l, Value t,
+                                                         unsigned tid, Level lvl);
 
 /// Helper function to create a simple SparseIterator object that iterate over
 /// the SparseTensorLevel.

Corrected filename mismatch to resolve conflict

nikic

InstCombine change looks good.

github-actions · 2024-04-19T00:02:58Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff e772a268ef75332b72dd9b9ca0341a6af8b0db72 7742e6dc1b9cae750ff210e4b778750038275650 -- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h llvm/lib/TextAPI/TextStub.cpp llvm/lib/Transforms/InstCombine/InstCombineInternal.h llvm/lib/Transforms/Vectorize/VPlan.h mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorIterator.h

View the diff from clang-format here.

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
index e60a49f68b..ef23b160a3 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -225,15 +225,11 @@ public:
   ///   invalidated. This should include any regions explicitly invalidated
   ///   even if they do not currently have bindings. Pass \c NULL if this
   ///   information will not be used.
-  virtual StoreRef invalidateRegions(Store store,
-                                  ArrayRef<SVal> Values,
-                                  const Expr *Ex, unsigned Count,
-                                  const LocationContext *LCtx,
-                                  const CallEvent *Call,
-                                  InvalidatedSymbols &IS,
-                                  RegionAndSymbolInvalidationTraits &ITraits,
-                                  InvalidatedRegions *TopLevelRegions,
-                                  InvalidatedRegions *Invalidated) = 0;
+  virtual StoreRef invalidateRegions(
+      Store store, ArrayRef<SVal> Values, const Expr *Ex, unsigned Count,
+      const LocationContext *LCtx, const CallEvent *Call,
+      InvalidatedSymbols &IS, RegionAndSymbolInvalidationTraits &ITraits,
+      InvalidatedRegions *TopLevelRegions, InvalidatedRegions *Invalidated) = 0;
 
   /// enterStackFrame - Let the StoreManager to do something when execution
   /// engine is about to execute into a callee.
diff --git a/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorIterator.h b/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorIterator.h
index 1668637c14..61220b5c78 100644
--- a/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorIterator.h
+++ b/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorIterator.h
@@ -287,7 +287,8 @@ private:
 /// Helper function to create a TensorLevel object from given `tensor`.
 std::unique_ptr<SparseTensorLevel> makeSparseTensorLevel(OpBuilder &b,
                                                          Location l, Value t,
-                                                         unsigned tid, Level lvl);
+                                                         unsigned tid,
+                                                         Level lvl);
 
 /// Helper function to create a simple SparseIterator object that iterate over
 /// the SparseTensorLevel.

llvm/lib/Transforms/InstCombine/InstCombineInternal.h

nikic · 2024-04-19T08:15:37Z

Isn't the warning about a mismatch between declaration and definition, not call args? The InstCombine change does make the definition and declaration match.

…

On Fri, Apr 19, 2024, at 17:07, Mehdi Amini wrote: ***@***.**** commented on this pull request. In llvm/lib/Transforms/InstCombine/InstCombineInternal.h <#89294 (comment)>: > @@ -433,7 +433,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final Value *foldAndOrOfICmpsOfAndWithPow2(ICmpInst *LHS, ICmpInst *RHS, Instruction *CxtI, bool IsAnd, bool IsLogical = false); - Value *matchSelectFromAndOr(Value *A, Value *B, Value *C, Value *D, You could fix the warning by using vastly different names for the function parameters? — Reply to this email directly, view it on GitHub <#89294 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABUYEAZJZLPIULGGF2GY3TY6DGDPAVCNFSM6AAAAABGN5RRXWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJQG44TSNJQGY>. You are receiving this because your review was requested.Message ID: ***@***.***>

dtcxzyw · 2024-04-19T08:22:57Z

Isn't the warning about a mismatch between declaration and definition, not call args? The InstCombine change does make the definition and declaration match.
…
On Fri, Apr 19, 2024, at 17:07, Mehdi Amini wrote: @.**** commented on this pull request. In llvm/lib/Transforms/InstCombine/InstCombineInternal.h <#89294 (comment)>: > @@ -433,7 +433,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final Value *foldAndOrOfICmpsOfAndWithPow2(ICmpInst *LHS, ICmpInst *RHS, Instruction *CxtI, bool IsAnd, bool IsLogical = false); - Value *matchSelectFromAndOr(Value *A, Value B, Value C, Value D, You could fix the warning by using vastly different names for the function parameters? — Reply to this email directly, view it on GitHub <#89294 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABUYEAZJZLPIULGGF2GY3TY6DGDPAVCNFSM6AAAAABGN5RRXWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJQG44TSNJQGY. You are receiving this because your review was requested.Message ID: @.>

Sorry for my misreading. Would be better to add a header comment for the declaration of matchSelectFromAndOr.

steakhal · 2024-04-19T08:47:03Z

StaticAnalyzer changes LGTM.

Troy-Butler · 2024-04-19T16:43:51Z

Isn't the warning about a mismatch between declaration and definition, not call args? The InstCombine change does make the definition and declaration match.
…
On Fri, Apr 19, 2024, at 17:07, Mehdi Amini wrote: @.**** commented on this pull request. In llvm/lib/Transforms/InstCombine/InstCombineInternal.h <#89294 (comment)>: > @@ -433,7 +433,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final Value *foldAndOrOfICmpsOfAndWithPow2(ICmpInst *LHS, ICmpInst *RHS, Instruction *CxtI, bool IsAnd, bool IsLogical = false); - Value *matchSelectFromAndOr(Value *A, Value B, Value C, Value D, You could fix the warning by using vastly different names for the function parameters? — Reply to this email directly, view it on GitHub <#89294 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABUYEAZJZLPIULGGF2GY3TY6DGDPAVCNFSM6AAAAABGN5RRXWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJQG44TSNJQGY. You are receiving this because your review was requested.Message ID: @.>

Sorry for my misreading. Would be better to add a header comment for the declaration of matchSelectFromAndOr.

Troy-Butler · 2024-04-19T16:44:44Z

Isn't the warning about a mismatch between declaration and definition, not call args? The InstCombine change does make the definition and declaration match.
…
On Fri, Apr 19, 2024, at 17:07, Mehdi Amini wrote: @.**** commented on this pull request. In llvm/lib/Transforms/InstCombine/InstCombineInternal.h <#89294 (comment)>: > @@ -433,7 +433,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final Value *foldAndOrOfICmpsOfAndWithPow2(ICmpInst *LHS, ICmpInst *RHS, Instruction *CxtI, bool IsAnd, bool IsLogical = false); - Value *matchSelectFromAndOr(Value *A, Value B, Value C, Value D, You could fix the warning by using vastly different names for the function parameters? — Reply to this email directly, view it on GitHub <#89294 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABUYEAZJZLPIULGGF2GY3TY6DGDPAVCNFSM6AAAAABGN5RRXWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJQG44TSNJQGY. You are receiving this because your review was requested.Message ID: @.>

Sorry for my misreading. Would be better to add a header comment for the declaration of matchSelectFromAndOr.

What do I need to change?

Troy-Butler · 2024-04-20T05:28:59Z

I'm not sure of how it happened, but despite fetching the latest version of files, my clone/fork didn't fully update before I made this pull request, and it's causing issues that I'm not experienced enough to resolve. To be safe, I'm going to close this pull request, erase my fork/clone, and start again. I apologize.

Troy-Butler added 4 commits March 25, 2024 13:28

[LLVM][LIB] Refactor Redundant Condition

fba4b05

Signed-off-by: Troy-Butler <[email protected]>

[LLVM][TextAPI] Refactor Redundant Condition

780151b

Signed-off-by: Troy-Butler <[email protected]>

Merge branch 'fix-redundancies' of https://github.com/Troy-Butler/llv…

32a69c6

…m-project_tb into fix-redundancies

Fix Definition Mismatches

7315c84

Signed-off-by: Troy-Butler <[email protected]>

Troy-Butler requested review from cyndyishida, aartbik, PeimingLiu, yinying-lisa-li, matthias-springer and nikic as code owners April 18, 2024 19:50

llvmbot added clang Clang issues not falling into any other category clang:static analyzer mlir:sparse Sparse compiler in MLIR vectorizers mlir llvm:transforms labels Apr 18, 2024

Rename SparseTensorLevel.h to SparseTensorIterator.h

53ace0b

Corrected filename mismatch to resolve conflict

steakhal self-requested a review April 18, 2024 20:16

Merge branch 'main' into fix-mismatches

7742e6d

nikic reviewed Apr 19, 2024

View reviewed changes

dtcxzyw requested changes Apr 19, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineInternal.h Show resolved Hide resolved

joker-eph changed the title ~~Fix Definition Mismatches~~ Fix warning about mismatches between function parameter and call-site args names Apr 19, 2024

dtcxzyw self-requested a review April 19, 2024 08:23

Troy-Butler closed this Apr 19, 2024

Troy-Butler reopened this Apr 19, 2024

Troy-Butler closed this Apr 20, 2024

Troy-Butler deleted the fix-mismatches branch April 20, 2024 05:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix warning about mismatches between function parameter and call-site args names #89294

Fix warning about mismatches between function parameter and call-site args names #89294

Uh oh!

Troy-Butler commented Apr 18, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Apr 18, 2024

Uh oh!

llvmbot commented Apr 18, 2024 •

edited

Loading

Uh oh!

nikic left a comment

Uh oh!

github-actions bot commented Apr 19, 2024

Uh oh!

Uh oh!

nikic commented Apr 19, 2024 via email

Uh oh!

dtcxzyw commented Apr 19, 2024

Uh oh!

steakhal commented Apr 19, 2024

Uh oh!

Troy-Butler commented Apr 19, 2024

Uh oh!

Troy-Butler commented Apr 19, 2024

Uh oh!

Troy-Butler commented Apr 20, 2024

Uh oh!

Uh oh!

Fix warning about mismatches between function parameter and call-site args names #89294

Fix warning about mismatches between function parameter and call-site args names #89294

Uh oh!

Conversation

Troy-Butler commented Apr 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 18, 2024

Uh oh!

llvmbot commented Apr 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 19, 2024

Uh oh!

Uh oh!

nikic commented Apr 19, 2024 via email

Uh oh!

dtcxzyw commented Apr 19, 2024

Uh oh!

steakhal commented Apr 19, 2024

Uh oh!

Troy-Butler commented Apr 19, 2024

Uh oh!

Troy-Butler commented Apr 19, 2024

Uh oh!

Troy-Butler commented Apr 20, 2024

Uh oh!

Uh oh!

Troy-Butler commented Apr 18, 2024 •

edited

Loading

llvmbot commented Apr 18, 2024 •

edited

Loading