This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe6e753d173db: [clang][dataflow] Fix wrong assert for CXXConstructExpr (authored by paulsemel). Herald added a subscriber: cfe-commits.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144546/new/ https://reviews.llvm.org/D144546 Files: clang/lib/Analysis/FlowSensitive/Transfer.cpp clang/unittests/Analysis/FlowSensitive/TransferTest.cpp Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp =================================================================== --- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -2102,6 +2102,54 @@ }); } +TEST(TransferTest, CopyConstructorWithDefaultArgument) { + std::string Code = R"( + struct A { + int Baz; + A() = default; + A(const A& a, bool def = true) { Baz = a.Baz; } + }; + + void target() { + A Foo; + (void)Foo.Baz; + A Bar = Foo; + // [[p]] + } + )"; + runDataflow( + Code, + [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz"); + ASSERT_THAT(BazDecl, NotNull()); + + const auto *FooLoc = cast<AggregateStorageLocation>( + Env.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc = cast<AggregateStorageLocation>( + Env.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc)); + const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc)); + EXPECT_EQ(FooVal, BarVal); + + const auto *FooBazVal = + cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl))); + const auto *BarBazVal = + cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl))); + EXPECT_EQ(FooBazVal, BarBazVal); + }); +} + TEST(TransferTest, CopyConstructorWithParens) { std::string Code = R"( struct A { Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -561,7 +561,9 @@ assert(ConstructorDecl != nullptr); if (ConstructorDecl->isCopyOrMoveConstructor()) { - assert(S->getNumArgs() == 1); + // It is permissible for a copy/move constructor to have additional + // parameters as long as they have default arguments defined for them. + assert(S->getNumArgs() != 0); const Expr *Arg = S->getArg(0); assert(Arg != nullptr);
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp =================================================================== --- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -2102,6 +2102,54 @@ }); } +TEST(TransferTest, CopyConstructorWithDefaultArgument) { + std::string Code = R"( + struct A { + int Baz; + A() = default; + A(const A& a, bool def = true) { Baz = a.Baz; } + }; + + void target() { + A Foo; + (void)Foo.Baz; + A Bar = Foo; + // [[p]] + } + )"; + runDataflow( + Code, + [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz"); + ASSERT_THAT(BazDecl, NotNull()); + + const auto *FooLoc = cast<AggregateStorageLocation>( + Env.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc = cast<AggregateStorageLocation>( + Env.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc)); + const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc)); + EXPECT_EQ(FooVal, BarVal); + + const auto *FooBazVal = + cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl))); + const auto *BarBazVal = + cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl))); + EXPECT_EQ(FooBazVal, BarBazVal); + }); +} + TEST(TransferTest, CopyConstructorWithParens) { std::string Code = R"( struct A { Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -561,7 +561,9 @@ assert(ConstructorDecl != nullptr); if (ConstructorDecl->isCopyOrMoveConstructor()) { - assert(S->getNumArgs() == 1); + // It is permissible for a copy/move constructor to have additional + // parameters as long as they have default arguments defined for them. + assert(S->getNumArgs() != 0); const Expr *Arg = S->getArg(0); assert(Arg != nullptr);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits