r183283 - Model temporary lifetime-extension explicitly in the AST. Use this model to

[Eli Friedman]

On Mon, Jul 1, 2013 at 3:42 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> On Sun, Jun 30, 2013 at 6:39 PM, Eli Friedman <eli.friedman at gmail.com>
> wrote:
> > On Tue, Jun 4, 2013 at 5:46 PM, Richard Smith <
> richard-llvm at metafoo.co.uk>
> > wrote:
> >>
> >> Author: rsmith
> >> Date: Tue Jun  4 19:46:14 2013
> >> New Revision: 183283
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=183283&view=rev
> >> Log:
> >> Model temporary lifetime-extension explicitly in the AST. Use this model
> >> to
> >> handle temporaries which have been lifetime-extended to static storage
> >> duration
> >> within constant expressions. This correctly handles nested lifetime
> >> extension
> >> (through reference members of aggregates in aggregate initializers) but
> >> non-constant-expression emission hasn't yet been updated to do the same.
> >>
> >> Modified:
> >>     cfe/trunk/include/clang/AST/ASTContext.h
> >>     cfe/trunk/include/clang/AST/Decl.h
> >>     cfe/trunk/include/clang/AST/ExprCXX.h
> >>     cfe/trunk/include/clang/AST/Stmt.h
> >>     cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
> >>     cfe/trunk/include/clang/Basic/Specifiers.h
> >>     cfe/trunk/include/clang/Sema/Initialization.h
> >>     cfe/trunk/lib/AST/ASTContext.cpp
> >>     cfe/trunk/lib/AST/ASTDumper.cpp
> >>     cfe/trunk/lib/AST/ExprConstant.cpp
> >>     cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> >>     cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> >>     cfe/trunk/lib/CodeGen/CodeGenModule.h
> >>     cfe/trunk/lib/Sema/SemaExpr.cpp
> >>     cfe/trunk/lib/Sema/SemaInit.cpp
> >>     cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
> >>     cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
> >>     cfe/trunk/test/CodeGenCXX/const-init-cxx11.cpp
> >>     cfe/trunk/test/CodeGenCXX/const-init-cxx1y.cpp
> >>     cfe/trunk/test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
> >>     cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
> >>     cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp
> >>
> >> Modified: cfe/trunk/include/clang/AST/ASTContext.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=183283&r1=183282&r2=183283&view=diff
> >>
> >>
> ==============================================================================
> >> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
> >> +++ cfe/trunk/include/clang/AST/ASTContext.h Tue Jun  4 19:46:14 2013
> >> @@ -55,6 +55,7 @@ namespace clang {
> >>    class ExternalASTSource;
> >>    class ASTMutationListener;
> >>    class IdentifierTable;
> >> +  class MaterializeTemporaryExpr;
> >>    class SelectorTable;
> >>    class TargetInfo;
> >>    class CXXABI;
> >> @@ -163,6 +164,11 @@ class ASTContext : public RefCountedBase
> >>    llvm::DenseMap<const FunctionDecl*, FunctionDecl*>
> >>      ClassScopeSpecializationPattern;
> >>
> >> +  /// \brief Mapping from materialized temporaries with static storage
> >> duration
> >> +  /// that appear in constant initializers to their evaluated values.
> >> +  llvm::DenseMap<const MaterializeTemporaryExpr*, APValue>
> >> +    MaterializedTemporaryValues;
> >> +
> >>    /// \brief Representation of a "canonical" template template
> parameter
> >> that
> >>    /// is used in canonical template names.
> >>    class CanonicalTemplateTemplateParm : public llvm::FoldingSetNode {
> >> @@ -2117,7 +2123,12 @@ public:
> >>    /// \brief Used by ParmVarDecl to retrieve on the side the
> >>    /// index of the parameter when it exceeds the size of the normal
> >> bitfield.
> >>    unsigned getParameterIndex(const ParmVarDecl *D) const;
> >> -
> >> +
> >> +  /// \brief Get the storage for the constant value of a materialized
> >> temporary
> >> +  /// of static storage duration.
> >> +  APValue *getMaterializedTemporaryValue(const MaterializeTemporaryExpr
> >> *E,
> >> +                                         bool MayCreate);
> >>
> >
> > Having a global table of temporary values seems really fragile given the
> > ability to modify the values of the temporaries in C++1y.
> > (const-init-cxx1y.cpp happens to work given the precise order we call
> into
> > the constant evaluator, but could easily break if we change how we do
> > CodeGen.)  Have you given any thought to this?
>
> Yes; I'm also not entirely happy with the current state of affairs.
>
> The best approach I've thought of to improve the situation is to
> ensure the ASTContext storage is only used when evaluating the
> initializer for the variable that lifetime-extends the temporary (that
> is, refuse to evaluate the temporary otherwise, and clear out all the
> storage for the variable's extended temporaries if the overall
> evaluation fails). It's not possible for us to emit a reference to a
> MaterializeTemporaryExpr from outside the variable's initializer if
> we've not succeeded in evaluating its initializer, so we shouldn't
> emit it with the "wrong" initializer.
>
> In addition, we should probably move the code that emits the constant
> in the ASTContext storage from GetAddrOfGlobalTemporary into
> CGExprConstant, so that we can more aggressively assert that we get
> this right.
>
> This would still be a little untidy. Do you have a better suggestion?
>

This might be a bit crazy, but have you considered extending APValue?
 Specifically, we could add a new kind of LValue whose base is a temporary
with the given value.  That way, when CGExprConstant is doing evaluation,
the value is right there, instead of messing with a global table.

-Eli
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130701/053f558c/attachment.html>