Re: [PATCH] D28467: [Sema] Add warning for unused lambda captures

[Aaron Ballman via cfe-commits]

On Thu, Jan 19, 2017 at 8:45 AM, Malcolm Parsons
 wrote:
> On 19 January 2017 at 13:16, Aaron Ballman  wrote:
>> I wasn't thinking about that kind of odr-unuse when reviewing your
>> patch, so I am starting to think that perhaps it's not worth
>> distinguishing unevaluated contexts or not in the diagnostic. :-( If
>> we could do it, then great (we seem to be able to do it for regular
>> variable use: http://coliru.stacked-crooked.com/a/4bde9b5daf48956a),
>> but if not, then I think we should just go back to the original
>> wording that says it's not required to be captured (in all cases, not
>> distinguishing odr-use) and put in a FIXME with the test cases that
>> could have an improved diagnostic (including the test case talked
>> about here, which we should add). What do you think?
>
> The warning can distinguish:
> * not looked up
> * looked up
> * looked up and used
>
> It doesn't know why a variable was looked up but not used.

Drat, though I don't think this diagnostic warrants making changes to that.

>
> You suggested the wording "not required to be captured for this use"
> earlier in this thread; is that better?

Yes, I think that wording makes sense.

~Aaron

>
> --
> Malcolm Parsons
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D28467: [Sema] Add warning for unused lambda captures

[Malcolm Parsons via cfe-commits]

On 19 January 2017 at 13:16, Aaron Ballman  wrote:
> I wasn't thinking about that kind of odr-unuse when reviewing your
> patch, so I am starting to think that perhaps it's not worth
> distinguishing unevaluated contexts or not in the diagnostic. :-( If
> we could do it, then great (we seem to be able to do it for regular
> variable use: http://coliru.stacked-crooked.com/a/4bde9b5daf48956a),
> but if not, then I think we should just go back to the original
> wording that says it's not required to be captured (in all cases, not
> distinguishing odr-use) and put in a FIXME with the test cases that
> could have an improved diagnostic (including the test case talked
> about here, which we should add). What do you think?

The warning can distinguish:
* not looked up
* looked up
* looked up and used

It doesn't know why a variable was looked up but not used.

You suggested the wording "not required to be captured for this use"
earlier in this thread; is that better?

-- 
Malcolm Parsons
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D28467: [Sema] Add warning for unused lambda captures

[Aaron Ballman via cfe-commits]

On Thu, Jan 19, 2017 at 7:49 AM, Aaron Ballman  wrote:
> On Thu, Jan 19, 2017 at 4:37 AM, Malcolm Parsons
>  wrote:
>> On 19 January 2017 at 03:47, Aaron Ballman  wrote:
>>> It is not used in an unevaluated context -- that is a bug.
>>
>> It is an evaluated expression, but is it odr-used?
>>
>> C++14 [basic.def.odr] p3:
>>
>> A variable x whose name appears as a potentially-evaluated expression
>> ex is odr-used by ex unless applying the lvalue-to-rvalue conversion
>> (4.1) to x yields a constant expression (5.20) that does not invoke
>> any nontrivial functions and, if x is an object, ex is an element of
>> the set of potential results of an expression e, where either the
>> lvalue-to-rvalue conversion (4.1) is applied to e, or e is a
>> discarded-value expression (Clause 5). ...
>>
>> 5.20 [expr.const] p3:
>>
>> An integral constant expression is an expression of integral or
>> unscoped enumeration type, implicitly converted to a prvalue, where
>> the converted expression is a core constant expression. [ Note: Such
>> expressions may be used as array bounds (8.3.4, 5.3.4), as bit-field
>> lengths (9.6), as enumerator initializers if the underlying type is
>> not fixed (7.2), and as alignments (7.6.2). — end note ]
>>
>> I read that as kDelta is not odr-used.
>>
>> GCC and ICC don't require kDelta to be captured either.
>
> You are correct, it is not an odr use. MSVC is wrong to require the capture.

I wasn't thinking about that kind of odr-unuse when reviewing your
patch, so I am starting to think that perhaps it's not worth
distinguishing unevaluated contexts or not in the diagnostic. :-( If
we could do it, then great (we seem to be able to do it for regular
variable use: http://coliru.stacked-crooked.com/a/4bde9b5daf48956a),
but if not, then I think we should just go back to the original
wording that says it's not required to be captured (in all cases, not
distinguishing odr-use) and put in a FIXME with the test cases that
could have an improved diagnostic (including the test case talked
about here, which we should add). What do you think?

~Aaron

>
> ~Aaron
>
>>
>> --
>> Malcolm Parsons
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D28467: [Sema] Add warning for unused lambda captures

[Malcolm Parsons via cfe-commits]

On 19 January 2017 at 12:49, Aaron Ballman  wrote:
> You are correct, it is not an odr use. MSVC is wrong to require the capture.

Should the warning be rephrased?

-- 
Malcolm Parsons
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D28467: [Sema] Add warning for unused lambda captures

[Aaron Ballman via cfe-commits]

On Thu, Jan 19, 2017 at 4:37 AM, Malcolm Parsons
 wrote:
> On 19 January 2017 at 03:47, Aaron Ballman  wrote:
>> It is not used in an unevaluated context -- that is a bug.
>
> It is an evaluated expression, but is it odr-used?
>
> C++14 [basic.def.odr] p3:
>
> A variable x whose name appears as a potentially-evaluated expression
> ex is odr-used by ex unless applying the lvalue-to-rvalue conversion
> (4.1) to x yields a constant expression (5.20) that does not invoke
> any nontrivial functions and, if x is an object, ex is an element of
> the set of potential results of an expression e, where either the
> lvalue-to-rvalue conversion (4.1) is applied to e, or e is a
> discarded-value expression (Clause 5). ...
>
> 5.20 [expr.const] p3:
>
> An integral constant expression is an expression of integral or
> unscoped enumeration type, implicitly converted to a prvalue, where
> the converted expression is a core constant expression. [ Note: Such
> expressions may be used as array bounds (8.3.4, 5.3.4), as bit-field
> lengths (9.6), as enumerator initializers if the underlying type is
> not fixed (7.2), and as alignments (7.6.2). — end note ]
>
> I read that as kDelta is not odr-used.
>
> GCC and ICC don't require kDelta to be captured either.

You are correct, it is not an odr use. MSVC is wrong to require the capture.

~Aaron

>
> --
> Malcolm Parsons
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D28467: [Sema] Add warning for unused lambda captures

[Malcolm Parsons via cfe-commits]

On 19 January 2017 at 03:47, Aaron Ballman  wrote:
> It is not used in an unevaluated context -- that is a bug.

It is an evaluated expression, but is it odr-used?

C++14 [basic.def.odr] p3:

A variable x whose name appears as a potentially-evaluated expression
ex is odr-used by ex unless applying the lvalue-to-rvalue conversion
(4.1) to x yields a constant expression (5.20) that does not invoke
any nontrivial functions and, if x is an object, ex is an element of
the set of potential results of an expression e, where either the
lvalue-to-rvalue conversion (4.1) is applied to e, or e is a
discarded-value expression (Clause 5). ...

5.20 [expr.const] p3:

An integral constant expression is an expression of integral or
unscoped enumeration type, implicitly converted to a prvalue, where
the converted expression is a core constant expression. [ Note: Such
expressions may be used as array bounds (8.3.4, 5.3.4), as bit-field
lengths (9.6), as enumerator initializers if the underlying type is
not fixed (7.2), and as alignments (7.6.2). — end note ]

I read that as kDelta is not odr-used.

GCC and ICC don't require kDelta to be captured either.

-- 
Malcolm Parsons
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D28467: [Sema] Add warning for unused lambda captures

[Aaron Ballman via cfe-commits]

On Jan 18, 2017 7:34 PM, "Akira Hatanaka via Phabricator" <
revi...@reviews.llvm.org> wrote:

ahatanak added a comment.

In https://reviews.llvm.org/D28467#649861, @krasin wrote:

> This change makes Clang hardly incompatible with MSVC++. Consider the
following program:
>
>   #include 
>
>   int main(void) {
> const int kDelta = 1001;
> auto g = [kDelta](int i)
>  {
>printf("%d\n", i % kDelta);
>  };
> g(2);
>   }
>
>
> Clang will warn about the unused lambda capture:
>
>   $ clang++ lala.cc -o lala -std=c++14 -Wall -Werror && ./lala
>   lala.cc:5:13: error: lambda capture 'kDelta' is not required to be
captured for use in an unevaluated context [-Werror,-Wunused-lambda-capture]
> auto g = [kDelta](int i)
>   ^
>   1 error generated.
>


Is kDelta considered to be used in an unevaluated context here? I thought
unevaluated context in c++ means the expression is used as operands of
operators such as typeid and decltype.


It is not used in an unevaluated context -- that is a bug.

~Aaron



Repository:
  rL LLVM

https://reviews.llvm.org/D28467
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits