mibintc marked 3 inline comments as done.
mibintc added a comment.
In D76384#1929774 <https://reviews.llvm.org/D76384#1929774>, @mibintc wrote:
> There's a bug in this patch that i haven't solved yet. I think it's just 1
> bug. The bug is that the AST statement visitor is going to the "fallback"
> instead of the right destination when walking CompoundAssignmentOperator.
> This patch collapses CompoundAssignmentOperator class into BinaryOperator
> class. In a spot check, i think all the failing tests are because the
> Visitor walk isn't working correctly for CompoundAssign (for example += ).
> These are the test fails reported by check-clang in my sandbox,
> Failing Tests (15):
>
> Clang :: Analysis/loopexit-cfg-output.cpp
> Clang :: CXX/drs/dr2xx.cpp
> Clang :: CXX/expr/expr.const/p2-0x.cpp
> Clang ::
> CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p17.cpp
> Clang :: CodeGen/ffp-contract-fast-option.cpp
> Clang :: CodeGenCXX/const-init-cxx1y.cpp
> Clang :: CodeGenCXX/const-init-cxx2a.cpp
> Clang :: CodeGenCXX/non-const-init-cxx2a.cpp
> Clang :: Sema/warn-unreachable.c
> Clang :: Sema/warn-unsequenced.c
> Clang :: SemaCXX/constant-expression-cxx1y.cpp
> Clang :: SemaCXX/constant-expression-cxx2a.cpp
> Clang :: SemaCXX/decomposed-condition.cpp
> Clang :: SemaCXX/integer-overflow.cpp
> Clang :: SemaCXX/warn-unsequenced.cpp
The new revision corrects all these fails. i have a couple questions about test
cases, i've inline those in the test case modification. Also need to do
clang-format and also some changes to CXXCallOperator to remove redundant
accessor
================
Comment at: clang/include/clang/AST/ExprCXX.h:168
// operations on floating point types.
void setFPFeatures(FPOptions F) {
+ FPFeatures = F;
----------------
i can get rid of these 2 accessor functions, we get them from CallExpr
================
Comment at: clang/test/Analysis/loopexit-cfg-output.cpp:211
// CHECK: [B3]
-// CHECK-NEXT: 1: j
-// CHECK-NEXT: 2: 2
-// CHECK-NEXT: 3: [B3.1] += [B3.2]
+// CHECK-NEXT: 1: 2
+// CHECK-NEXT: 2: j
----------------
With these changes, the blocks print out in a different order but the semantic
meaning appears to be the same. Is this difference acceptable?
================
Comment at: clang/test/Sema/warn-unreachable.c:86
case 8:
- i
- += // expected-warning {{will never be executed}}
+ i // expected-warning {{will never be executed}}
+ +=
----------------
The srcpos for compound assignment is now the same as srcpos for ordinary
binary operator, so i changed the test case to move the diagnostic, ok?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76384/new/
https://reviews.llvm.org/D76384
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
