[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage - preliminary

Melanie Blower via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 18 13:35:25 PDT 2020


mibintc created this revision.
mibintc added reviewers: rjmccall, sepavloff.
Herald added subscribers: martong, jfb, arphaman.
Herald added a reviewer: shafik.
Herald added a project: clang.
mibintc added a comment.

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


This patch moves the FPFeatures field from BinaryOperator bitfields to Trailing storage, since there is a current desire to widen the FPOptions type.  It's a preliminary patch, looking for feedback from @rjmccall and others. On the dev email list, there was an alternate proposal to maintain this information in state.  I had already partly implemented this change so I just continued along this way.

There's still a problem in the patch, there's a bug in the AST statement visitor, something wrong with the template metaprogramming? i still haven't found that problem, but i wanted to put this up for any feedback before i find and fix that problem, If you see what i did wrong please let me know. The problem has to do with combining the CompoundAssignmentOperator and BinaryOperator.  I needed to do that because BinaryOperator needs to be finalized before Trailing storage can be used.

FPFeatures are also needed on CXXOperatorCall. I put that field into CallExpr. Do you want that to go into Trailing storage as well? I didn't do that. It would also require CallExpr and CXXOperatorCall to be collapsed together.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76384

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/Analysis/ReachableCode.cpp
  clang/lib/Analysis/ThreadSafetyCommon.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Index/IndexBody.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaPseudoObject.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/AST/ast-dump-expr-json.c
  clang/test/AST/ast-dump-expr.c
  clang/test/AST/dump.cpp
  clang/test/Import/compound-assign-op/test.cpp
  clang/tools/libclang/CXCursor.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76384.251158.patch
Type: text/x-patch
Size: 95328 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200318/55babbc8/attachment-0001.bin>


More information about the cfe-commits mailing list