Skip to content

Commit 7549b45

Browse files
authored
Revert "[clang][dataflow] Propagate locations from result objects to initializers." (#88315)
Reverts #87320 This is causing buildbots to fail because `isOriginalRecordConstructor()` is now unused.
1 parent e3ef461 commit 7549b45

File tree

6 files changed

+283
-590
lines changed

6 files changed

+283
-590
lines changed

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

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

@@ -345,6 +344,17 @@ class Environment {
345344
/// location of the result object to pass in `this`, even though prvalues are
346345
/// otherwise not associated with storage locations.
347346
///
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+
///
348358
/// Requirements:
349359
/// `E` must be a prvalue of record type.
350360
RecordStorageLocation &
@@ -452,13 +462,7 @@ class Environment {
452462
/// Initializes the fields (including synthetic fields) of `Loc` with values,
453463
/// unless values of the field type are not supported or we hit one of the
454464
/// limits at which we stop producing values.
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-
}
465+
void initializeFieldsWithValues(RecordStorageLocation &Loc);
462466

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

651655
private:
652-
using PrValueToResultObject =
653-
llvm::DenseMap<const Expr *, RecordStorageLocation *>;
654-
655656
// The copy-constructor is for use in fork() only.
656657
Environment(const Environment &) = default;
657658

@@ -681,10 +682,8 @@ class Environment {
681682
/// Initializes the fields (including synthetic fields) of `Loc` with values,
682683
/// unless values of the field type are not supported or we hit one of the
683684
/// limits at which we stop producing values (controlled by `Visited`,
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,
685+
/// `Depth`, and `CreatedValuesCount`).
686+
void initializeFieldsWithValues(RecordStorageLocation &Loc,
688687
llvm::DenseSet<QualType> &Visited, int Depth,
689688
int &CreatedValuesCount);
690689

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

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

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.
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.
718711
// https://github.com/llvm/llvm-project/issues/59005
719712

720713
// `DeclContext` of the block being analysed if provided.
721714
std::vector<const DeclContext *> CallStack;
722715

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.
716+
// Value returned by the function (if it has non-reference return type).
736717
Value *ReturnVal = nullptr;
737-
// - If the return type is a reference: Storage location of the reference
738-
// returned by the function.
718+
// Storage location of the reference returned by the function (if it has
719+
// reference return type).
739720
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-
745721
// The storage location of the `this` pointee. Should only be null if the
746722
// function being analyzed is only a function and not a method.
747723
RecordStorageLocation *ThisPointeeLoc = nullptr;

0 commit comments

Comments
 (0)