[PATCH] D156788: [clang][dataflow] Rename `AggregateStorageLocation` to `RecordStorageLocation` and `StructValue` to `RecordValue`.

[Martin Böhme via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ecdbe3855a8: [clang][dataflow] Rename 
`AggregateStorageLocation` to `RecordStorageLocation`… (authored by mboehme).

Changed prior to commit:
  https://reviews.llvm.org/D156788?vs=546021=546201#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156788/new/

https://reviews.llvm.org/D156788

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/RecordOps.h
  clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/RecordOps.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
  clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -415,7 +415,7 @@
 if (const auto *E = selectFirst(
 "call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S,
   getASTContext( {
-  cast(Env.getValue(*E))
+  cast(Env.getValue(*E))
   ->setProperty("is_set", Env.getBoolLiteralValue(false));
 } else if (const auto *E = selectFirst(
"call", match(cxxMemberCallExpr(callee(cxxMethodDecl(ofClass(
@@ -423,9 +423,9 @@
  .bind("call"),
  *S, getASTContext( {
   auto  =
-  *cast(getImplicitObjectLocation(*E, Env));
+  *cast(getImplicitObjectLocation(*E, Env));
 
-  refreshStructValue(ObjectLoc, Env)
+  refreshRecordValue(ObjectLoc, Env)
   .setProperty("is_set", Env.getBoolLiteralValue(true));
 }
   }
@@ -572,7 +572,7 @@
 *S, getASTContext());
 if (const auto *E = selectFirst(
 "construct", Matches)) {
-  cast(Env.getValue(*E))
+  cast(Env.getValue(*E))
   ->setProperty("has_value", Env.getBoolLiteralValue(false));
 } else if (const auto *E =
selectFirst("operator", Matches)) {
@@ -580,7 +580,7 @@
   auto *Object = E->getArg(0);
   assert(Object != nullptr);
 
-  refreshStructValue(*Object, Env)
+  refreshRecordValue(*Object, Env)
   .setProperty("has_value", Env.getBoolLiteralValue(true));
 }
   }
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -164,10 +164,10 @@
 auto *FooValue = dyn_cast_or_null(Env.getValue(*FooDecl));
 ASSERT_THAT(FooValue, NotNull());
 
-EXPECT_TRUE(isa(FooValue->getPointeeLoc()));
+EXPECT_TRUE(isa(FooValue->getPointeeLoc()));
 auto *FooPointeeValue = Env.getValue(FooValue->getPointeeLoc());
 ASSERT_THAT(FooPointeeValue, NotNull());
-EXPECT_TRUE(isa(FooPointeeValue));
+EXPECT_TRUE(isa(FooPointeeValue));
   });
 }
 
@@ -215,9 +215,9 @@
 ASSERT_THAT(UnmodeledDecl, NotNull());
 
 const auto *FooLoc =
-cast(Env.getStorageLocation(*FooDecl));
+cast(Env.getStorageLocation(*FooDecl));
 const auto *UnmodeledLoc = FooLoc->getChild(*UnmodeledDecl);
-ASSERT_TRUE(isa(UnmodeledLoc));
+ASSERT_TRUE(isa(UnmodeledLoc));
 EXPECT_THAT(Env.getValue(*UnmodeledLoc), IsNull());
 
 const ValueDecl *ZabDecl = findValueDecl(ASTCtx, "Zab");
@@ -262,7 +262,7 @@
 ASSERT_THAT(BarDecl, NotNull());
 
 const auto *FooLoc =
-cast(Env.getStorageLocation(*FooDecl));
+cast(Env.getStorageLocation(*FooDecl));
 EXPECT_TRUE(isa(getFieldValue(FooLoc, *BarDecl, Env)));
   });
 }
@@ -305,7 +305,7 @@
 ASSERT_THAT(BarDecl, NotNull());
 
 const auto *FooLoc =
-cast(Env.getStorageLocation(*FooDecl));
+cast(Env.getStorageLocation(*FooDecl));
 EXPECT_TRUE(isa(getFieldValue(FooLoc, *BarDecl, Env)));
   });
 }

[PATCH] D156788: [clang][dataflow] Rename `AggregateStorageLocation` to `RecordStorageLocation` and `StructValue` to `RecordValue`.

[Martin Böhme via Phabricator via cfe-commits]

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Both of these constructs are used to represent structs, classes, and unions; 
Clang uses the collective term "record" for these.

- The term "aggregate" in `AggregateStorageLocation` implies that, at some 
point, the intention may have been to use it also for arrays, but it don't 
think it's possible to use it for arrays. Records and arrays are very different 
and therefore need to be modeled differently. Records have a fixed set of named 
fields, which can have different type; arrays have a variable number of 
elements, but they all have the same type.

- Futhermore, "aggregate" has a very specific meaning in C++ 
(https://en.cppreference.com/w/cpp/language/aggregate_initialization). 
Aggregates of class type may not have any user-declared or inherited 
constructors, no private or protected non-static data members, no virtual 
member functions, and so on, but we use `AggregateStorageLocations` to model 
all objects of class type.

In addition, for consistency, we also rename the following:

- `getAggregateLoc()` (in `RecordValue`, formerly known as `StructValue`) to 
simply `getLoc()`.

- `refreshStructValue()` to `refreshRecordValue()`

We keep the old names around as deprecated synonyms to enable clients to be
migrated to the new names.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156788

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/RecordOps.h
  clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/RecordOps.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
  clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -415,7 +415,7 @@
 if (const auto *E = selectFirst(
 "call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S,
   getASTContext( {
-  cast(Env.getValue(*E))
+  cast(Env.getValue(*E))
   ->setProperty("is_set", Env.getBoolLiteralValue(false));
 } else if (const auto *E = selectFirst(
"call", match(cxxMemberCallExpr(callee(cxxMethodDecl(ofClass(
@@ -423,9 +423,9 @@
  .bind("call"),
  *S, getASTContext( {
   auto  =
-  *cast(getImplicitObjectLocation(*E, Env));
+  *cast(getImplicitObjectLocation(*E, Env));
 
-  refreshStructValue(ObjectLoc, Env)
+  refreshRecordValue(ObjectLoc, Env)
   .setProperty("is_set", Env.getBoolLiteralValue(true));
 }
   }
@@ -572,7 +572,7 @@
 *S, getASTContext());
 if (const auto *E = selectFirst(
 "construct", Matches)) {
-  cast(Env.getValue(*E))
+  cast(Env.getValue(*E))
   ->setProperty("has_value", Env.getBoolLiteralValue(false));
 } else if (const auto *E =
selectFirst("operator", Matches)) {
@@ -580,7 +580,7 @@
   auto *Object = E->getArg(0);
   assert(Object != nullptr);
 
-  refreshStructValue(*Object, Env)
+  refreshRecordValue(*Object, Env)
   .setProperty("has_value", Env.getBoolLiteralValue(true));
 }
   }
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -164,10 +164,10 @@
 auto *FooValue = dyn_cast_or_null(Env.getValue(*FooDecl));
 ASSERT_THAT(FooValue, NotNull());
 
-EXPECT_TRUE(isa(FooValue->getPointeeLoc()));
+EXPECT_TRUE(isa(FooValue->getPointeeLoc()));
 auto *FooPointeeValue = Env.getValue(FooValue->getPointeeLoc());
 ASSERT_THAT(FooPointeeValue,