[cfe-dev] [PATCH] Implements DR 712 (odr-use, potential results of an expression)
Faisal Vali
faisalv at gmail.com
Wed May 29 11:19:03 PDT 2013
Attached is an attempt to implement DR712 - please see the copied post at
the end of the message for the general implementation strategy, and Richard
smith's initial guidance.
In regards to the testing, I test odr use through lambda capturing - I am
not sure if this is the best way to test it - any guidance will be
appreciated.
In my ODR use visitor:
- am i correct in having the relevant traverses return false? I felt it
was safe to abort, but am not certain.
- do i need the empty VisitStmt?
- do i need the constexpr check of the second operator of (s.*cexpr)?
Also, as a caveat, I feel especially unsure patching codegen - so will
appreciate an especially careful review of those changes.
-I have left some fixme comments in there as I copied code - not sure if
that was the
right thing to do.
- do I need to check for IsLambdaCallOperator in CGExpr.cpp
EmitDeclRefValue?
- should I refactor out the block of code that emits a constant in
EmitDeclRefValue, EmitMemberExpr and EmitPointerToData .. into
CodeGenFunction?
The regression tests seem to work.
Also, the patch (
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130506/079656.html)
was LGTM'd by Doug, anyway we can get that one committed please - thank you!
On Sat, May 18, 2013 at 1:42 PM, Richard Smith <richard at metafoo.co.uk>wrote:
> On Sat, May 18, 2013 at 9:22 AM, Faisal Vali <faisalv at gmail.com> wrote:
>
>> Before I return to working on generic lambdas, I thought it might be
>> useful to nail down DR 712 which reworded the definition of odr-use in
>> terms of the potential results of an expression (in order to avoid
>> odr-using a variable that represents a constant expression whenever we can
>> get away with it). This is relevant to lambdas, because it affects which
>> variables get captured and which don't.
>>
>>
>> Let me review how I think clang currently handles this (pre-712
>> resolution - please correct me where I err). While parsing and
>> semanalyzing expressions, all variables that are usable in constant
>> expressions (and so might not need to be odr-used) are stored in
>> MaybeODRUseExprs (a set container) as DeclRefExprs (DRE). Then, if an
>> Lvalue-to-Rvalue conversion is performed on the DRE, it is removed from the
>> set above (In UpdateMarkingForLValueToRValue). Anything left in that set
>> is marked as odr-used by ActOnFinishFullExpr.
>>
>>
>> OK, with the above in mind, these are my initial thoughts on how to
>> implement it so that not only do we handle those constant expressions that
>> are lvalue-to-rvalue converted, but also those that are discarded, and also
>> handle the conditional expressions and member-access expressions.
>>
>> The relevant hooks/callbacks/functions within clang are:
>> - UpdateMarkingForLValueToRValue
>> - IgnoredValueConversions
>>
>> A function i intend to write is GetPotentialResultsOfExpression: when
>> passed an Expr* E, returns a vector of its potential results as
>> DeclRefExpr's (and MemberExpr's??); this function should follow from 3.2
>> para 2.
>>
>> Now in each hook, GetPotentialResults shall be called, and those
>> expressions shall be removed from the MaybeODRUseExpr.
>>
>
> This seems like the right approach, although you presumably don't actually
> need to build the vector and could just remove the odr-use marker directly.
>
>
>> What are your thoughts on the above preliminary strategy? I have this
>> sense that Member expressions are going to require some more work, but
>> we'll see...
>>
>> Also before I proceed any further, I was hoping to get some more clarity
>> on 3.2 para 2 and para 3 - please see my queries below:
>>
>>
>> Consider the following code:
>>
>>
>> struct S {
>> const int mi;
>> constexpr S(int i) : mi(i) { }
>> };
>>
>> int f(const int& );
>> void g(int);
>> int main() {
>> constexpr S r{5};
>> constexpr S s{r};
>> [](char c) {
>> c = s.mi; // #1 does this odr-use 's'
>>
>
> No. Here, e = 's.mi', ex = 's' (a member of the set of potential results
> of e), and e is subject to an lvalue-to-rvalue conversion.
>
>
>> g(s.mi); // #2 what about this?
>>
>
> Also no, because an lvalue-to-rvalue conversion is applied to s.mi.
>
>
>> s.mi; // #3 and this?
>>
>
> Also no, because this is a discarded-value expression.
>
>
>> c ? s.mi : r.mi; // #4 and this?
>>
>
> Also no.
>
>
>> g( c ? s.mi : r.mi ); // #5
>>
>
> Also no.
>
>
>> };
>> }
>>
>> My intial suspicion was that neither s nor r were odr-used in the lambda
>> (because the compiler can figure out the value of 'mi' at its point of use,
>> and use it or discard it) - but then based on Richard's analysis of the
>> example below where he states that s.operator int() is an odr-use of 's', i
>> am now unsure.
>>
>
> Note that member function calls aren't listed in the rules of 3.2/2. 's'
> is not in the set of potential results of 's.operator int()'.
>
>
>> Here is my attempt at trying to figure this out (e refers to expression,
>> ex as a potentially-evaluated expression, using the language of 3.2 para 2
>> and 3)
>>
>> In #1,
>> the variable 's' appears as a potentially evaluated expression and
>> satisfies the requirements for appearing in a constant expression, and it
>> is an object, and it is an element of the set of potential results of an
>> expression 'e' (s.mi) and the lvalue-to-rvalue conversion is applied to
>> that expression so it is not odr-used? But what if 'e' is deemed to be 's'
>> in 's.mi' - no lvalue-to-rvalue conversion is applied so it should be
>> captured?
>>
>
> The rule says: "x [...] is odr-used unless [...] ex is is an element of
> the set of potential results of an expression e, where [...]"
>
> That is, if there *exists* such an 'e', then 'x' is not odr-used.
>
> Hope that helps!
>
>
>> OK, I'm going to give up on analyzing the rest, because I feel I'm
>> missing something and am unable to think about this clearly.
>>
>> Any help or guidance will be appreciated!
>>
>>
>>
>> On Wed, May 15, 2013 at 3:21 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>>
>>> On Tue, May 14, 2013 at 9:48 PM, Faisal Vali <faisalv at gmail.com> wrote:
>>>
>>>>
>>>>
>>>> While we're on the topic, can i ask you to clarify a few capture and
>>>> constexpr
>>>> questions below:
>>>>
>>>
>>> OK, the relevant context is [basic.def.odr]p2 and p3.
>>>
>>>
>>>> struct S {
>>>> constexpr S() {}
>>>> constexpr operator int() const { return 0; }
>>>> };
>>>> void fooS(S s) { }
>>>>
>>>> void fooInt(int i) { }
>>>>
>>>> void f() {
>>>> constexpr S s {};
>>>> auto L = [] (int x) {
>>>> (void) s; // 1
>>>>
>>>
>>> Here, the 's' expression is a discarded-value expression, and the 's'
>>> variable is in the set of potential results of the 's' expression, and 's'
>>> satisfies the constraints for appearing in a constant expression, so 's' is
>>> not odr-used, so is not captured. We should not require a capture-default
>>> here.
>>>
>>>
>>>> fooS(s); // 2
>>>>
>>>
>>> Here, 's' is implicitly passed to the implicit S::S(const S&)
>>> constructor. This is neither an lvalue-to-rvalue conversion nor a
>>> discarded-value expression, so 's' is odr-used and we have an error due to
>>> the missing capture-default.
>>>
>>>
>>>> fooInt(s); // 3
>>>>
>>>
>>> This is equivalent to 's.operator int()', which again odr-uses 's', so
>>> requires a capture-default.
>>>
>>>
>>>> };
>>>> }
>>>>
>>>> Should the above be ok in spite of L not having a default capture?
>>>> Currently clang errors on all of them individually - is that the right
>>>> behavior?
>>>>
>>>> Also, how would you want me to start submitting patches for commit - do
>>>> you want
>>>> me to break up the patch into smaller patches? - and if so, do you have
>>>> any thoughts
>>>> on how I might want to break up the functionality per patch?
>>>>
>>>
>>> Smaller patches are definitely better, if you can decompose the
>>> functionality into coherent smaller chunks. There are some hints on how to
>>> decompose the functionality here (but this division may or may not work for
>>> you):
>>>
>>>
>>> http://clang.llvm.org/docs/InternalsManual.html#how-to-add-an-expression-or-statement
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130529/c5e37c11/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DR712-odr-use-intial-submit.patch
Type: application/octet-stream
Size: 17131 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130529/c5e37c11/attachment.obj>
More information about the cfe-dev
mailing list