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