<div dir="ltr">On Mon, Jul 1, 2013 at 7:05 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 Mon, Jul 1, 2013 at 3:53 PM, Eli Friedman <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>> wrote:<br>
> On Mon, Jul 1, 2013 at 3:42 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
>><br>
>> On Sun, Jun 30, 2013 at 6:39 PM, Eli Friedman <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>><br>
>> wrote:<br>
>> > On Tue, Jun 4, 2013 at 5:46 PM, Richard Smith<br>
>> > <<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<br>
>> >> 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<br>
>> >> 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>
>> >><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>
>> >> ==============================================================================<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<br>
>> >> 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<br>
>> >> 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<br>
>> > 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>
>> 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>
><br>
><br>
> This might be a bit crazy, but have you considered extending APValue?<br>
> Specifically, we could add a new kind of LValue whose base is a temporary<br>
> with the given value. That way, when CGExprConstant is doing evaluation,<br>
> the value is right there, instead of messing with a global table.<br>
<br>
</div></div>We'd need all the LValues pointing at the same temporary to share the<br>
same value; that risks misbehavior if we evaluate the initializer for<br>
a variable multiple times for whatever reason, </blockquote><div><br></div><div>I'm not sure why this is an issue; as long as the LValue in question represents a reference temporary with the same mangled name and the same value, it doesn't really matter how many instances of it exist. </div>
<div> </div><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">but I think it can be<br>
made to work. The semantic model feels a bit strange; the initial<br>
value for a global temporary really is a global thing, and shouldn't<br>
depend on how you constructed the LValue referring to it.<br>
</blockquote></div><br></div><div class="gmail_extra">I think that's just the nature of the C++1y model.</div><div class="gmail_extra"><br></div><div class="gmail_extra">-Eli</div></div>