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][lldb][mlir] Fix some identical sub-expressions warnings (NFC) #95715

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

xgupta
Copy link
Contributor

@xgupta xgupta commented Jun 16, 2024

This is reported in https://pvs-studio.com/en/blog/posts/cpp/1126/, fragment N4-8

V501 There are identical sub-expressions 'FirstStmt->getStmtClass()' to the left and to the right of the '!=' operator. ASTUtils.cpp:99
V501 There are identical sub-expressions to the left and to the right of the '!=' operator: Fn1->isVariadic() != Fn1->isVariadic(). SemaOverload.cpp:10190
V501 There are identical sub-expressions to the left and to the right of the '<' operator: G1->Rank < G1->Rank. SCCIterator.h:285
V501 There are identical sub-expressions 'slice1.getMixedOffsets().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:581
V501 There are identical sub-expressions 'slice1.getMixedSizes().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:583
V501 There are identical sub-expressions 'slice1.getMixedStrides().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:585
V501 There are identical sub-expressions 'slice1.getMixedOffsets().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:646
V501 There are identical sub-expressions 'slice1.getMixedSizes().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:648
V501 There are identical sub-expressions 'slice1.getMixedStrides().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:650
V501 There are identical sub-expressions 'EltRange.getEnd() >= Range.getEnd()' to the left and to the right of the '||' operator. HTMLLogger.cpp:421
V501 There are identical sub-expressions 'SrcExpr.get()->containsErrors()' to the left and to the right of the '||' operator. SemaCast.cpp:2938
V501 There are identical sub-expressions 'ND->getDeclContext()' to the left and to the right of the '!=' operator. SemaDeclCXX.cpp:4391
V501 There are identical sub-expressions 'DepType != OMPC_DOACROSS_source' to the left and to the right of the '&&' operator. SemaOpenMP.cpp:24348
V501 There are identical sub-expressions '!OldMethod->isStatic()' to the left and to the right of the '&&' operator. SemaOverload.cpp:1425
V501 There are identical sub-expressions 'lldb::eTypeClassUnion' to the left and to the right of the '|' operator. JSONUtils.cpp:139
V501 There are identical sub-expressions to the left and to the right of the '&&' operator: !BFI &&!BFI. JumpThreading.cpp:2531
V501 There are identical sub-expressions 'BI->isConditional()' to the left and to the right of the '&&' operator. VPlanHCFGBuilder.cpp:401
V501 There are identical sub-expressions to the left and to the right of the '==' operator: getNumRows() == getNumRows(). Simplex.cpp:108

@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra clang-tidy clang:frontend Language frontend issues, e.g. anything involving "Sema" vectorizers mlir mlir:presburger clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang:analysis llvm:transforms clang:openmp OpenMP related changes to Clang labels Jun 16, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jun 16, 2024

@llvm/pr-subscribers-clang-tidy
@llvm/pr-subscribers-mlir-presburger
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-clang-analysis
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-clang

Author: Shivam Gupta (xgupta)

Changes

This is reported in https://pvs-studio.com/en/blog/posts/cpp/1126/, fragment N4-8

