ymandel created this revision.
ymandel added reviewers: xazax.hun, sgatev.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.
Currently, when an `AggregateStorageLocation` is mapped to a `StructValue`, we
enforce that it's type is a struct or class. However, we don't need this to hold
and there are reasonable violations of this assumption -- for example, using the
`StructValue` for its properties, while modeling a non-struct type.
This patch removes the assertion.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126973
Files:
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -10,6 +10,7 @@
#include "NoopAnalysis.h"
#include "TestingSupport.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
#include "clang/Analysis/FlowSensitive/Value.h"
#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
#include "gmock/gmock.h"
@@ -20,8 +21,7 @@
using namespace clang;
using namespace dataflow;
-using ::testing::ElementsAre;
-using ::testing::Pair;
+using ::testing::NotNull;
class EnvironmentTest : public ::testing::Test {
DataflowAnalysisContext Context;
@@ -98,4 +98,25 @@
EXPECT_NE(PV, nullptr);
}
+TEST_F(EnvironmentTest, SetValueAdmitsNonStructAggregateStorageLocations) {
+ using namespace ast_matchers;
+
+ std::string Code = "bool X;";
+ auto Unit =
+ tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+ auto &Context = Unit->getASTContext();
+
+ ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+ const auto *Ty = selectFirst<QualType>(
+ "target", match(qualType(builtinType()).bind("target"), Context));
+ ASSERT_TRUE(Ty != nullptr && !Ty->isNull());
+
+ // Create an `AggregateStorageLocation` that is *not* backed by a struct.
+ auto &Loc =
+ Env.takeOwnership(std::make_unique<AggregateStorageLocation>(*Ty));
+ auto &V = Env.takeOwnership(std::make_unique<StructValue>());
+ Env.setValue(Loc, V);
+ EXPECT_EQ(Env.getValue(Loc), &V);
+}
} // namespace
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -374,11 +374,7 @@
if (auto *StructVal = dyn_cast<StructValue>(&Val)) {
auto &AggregateLoc = *cast<AggregateStorageLocation>(&Loc);
-
- const QualType Type = AggregateLoc.getType();
- assert(Type->isStructureOrClassType());
-
- for (const FieldDecl *Field : getObjectFields(Type)) {
+ for (const FieldDecl *Field : getObjectFields(AggregateLoc.getType())) {
assert(Field != nullptr);
StorageLocation &FieldLoc = AggregateLoc.getChild(*Field);
MemberLocToStruct[&FieldLoc] = std::make_pair(StructVal, Field);
Index: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -10,6 +10,7 @@
#include "NoopAnalysis.h"
#include "TestingSupport.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
#include "clang/Analysis/FlowSensitive/Value.h"
#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
#include "gmock/gmock.h"
@@ -20,8 +21,7 @@
using namespace clang;
using namespace dataflow;
-using ::testing::ElementsAre;
-using ::testing::Pair;
+using ::testing::NotNull;
class EnvironmentTest : public ::testing::Test {
DataflowAnalysisContext Context;
@@ -98,4 +98,25 @@
EXPECT_NE(PV, nullptr);
}
+TEST_F(EnvironmentTest, SetValueAdmitsNonStructAggregateStorageLocations) {
+ using namespace ast_matchers;
+
+ std::string Code = "bool X;";
+ auto Unit =
+ tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+ auto &Context = Unit->getASTContext();
+
+ ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+ const auto *Ty = selectFirst<QualType>(
+ "target", match(qualType(builtinType()).bind("target"), Context));
+ ASSERT_TRUE(Ty != nullptr && !Ty->isNull());
+
+ // Create an `AggregateStorageLocation` that is *not* backed by a struct.
+ auto &Loc =
+ Env.takeOwnership(std::make_unique<AggregateStorageLocation>(*Ty));
+ auto &V = Env.takeOwnership(std::make_unique<StructValue>());
+ Env.setValue(Loc, V);
+ EXPECT_EQ(Env.getValue(Loc), &V);
+}
} // namespace
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -374,11 +374,7 @@
if (auto *StructVal = dyn_cast<StructValue>(&Val)) {
auto &AggregateLoc = *cast<AggregateStorageLocation>(&Loc);
-
- const QualType Type = AggregateLoc.getType();
- assert(Type->isStructureOrClassType());
-
- for (const FieldDecl *Field : getObjectFields(Type)) {
+ for (const FieldDecl *Field : getObjectFields(AggregateLoc.getType())) {
assert(Field != nullptr);
StorageLocation &FieldLoc = AggregateLoc.getChild(*Field);
MemberLocToStruct[&FieldLoc] = std::make_pair(StructVal, Field);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
