Skip to content

Commit 71f1932

Browse files
authored
[clang][dataflow] Reland #87320: Propagate locations from result objects to initializers. (#88316)
This relands #87320 and additionally removes the now-unused function `isOriginalRecordConstructor()`, which was causing buildbots to fail.
1 parent 4514608 commit 71f1932

File tree

6 files changed

+590
-305
lines changed

6 files changed

+590
-305
lines changed

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "llvm/ADT/MapVector.h"
3131
#include "llvm/Support/Compiler.h"
3232
#include "llvm/Support/ErrorHandling.h"
33+
#include <memory>
3334
#include <type_traits>
3435
#include <utility>
3536

@@ -344,17 +345,6 @@ class Environment {
344345
/// location of the result object to pass in `this`, even though prvalues are
345346
/// otherwise not associated with storage locations.
346347
///
347-
/// FIXME: Currently, this simply returns a stable storage location for `E`,
348-
/// but this doesn't do the right thing in scenarios like the following:
349-
/// ```
350-
/// MyClass c = some_condition()? MyClass(foo) : MyClass(bar);
351-
/// ```
352-
/// Here, `MyClass(foo)` and `MyClass(bar)` will have two different storage
353-
/// locations, when in fact their storage locations should be the same.
354-
/// Eventually, we want to propagate storage locations from result objects
355-
/// down to the prvalues that initialize them, similar to the way that this is
356-
/// done in Clang's CodeGen.
357-
///
358348
/// Requirements:
359349
/// `E` must be a prvalue of record type.
360350
RecordStorageLocation &
@@ -462,7 +452,13 @@ class Environment {
462452
/// Initializes the fields (including synthetic fields) of `Loc` with values,
463453
/// unless values of the field type are not supported or we hit one of the
464454
/// limits at which we stop producing values.
465-
void initializeFieldsWithValues(RecordStorageLocation &Loc);
455+
/// If `Type` is provided, initializes only those fields that are modeled for
456+
/// `Type`; this is intended for use in cases where `Loc` is a derived type
457+
/// and we only want to initialize the fields of a base type.
458+
void initializeFieldsWithValues(RecordStorageLocation &Loc, QualType Type);
459+
void initializeFieldsWithValues(RecordStorageLocation &Loc) {
460+
initializeFieldsWithValues(Loc, Loc.getType());
461+
}
466462

467463
/// Assigns `Val` as the value of `Loc` in the environment.
468464
void setValue(const StorageLocation &Loc, Value &Val);
@@ -653,6 +649,9 @@ class Environment {
653649
LLVM_DUMP_METHOD void dump(raw_ostream &OS) const;
654650

655651
private:
652+
using PrValueToResultObject =
653+
llvm::DenseMap<const Expr *, RecordStorageLocation *>;
654+
656655
// The copy-constructor is for use in fork() only.
657656
Environment(const Environment &) = default;
658657

@@ -682,8 +681,10 @@ class Environment {
682681
/// Initializes the fields (including synthetic fields) of `Loc` with values,
683682
/// unless values of the field type are not supported or we hit one of the
684683
/// limits at which we stop producing values (controlled by `Visited`,
685-
/// `Depth`, and `CreatedValuesCount`).
686-
void initializeFieldsWithValues(RecordStorageLocation &Loc,
684+
/// `Depth`, and `CreatedValuesCount`). If `Type` is different from
685+
/// `Loc.getType()`, initializes only those fields that are modeled for
686+
/// `Type`.
687+
void initializeFieldsWithValues(RecordStorageLocation &Loc, QualType Type,
687688
llvm::DenseSet<QualType> &Visited, int Depth,
688689
int &CreatedValuesCount);
689690

@@ -702,22 +703,45 @@ class Environment {
702703
/// and functions referenced in `FuncDecl`. `FuncDecl` must have a body.
703704
void initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl);
704705

706+
static PrValueToResultObject
707+
buildResultObjectMap(DataflowAnalysisContext *DACtx,
708+
const FunctionDecl *FuncDecl,
709+
RecordStorageLocation *ThisPointeeLoc,
710+
RecordStorageLocation *LocForRecordReturnVal);
711+
705712
// `DACtx` is not null and not owned by this object.
706713
DataflowAnalysisContext *DACtx;
707714

708-
// FIXME: move the fields `CallStack`, `ReturnVal`, `ReturnLoc` and
709-
// `ThisPointeeLoc` into a separate call-context object, shared between
710-
// environments in the same call.
715+
// FIXME: move the fields `CallStack`, `ResultObjectMap`, `ReturnVal`,
716+
// `ReturnLoc` and `ThisPointeeLoc` into a separate call-context object,
717+
// shared between environments in the same call.
711718
// https://github.com/llvm/llvm-project/issues/59005
712719

713720
// `DeclContext` of the block being analysed if provided.
714721
std::vector<const DeclContext *> CallStack;
715722

716-
// Value returned by the function (if it has non-reference return type).
723+
// Maps from prvalues of record type to their result objects. Shared between
724+
// all environments for the same function.
725+
// FIXME: It's somewhat unsatisfactory that we have to use a `shared_ptr`
726+
// here, though the cost is acceptable: The overhead of a `shared_ptr` is
727+
// incurred when it is copied, and this happens only relatively rarely (when
728+
// we fork the environment). The need for a `shared_ptr` will go away once we
729+
// introduce a shared call-context object (see above).
730+
std::shared_ptr<PrValueToResultObject> ResultObjectMap;
731+
732+
// The following three member variables handle various different types of
733+
// return values.
734+
// - If the return type is not a reference and not a record: Value returned
735+
// by the function.
717736
Value *ReturnVal = nullptr;
718-
// Storage location of the reference returned by the function (if it has
719-
// reference return type).
737+
// - If the return type is a reference: Storage location of the reference
738+
// returned by the function.
720739
StorageLocation *ReturnLoc = nullptr;
740+
// - If the return type is a record or the function being analyzed is a
741+
// constructor: Storage location into which the return value should be
742+
// constructed.
743+
RecordStorageLocation *LocForRecordReturnVal = nullptr;
744+
721745
// The storage location of the `this` pointee. Should only be null if the
722746
// function being analyzed is only a function and not a method.
723747
RecordStorageLocation *ThisPointeeLoc = nullptr;

0 commit comments

Comments
 (0)