V501 There are identical sub-expressions 'FirstStmt->getStmtClass()' to the left and to the right of the '!=' operator. ASTUtils.cpp:99
V501 There are identical sub-expressions to the left and to the right of the '!=' operator: Fn1->isVariadic() != Fn1->isVariadic(). SemaOverload.cpp:10190
V501 There are identical sub-expressions to the left and to the right of the '<' operator: G1->Rank < G1->Rank. SCCIterator.h:285
V501 There are identical sub-expressions 'slice1.getMixedOffsets().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:581
V501 There are identical sub-expressions 'slice1.getMixedSizes().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:583
V501 There are identical sub-expressions 'slice1.getMixedStrides().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:585
V501 There are identical sub-expressions 'slice1.getMixedOffsets().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:646
V501 There are identical sub-expressions 'slice1.getMixedSizes().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:648
V501 There are identical sub-expressions 'slice1.getMixedStrides().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:650
V501 There are identical sub-expressions 'EltRange.getEnd() >= Range.getEnd()' to the left and to the right of the '||' operator. HTMLLogger.cpp:421
V501 There are identical sub-expressions 'SrcExpr.get()->containsErrors()' to the left and to the right of the '||' operator. SemaCast.cpp:2938
V501 There are identical sub-expressions 'ND->getDeclContext()' to the left and to the right of the '!=' operator. SemaDeclCXX.cpp:4391
V501 There are identical sub-expressions 'DepType != OMPC_DOACROSS_source' to the left and to the right of the '&&' operator. SemaOpenMP.cpp:24348
V501 There are identical sub-expressions '!OldMethod->isStatic()' to the left and to the right of the '&&' operator. SemaOverload.cpp:1425
V501 There are identical sub-expressions 'lldb::eTypeClassUnion' to the left and to the right of the '|' operator. JSONUtils.cpp:139
V501 There are identical sub-expressions to the left and to the right of the '&&' operator: !BFI &&!BFI. JumpThreading.cpp:2531
V501 There are identical sub-expressions 'BI->isConditional()' to the left and to the right of the '&&' operator. VPlanHCFGBuilder.cpp:401
V501 There are identical sub-expressions to the left and to the right of the '==' operator: getNumRows() == getNumRows(). Simplex.cpp:108


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

9 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/utils/ASTUtils.cpp (+1-1)
  • (modified) clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp (+3-4)
  • (modified) clang/lib/Sema/SemaCast.cpp (+1-2)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaOpenMP.cpp (+1-2)
  • (modified) llvm/lib/Transforms/Scalar/JumpThreading.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp (+1-1)
  • (modified) mlir/lib/Analysis/Presburger/Simplex.cpp (+2-1)
  • (modified) mlir/lib/Interfaces/ValueBoundsOpInterface.cpp (+6-6)
diff --git a/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp b/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp
index fd5dadc9b01db..0cdc7d08abc99 100644
--- a/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp
@@ -96,7 +96,7 @@ bool areStatementsIdentical(const Stmt *FirstStmt, const Stmt *SecondStmt,
   if (FirstStmt == SecondStmt)
     return true;
 
-  if (FirstStmt->getStmtClass() != FirstStmt->getStmtClass())
+  if (FirstStmt->getStmtClass() != SecondStmt->getStmtClass())
     return false;
 
   if (isa<Expr>(FirstStmt) && isa<Expr>(SecondStmt)) {
diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index a36cb41a63dfb..323f9698dc2aa 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -430,11 +430,10 @@ class HTMLLogger : public Logger {
               AST.getSourceManager(), AST.getLangOpts());
           if (EltRange.isInvalid())
             continue;
-          if (EltRange.getBegin() < Range.getBegin() ||
-              EltRange.getEnd() >= Range.getEnd() ||
-              EltRange.getEnd() < Range.getBegin() ||
-              EltRange.getEnd() >= Range.getEnd())
+          if (EltRange.getEnd() <= Range.getBegin() ||
+              EltRange.getBegin() >= Range.getEnd())
             continue;
+
           unsigned Off = EltRange.getBegin().getRawEncoding() -
                          Range.getBegin().getRawEncoding();
           unsigned Len = EltRange.getEnd().getRawEncoding() -
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index f03dcf05411df..bfdb8896f81c3 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -2947,8 +2947,7 @@ void CastOperation::CheckCStyleCast() {
   if (Self.getASTContext().isDependenceAllowed() &&
       (DestType->isDependentType() || SrcExpr.get()->isTypeDependent() ||
        SrcExpr.get()->isValueDependent())) {
-    assert((DestType->containsErrors() || SrcExpr.get()->containsErrors() ||
-            SrcExpr.get()->containsErrors()) &&
+    assert((DestType->containsErrors() || SrcExpr.get()->containsErrors()) &&
            "should only occur in error-recovery path.");
     assert(Kind == CK_Dependent);
     return;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 37f0df2a6a27d..27c16cebde419 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4368,7 +4368,7 @@ bool Sema::DiagRedefinedPlaceholderFieldDecl(SourceLocation Loc,
   Diag(Loc, diag::err_using_placeholder_variable) << Name;
   for (DeclContextLookupResult::iterator It = Found; It != Result.end(); It++) {
     const NamedDecl *ND = *It;
-    if (ND->getDeclContext() != ND->getDeclContext())
+    if (ND->getDeclContext() != ClassDecl->getDeclContext())
       break;
     if (isa<FieldDecl, IndirectFieldDecl>(ND) &&
         ND->isPlaceholderVar(getLangOpts()))
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 5c759aedf9798..0878a2f635c36 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -24771,8 +24771,7 @@ OMPClause *SemaOpenMP::ActOnOpenMPDoacrossClause(
   if (DSAStack->getCurrentDirective() == OMPD_ordered &&
       DepType != OMPC_DOACROSS_source && DepType != OMPC_DOACROSS_sink &&
       DepType != OMPC_DOACROSS_sink_omp_cur_iteration &&
-      DepType != OMPC_DOACROSS_source_omp_cur_iteration &&
-      DepType != OMPC_DOACROSS_source) {
+      DepType != OMPC_DOACROSS_source_omp_cur_iteration) {
     Diag(DepLoc, diag::err_omp_unexpected_clause_value)
         << "'source' or 'sink'" << getOpenMPClauseName(OMPC_doacross);
     return nullptr;
diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index 1aef2800e9846..d9c761e2b08ec 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -2533,7 +2533,7 @@ void JumpThreadingPass::updateBlockFreqAndEdgeWeight(BasicBlock *PredBB,
                                                      BlockFrequencyInfo *BFI,
                                                      BranchProbabilityInfo *BPI,
                                                      bool HasProfile) {
-  assert(((BFI && BPI) || (!BFI && !BFI)) &&
+  assert(((BFI && BPI) || (!BFI && !BPI)) &&
          "Both BFI & BPI should either be set or unset");
 
   if (!BFI) {
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index b57ff2840a729..a1c6521a3f361 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -397,7 +397,7 @@ void PlainCFGBuilder::buildPlainCFG() {
                                 : static_cast<VPBlockBase *>(Successor));
       continue;
     }
-    assert(BI->isConditional() && NumSuccs == 2 && BI->isConditional() &&
+    assert(BI->isConditional() && NumSuccs == 2 &&
            "block must have conditional branch with 2 successors");
     // Look up the branch condition to get the corresponding VPValue
     // representing the condition bit in VPlan (which may be in another VPBB).
diff --git a/mlir/lib/Analysis/Presburger/Simplex.cpp b/mlir/lib/Analysis/Presburger/Simplex.cpp
index 2cdd79d42732d..d6c920e835be1 100644
--- a/mlir/lib/Analysis/Presburger/Simplex.cpp
+++ b/mlir/lib/Analysis/Presburger/Simplex.cpp
@@ -104,8 +104,9 @@ Simplex::Unknown &SimplexBase::unknownFromRow(unsigned row) {
 
 unsigned SimplexBase::addZeroRow(bool makeRestricted) {
   // Resize the tableau to accommodate the extra row.
+  unsigned oldNumRows = getNumRows();
   unsigned newRow = tableau.appendExtraRow();
-  assert(getNumRows() == getNumRows() && "Inconsistent tableau size");
+  assert(getNumRows() == oldNumRows + 1 && "Inconsistent tableau size");
   rowUnknown.push_back(~con.size());
   con.emplace_back(Orientation::Row, makeRestricted, newRow);
   undoLog.push_back(UndoLogEntry::RemoveLastConstraint);
diff --git a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
index 6420c192b257d..4b9f026832044 100644
--- a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
+++ b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
@@ -778,11 +778,11 @@ FailureOr<bool>
 ValueBoundsConstraintSet::areOverlappingSlices(MLIRContext *ctx,
                                                HyperrectangularSlice slice1,
                                                HyperrectangularSlice slice2) {
-  assert(slice1.getMixedOffsets().size() == slice1.getMixedOffsets().size() &&
+  assert(slice1.getMixedOffsets().size() == slice2.getMixedOffsets().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedSizes().size() == slice1.getMixedSizes().size() &&
+  assert(slice1.getMixedSizes().size() == slice2.getMixedSizes().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedStrides().size() == slice1.getMixedStrides().size() &&
+  assert(slice1.getMixedStrides().size() == slice2.getMixedStrides().size() &&
          "expected slices of same rank");
 
   Builder b(ctx);
@@ -843,11 +843,11 @@ FailureOr<bool>
 ValueBoundsConstraintSet::areEquivalentSlices(MLIRContext *ctx,
                                               HyperrectangularSlice slice1,
                                               HyperrectangularSlice slice2) {
-  assert(slice1.getMixedOffsets().size() == slice1.getMixedOffsets().size() &&
+  assert(slice1.getMixedOffsets().size() == slice2.getMixedOffsets().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedSizes().size() == slice1.getMixedSizes().size() &&
+  assert(slice1.getMixedSizes().size() == slice2.getMixedSizes().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedStrides().size() == slice1.getMixedStrides().size() &&
+  assert(slice1.getMixedStrides().size() == slice2.getMixedStrides().size() &&
          "expected slices of same rank");
 
   // The two slices are equivalent if all of their offsets, sizes and strides

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 16, 2024

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

Author: Shivam Gupta (xgupta)

Changes

This is reported in https://pvs-studio.com/en/blog/posts/cpp/1126/, fragment N4-8

V501 There are identical sub-expressions 'FirstStmt->getStmtClass()' to the left and to the right of the '!=' operator. ASTUtils.cpp:99
V501 There are identical sub-expressions to the left and to the right of the '!=' operator: Fn1->isVariadic() != Fn1->isVariadic(). SemaOverload.cpp:10190
V501 There are identical sub-expressions to the left and to the right of the '<' operator: G1->Rank < G1->Rank. SCCIterator.h:285
V501 There are identical sub-expressions 'slice1.getMixedOffsets().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:581
V501 There are identical sub-expressions 'slice1.getMixedSizes().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:583
V501 There are identical sub-expressions 'slice1.getMixedStrides().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:585
V501 There are identical sub-expressions 'slice1.getMixedOffsets().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:646
V501 There are identical sub-expressions 'slice1.getMixedSizes().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:648
V501 There are identical sub-expressions 'slice1.getMixedStrides().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:650
V501 There are identical sub-expressions 'EltRange.getEnd() >= Range.getEnd()' to the left and to the right of the '||' operator. HTMLLogger.cpp:421
V501 There are identical sub-expressions 'SrcExpr.get()->containsErrors()' to the left and to the right of the '||' operator. SemaCast.cpp:2938
V501 There are identical sub-expressions 'ND->getDeclContext()' to the left and to the right of the '!=' operator. SemaDeclCXX.cpp:4391
V501 There are identical sub-expressions 'DepType != OMPC_DOACROSS_source' to the left and to the right of the '&&' operator. SemaOpenMP.cpp:24348
V501 There are identical sub-expressions '!OldMethod->isStatic()' to the left and to the right of the '&&' operator. SemaOverload.cpp:1425
V501 There are identical sub-expressions 'lldb::eTypeClassUnion' to the left and to the right of the '|' operator. JSONUtils.cpp:139
V501 There are identical sub-expressions to the left and to the right of the '&&' operator: !BFI &&!BFI. JumpThreading.cpp:2531
V501 There are identical sub-expressions 'BI->isConditional()' to the left and to the right of the '&&' operator. VPlanHCFGBuilder.cpp:401
V501 There are identical sub-expressions to the left and to the right of the '==' operator: getNumRows() == getNumRows(). Simplex.cpp:108


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

9 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/utils/ASTUtils.cpp (+1-1)
  • (modified) clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp (+3-4)
  • (modified) clang/lib/Sema/SemaCast.cpp (+1-2)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaOpenMP.cpp (+1-2)
  • (modified) llvm/lib/Transforms/Scalar/JumpThreading.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp (+1-1)
  • (modified) mlir/lib/Analysis/Presburger/Simplex.cpp (+2-1)
  • (modified) mlir/lib/Interfaces/ValueBoundsOpInterface.cpp (+6-6)
diff --git a/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp b/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp
index fd5dadc9b01db..0cdc7d08abc99 100644
--- a/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp
@@ -96,7 +96,7 @@ bool areStatementsIdentical(const Stmt *FirstStmt, const Stmt *SecondStmt,
   if (FirstStmt == SecondStmt)
     return true;
 
-  if (FirstStmt->getStmtClass() != FirstStmt->getStmtClass())
+  if (FirstStmt->getStmtClass() != SecondStmt->getStmtClass())
     return false;
 
   if (isa<Expr>(FirstStmt) && isa<Expr>(SecondStmt)) {
diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index a36cb41a63dfb..323f9698dc2aa 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -430,11 +430,10 @@ class HTMLLogger : public Logger {
               AST.getSourceManager(), AST.getLangOpts());
           if (EltRange.isInvalid())
             continue;
-          if (EltRange.getBegin() < Range.getBegin() ||
-              EltRange.getEnd() >= Range.getEnd() ||
-              EltRange.getEnd() < Range.getBegin() ||
-              EltRange.getEnd() >= Range.getEnd())
+          if (EltRange.getEnd() <= Range.getBegin() ||
+              EltRange.getBegin() >= Range.getEnd())
             continue;
+
           unsigned Off = EltRange.getBegin().getRawEncoding() -
                          Range.getBegin().getRawEncoding();
           unsigned Len = EltRange.getEnd().getRawEncoding() -
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index f03dcf05411df..bfdb8896f81c3 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -2947,8 +2947,7 @@ void CastOperation::CheckCStyleCast() {
   if (Self.getASTContext().isDependenceAllowed() &&
       (DestType->isDependentType() || SrcExpr.get()->isTypeDependent() ||
        SrcExpr.get()->isValueDependent())) {
-    assert((DestType->containsErrors() || SrcExpr.get()->containsErrors() ||
-            SrcExpr.get()->containsErrors()) &&
+    assert((DestType->containsErrors() || SrcExpr.get()->containsErrors()) &&
            "should only occur in error-recovery path.");
     assert(Kind == CK_Dependent);
     return;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 37f0df2a6a27d..27c16cebde419 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4368,7 +4368,7 @@ bool Sema::DiagRedefinedPlaceholderFieldDecl(SourceLocation Loc,
   Diag(Loc, diag::err_using_placeholder_variable) << Name;
   for (DeclContextLookupResult::iterator It = Found; It != Result.end(); It++) {
     const NamedDecl *ND = *It;
-    if (ND->getDeclContext() != ND->getDeclContext())
+    if (ND->getDeclContext() != ClassDecl->getDeclContext())
       break;
     if (isa<FieldDecl, IndirectFieldDecl>(ND) &&
         ND->isPlaceholderVar(getLangOpts()))
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 5c759aedf9798..0878a2f635c36 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -24771,8 +24771,7 @@ OMPClause *SemaOpenMP::ActOnOpenMPDoacrossClause(
   if (DSAStack->getCurrentDirective() == OMPD_ordered &&
       DepType != OMPC_DOACROSS_source && DepType != OMPC_DOACROSS_sink &&
       DepType != OMPC_DOACROSS_sink_omp_cur_iteration &&
-      DepType != OMPC_DOACROSS_source_omp_cur_iteration &&
-      DepType != OMPC_DOACROSS_source) {
+      DepType != OMPC_DOACROSS_source_omp_cur_iteration) {
     Diag(DepLoc, diag::err_omp_unexpected_clause_value)
         << "'source' or 'sink'" << getOpenMPClauseName(OMPC_doacross);
     return nullptr;
diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index 1aef2800e9846..d9c761e2b08ec 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -2533,7 +2533,7 @@ void JumpThreadingPass::updateBlockFreqAndEdgeWeight(BasicBlock *PredBB,
                                                      BlockFrequencyInfo *BFI,
                                                      BranchProbabilityInfo *BPI,
                                                      bool HasProfile) {
-  assert(((BFI && BPI) || (!BFI && !BFI)) &&
+  assert(((BFI && BPI) || (!BFI && !BPI)) &&
          "Both BFI & BPI should either be set or unset");
 
   if (!BFI) {
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index b57ff2840a729..a1c6521a3f361 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -397,7 +397,7 @@ void PlainCFGBuilder::buildPlainCFG() {
                                 : static_cast<VPBlockBase *>(Successor));
       continue;
     }
-    assert(BI->isConditional() && NumSuccs == 2 && BI->isConditional() &&
+    assert(BI->isConditional() && NumSuccs == 2 &&
            "block must have conditional branch with 2 successors");
     // Look up the branch condition to get the corresponding VPValue
     // representing the condition bit in VPlan (which may be in another VPBB).
diff --git a/mlir/lib/Analysis/Presburger/Simplex.cpp b/mlir/lib/Analysis/Presburger/Simplex.cpp
index 2cdd79d42732d..d6c920e835be1 100644
--- a/mlir/lib/Analysis/Presburger/Simplex.cpp
+++ b/mlir/lib/Analysis/Presburger/Simplex.cpp
@@ -104,8 +104,9 @@ Simplex::Unknown &SimplexBase::unknownFromRow(unsigned row) {
 
 unsigned SimplexBase::addZeroRow(bool makeRestricted) {
   // Resize the tableau to accommodate the extra row.
+  unsigned oldNumRows = getNumRows();
   unsigned newRow = tableau.appendExtraRow();
-  assert(getNumRows() == getNumRows() && "Inconsistent tableau size");
+  assert(getNumRows() == oldNumRows + 1 && "Inconsistent tableau size");
   rowUnknown.push_back(~con.size());
   con.emplace_back(Orientation::Row, makeRestricted, newRow);
   undoLog.push_back(UndoLogEntry::RemoveLastConstraint);
diff --git a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
index 6420c192b257d..4b9f026832044 100644
--- a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
+++ b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
@@ -778,11 +778,11 @@ FailureOr<bool>
 ValueBoundsConstraintSet::areOverlappingSlices(MLIRContext *ctx,
                                                HyperrectangularSlice slice1,
                                                HyperrectangularSlice slice2) {
-  assert(slice1.getMixedOffsets().size() == slice1.getMixedOffsets().size() &&
+  assert(slice1.getMixedOffsets().size() == slice2.getMixedOffsets().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedSizes().size() == slice1.getMixedSizes().size() &&
+  assert(slice1.getMixedSizes().size() == slice2.getMixedSizes().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedStrides().size() == slice1.getMixedStrides().size() &&
+  assert(slice1.getMixedStrides().size() == slice2.getMixedStrides().size() &&
          "expected slices of same rank");
 
   Builder b(ctx);
@@ -843,11 +843,11 @@ FailureOr<bool>
 ValueBoundsConstraintSet::areEquivalentSlices(MLIRContext *ctx,
                                               HyperrectangularSlice slice1,
                                               HyperrectangularSlice slice2) {
-  assert(slice1.getMixedOffsets().size() == slice1.getMixedOffsets().size() &&
+  assert(slice1.getMixedOffsets().size() == slice2.getMixedOffsets().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedSizes().size() == slice1.getMixedSizes().size() &&
+  assert(slice1.getMixedSizes().size() == slice2.getMixedSizes().size() &&
          "expected slices of same rank");
-  assert(slice1.getMixedStrides().size() == slice1.getMixedStrides().size() &&
+  assert(slice1.getMixedStrides().size() == slice2.getMixedStrides().size() &&
          "expected slices of same rank");
 
   // The two slices are equivalent if all of their offsets, sizes and strides

@AaronBallman
Copy link
Collaborator

Note, this fixes #95670

clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp Outdated Show resolved Hide resolved
@@ -4368,7 +4368,7 @@ bool Sema::DiagRedefinedPlaceholderFieldDecl(SourceLocation Loc,
Diag(Loc, diag::err_using_placeholder_variable) << Name;
for (DeclContextLookupResult::iterator It = Found; It != Result.end(); It++) {
const NamedDecl *ND = *It;
if (ND->getDeclContext() != ND->getDeclContext())
if (ND->getDeclContext() != ClassDecl->getDeclContext())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like a functional change to me; if no tests broke, we should probably figure out a test to add for this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not familiar with this part of code to add a testcase, and not even sure it is right fix. I will just remove this changes from this PR.

@@ -96,7 +96,7 @@ bool areStatementsIdentical(const Stmt *FirstStmt, const Stmt *SecondStmt,
if (FirstStmt == SecondStmt)
return true;

if (FirstStmt->getStmtClass() != FirstStmt->getStmtClass())
if (FirstStmt->getStmtClass() != SecondStmt->getStmtClass())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like a functional change, it would be good to have a test for it.

Copy link
Member

Choose a reason for hiding this comment

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

Thx for info, that were a bug.

Copy link
Contributor Author

@xgupta xgupta Jul 26, 2024

Choose a reason for hiding this comment

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

@PiotrZSL If I removed this condition and ninja target still passes. Shall I remove this or can you provide a test case or guidance? I have seen this function is used in BranchCloneCheck.cpp but I am not sure how the test case look like.

xgupta and others added 4 commits July 26, 2024 11:55
    This is actually reported in https://pvs-studio.com/en/blog/posts/cpp/1126/, fragment N4-8

    V501 There are identical sub-expressions 'FirstStmt->getStmtClass()' to the left and to the right of the '!=' operator. ASTUtils.cpp:99
    V501 There are identical sub-expressions to the left and to the right of the '!=' operator: Fn1->isVariadic() != Fn1->isVariadic(). SemaOverload.cpp:10190
    V501 There are identical sub-expressions to the left and to the right of the '<' operator: G1->Rank < G1->Rank. SCCIterator.h:285
    V501 There are identical sub-expressions 'slice1.getMixedOffsets().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:581
    V501 There are identical sub-expressions 'slice1.getMixedSizes().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:583
    V501 There are identical sub-expressions 'slice1.getMixedStrides().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:585
    V501 There are identical sub-expressions 'slice1.getMixedOffsets().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:646
    V501 There are identical sub-expressions 'slice1.getMixedSizes().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:648
    V501 There are identical sub-expressions 'slice1.getMixedStrides().size()' to the left and to the right of the '==' operator. ValueBoundsOpInterface.cpp:650
    V501 There are identical sub-expressions 'EltRange.getEnd() >= Range.getEnd()' to the left and to the right of the '||' operator. HTMLLogger.cpp:421
    V501 There are identical sub-expressions 'SrcExpr.get()->containsErrors()' to the left and to the right of the '||' operator. SemaCast.cpp:2938
    V501 There are identical sub-expressions 'ND->getDeclContext()' to the left and to the right of the '!=' operator. SemaDeclCXX.cpp:4391
    V501 There are identical sub-expressions 'DepType != OMPC_DOACROSS_source' to the left and to the right of the '&&' operator. SemaOpenMP.cpp:24348
    V501 There are identical sub-expressions '!OldMethod->isStatic()' to the left and to the right of the '&&' operator. SemaOverload.cpp:1425
    V501 There are identical sub-expressions 'lldb::eTypeClassUnion' to the left and to the right of the '|' operator. JSONUtils.cpp:139
    V501 There are identical sub-expressions to the left and to the right of the '&&' operator: !BFI &&!BFI. JumpThreading.cpp:2531
    V501 There are identical sub-expressions 'BI->isConditional()' to the left and to the right of the '&&' operator. VPlanHCFGBuilder.cpp:401
    V501 There are identical sub-expressions to the left and to the right of the '==' operator: getNumRows() == getNumRows(). Simplex.cpp:108
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:analysis clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category clang-tidy clang-tools-extra llvm:transforms mlir:presburger mlir vectorizers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants