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 subscribers: cfe-commits, wangpc. Herald added a project: clang.
This consolidates the code used in various places to initialize objects (usually for variables) into one central location. It will also help reduce the number of changes needed when we make the upcoming API changes to `AggregateStorageLocation` and `StructValue`. Depends On D155074 <https://reviews.llvm.org/D155074> Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D155075 Files: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp clang/lib/Analysis/FlowSensitive/Transfer.cpp
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -208,62 +208,15 @@ if (D.hasGlobalStorage()) return; - if (D.getType()->isReferenceType()) { - // If this is the holding variable for a `BindingDecl`, we may already - // have a storage location set up -- so check. (See also explanation below - // where we process the `BindingDecl`.) - if (Env.getStorageLocation(D) == nullptr) { - const Expr *InitExpr = D.getInit(); - assert(InitExpr != nullptr); - if (auto *InitExprLoc = - Env.getStorageLocation(*InitExpr, SkipPast::Reference)) { - Env.setStorageLocation(D, *InitExprLoc); - } else { - // Even though we have an initializer, we might not get an - // InitExprLoc, for example if the InitExpr is a CallExpr for which we - // don't have a function body. In this case, we just invent a storage - // location and value -- it's the best we can do. - StorageLocation &Loc = - Env.createStorageLocation(D.getType().getNonReferenceType()); - Env.setStorageLocation(D, Loc); - if (Value *Val = Env.createValue(D.getType().getNonReferenceType())) - Env.setValue(Loc, *Val); - } - } - } else { - // Not a reference type. + // If this is the holding variable for a `BindingDecl`, we may already + // have a storage location set up -- so check. (See also explanation below + // where we process the `BindingDecl`.) + if (D.getType()->isReferenceType() && Env.getStorageLocation(D) != nullptr) + return; - assert(Env.getStorageLocation(D) == nullptr); - StorageLocation &Loc = Env.createStorageLocation(D); - Env.setStorageLocation(D, Loc); + assert(Env.getStorageLocation(D) == nullptr); - const Expr *InitExpr = D.getInit(); - if (InitExpr == nullptr) { - // No initializer expression - associate `Loc` with a new value. - if (Value *Val = Env.createValue(D.getType())) - Env.setValue(Loc, *Val); - return; - } - - if (auto *InitExprVal = Env.getValueStrict(*InitExpr)) - Env.setValue(Loc, *InitExprVal); - - if (Env.getValue(Loc) == nullptr) { - // We arrive here in (the few) cases where an expression is - // intentionally "uninterpreted". There are two ways to handle this - // situation: propagate the status, so that uninterpreted initializers - // result in uninterpreted variables, or provide a default value. We - // choose the latter so that later refinements of the variable can be - // used for reasoning about the surrounding code. - // - // FIXME. If and when we interpret all language cases, change this to - // assert that `InitExpr` is interpreted, rather than supplying a - // default value (assuming we don't update the environment API to return - // references). - if (Value *Val = Env.createValue(D.getType())) - Env.setValue(Loc, *Val); - } - } + Env.setStorageLocation(D, Env.createObject(D)); // `DecompositionDecl` must be handled after we've interpreted the loc // itself, because the binding expression refers back to the Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -256,10 +256,8 @@ for (const VarDecl *D : Vars) { if (getStorageLocation(*D) != nullptr) continue; - auto &Loc = createStorageLocation(D->getType().getNonReferenceType()); - setStorageLocation(*D, Loc); - if (auto *Val = createValue(D->getType().getNonReferenceType())) - setValue(Loc, *Val); + + setStorageLocation(*D, createObject(*D)); } for (const FunctionDecl *FD : Funcs) { @@ -292,16 +290,7 @@ for (const auto *ParamDecl : FuncDecl->parameters()) { assert(ParamDecl != nullptr); - // References aren't objects, so the reference itself doesn't have a - // storage location. Instead, the storage location for a reference refers - // directly to an object of the referenced type -- so strip off any - // reference from the type. - auto &ParamLoc = - createStorageLocation(ParamDecl->getType().getNonReferenceType()); - setStorageLocation(*ParamDecl, ParamLoc); - if (Value *ParamVal = - createValue(ParamDecl->getType().getNonReferenceType())) - setValue(ParamLoc, *ParamVal); + setStorageLocation(*ParamDecl, createObject(*ParamDecl, nullptr)); } } @@ -378,27 +367,8 @@ // overloaded operators implemented as member functions, and parameter packs. for (unsigned ArgIndex = 0; ArgIndex < Args.size(); ++ParamIt, ++ArgIndex) { assert(ParamIt != FuncDecl->param_end()); - - const Expr *Arg = Args[ArgIndex]; - auto *ArgLoc = getStorageLocation(*Arg, SkipPast::Reference); - if (ArgLoc == nullptr) - continue; - const VarDecl *Param = *ParamIt; - - QualType ParamType = Param->getType(); - if (ParamType->isReferenceType()) { - setStorageLocation(*Param, *ArgLoc); - } else { - auto &Loc = createStorageLocation(*Param); - setStorageLocation(*Param, Loc); - - if (auto *ArgVal = getValue(*ArgLoc)) { - setValue(Loc, *ArgVal); - } else if (Value *Val = createValue(ParamType)) { - setValue(Loc, *Val); - } - } + setStorageLocation(*Param, createObject(*Param, Args[ArgIndex])); } } @@ -868,6 +838,54 @@ return nullptr; } +StorageLocation &Environment::createObjectInternal(const VarDecl *D, + QualType Ty, + const Expr *InitExpr) { + if (Ty->isReferenceType()) { + // Although variables of reference type always need to be initialized, it + // can happen that we can't see the initializer, so `InitExpr` may still + // be null. + if (InitExpr) { + if (auto *InitExprLoc = + getStorageLocation(*InitExpr, SkipPast::Reference)) + return *InitExprLoc; + } + + // Even though we have an initializer, we might not get an + // InitExprLoc, for example if the InitExpr is a CallExpr for which we + // don't have a function body. In this case, we just invent a storage + // location and value -- it's the best we can do. + return createObjectInternal(D, Ty.getNonReferenceType(), nullptr); + } + + Value *Val = nullptr; + if (InitExpr) + // In the (few) cases where an expression is intentionally + // "uninterpreted", `InitExpr` is not associated with a value. There are + // two ways to handle this situation: propagate the status, so that + // uninterpreted initializers result in uninterpreted variables, or + // provide a default value. We choose the latter so that later refinements + // of the variable can be used for reasoning about the surrounding code. + // For this reason, we let this case be handled by the `createValue()` + // call below. + // + // FIXME. If and when we interpret all language cases, change this to + // assert that `InitExpr` is interpreted, rather than supplying a + // default value (assuming we don't update the environment API to return + // references). + Val = getValueStrict(*InitExpr); + if (!Val) + Val = createValue(Ty); + + StorageLocation &Loc = + D ? createStorageLocation(*D) : createStorageLocation(Ty); + + if (Val) + setValue(Loc, *Val); + + return Loc; +} + StorageLocation &Environment::skip(StorageLocation &Loc, SkipPast SP) const { switch (SP) { case SkipPast::None: Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp +++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp @@ -72,7 +72,7 @@ DataflowAnalysisContext::getStableStorageLocation(const VarDecl &D) { if (auto *Loc = getStorageLocation(D)) return *Loc; - auto &Loc = createStorageLocation(D.getType()); + auto &Loc = createStorageLocation(D.getType().getNonReferenceType()); setStorageLocation(D, Loc); return Loc; } Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h =================================================================== --- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -392,6 +392,32 @@ /// `Type` must not be null. Value *createValue(QualType Type); + /// Creates an object (i.e. a storage location with an associated value) of + /// type `Ty`. If `InitExpr` is non-null and has a value associated with it, + /// initializes the object with this value. Otherwise, initializes the object + /// with a value created using `createValue()`. + StorageLocation &createObject(QualType Ty, const Expr *InitExpr = nullptr) { + return createObjectInternal(nullptr, Ty, InitExpr); + } + + /// Creates an object for the variable declaration `D`. If `D` has an + /// initializer and this initializer is associated with a value, initializes + /// the object with this value. Otherwise, initializes the object with a + /// value created using `createValue()`. Uses the storage location returned by + /// `DataflowAnalysisContext::getStableStorageLocation(D)`. + StorageLocation &createObject(const VarDecl &D) { + return createObjectInternal(&D, D.getType(), D.getInit()); + } + + /// Creates an object for the variable declaration `D`. If `InitExpr` is + /// non-null and has a value associated with it, initializes the object with + /// this value. Otherwise, initializes the object with a value created using + /// `createValue()`. Uses the storage location returned by + /// `DataflowAnalysisContext::getStableStorageLocation(D)`. + StorageLocation &createObject(const VarDecl &D, const Expr *InitExpr) { + return createObjectInternal(&D, D.getType(), InitExpr); + } + /// Assigns `Val` as the value of `Loc` in the environment. void setValue(const StorageLocation &Loc, Value &Val); @@ -580,6 +606,9 @@ llvm::DenseSet<QualType> &Visited, int Depth, int &CreatedValuesCount); + StorageLocation &createObjectInternal(const VarDecl *D, QualType Ty, + const Expr *InitExpr); + StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const; const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits