Incorrect result from DeclRefExpr::refersToEnclosingVariableCapture?

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Incorrect result from DeclRefExpr::refersToEnclosingVariableCapture?

Don Hinton via cfe-dev
Hi everyone,

We've been working on upgrading our frontend from clang 3.4 to 3.7 and have run
into an issue where 'DeclRefExpr::refersToEnclosingVariableOrCapture' seems to be
returning the incorrect result. Here's a test case:

  int fn1() {
    const int i = 0;
    return [] { return i; }();
  }

Calling 'refersToEnclosingVariableOrCapture' on the 'DeclRefExpr' for 'i' within
the lambda body yields false. In clang 3.4, the equivalent 'refersToCapturedVariable'
method yields true. It looks this behavior was probably introduced by r224323,
which changed 'Sema::BuildDeclRefExpr' to determine this via 'tryCaptureVariable'.

So with that background, I have a few questions:

- Is this code even valid? It seems like it shouldn't be since there's no
  capture-default or capture-list, but both clang and GCC accept it.
- Assuming the code is valid, is this a bug in clang?
- If not, is there some other way to detect when a DeclRefExpr in a lambda body
  refers to a declaration from the enclosing scope?

Incidentally, GCC accepts this similar test case, while clang does not:

  void fn1() {
    const int i = 0;
    [] { i; }();
  }

Thanks!
- Tim
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Incorrect result from DeclRefExpr::refersToEnclosingVariableCapture?

Don Hinton via cfe-dev
On Wed, Oct 21, 2015 at 11:14 AM, Tim Prince via cfe-dev <[hidden email]> wrote:
Hi everyone,

We've been working on upgrading our frontend from clang 3.4 to 3.7 and have run
into an issue where 'DeclRefExpr::refersToEnclosingVariableOrCapture' seems to be
returning the incorrect result. Here's a test case:

  int fn1() {
    const int i = 0;
    return [] { return i; }();
  }

Calling 'refersToEnclosingVariableOrCapture' on the 'DeclRefExpr' for 'i' within
the lambda body yields false. In clang 3.4, the equivalent 'refersToCapturedVariable'
method yields true. It looks this behavior was probably introduced by r224323,
which changed 'Sema::BuildDeclRefExpr' to determine this via 'tryCaptureVariable'.


That's an interesting case.  `i` is a compile-time constant expression there, so there's
no ODR-use of the variable, hence it's not captured.  I don't know what that should
imply for refersToEnclosingVariableOrCapture; the specification is a touch vague.

So with that background, I have a few questions:

- Is this code even valid? It seems like it shouldn't be since there's no
  capture-default or capture-list, but both clang and GCC accept it.

I believe it is, as only the value of the constant expression is used.
 
- Assuming the code is valid, is this a bug in clang?

I don't know.
 
- If not, is there some other way to detect when a DeclRefExpr in a lambda body
  refers to a declaration from the enclosing scope?

One way is to keep track of the enclosing lambdas, and while that's something I do in code right now (out of tree) I'd prefer to get rid of that.

-- James
 

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Incorrect result from DeclRefExpr::refersToEnclosingVariableCapture?

Don Hinton via cfe-dev
On Wed, Oct 21, 2015 at 4:08 PM, James Dennett via cfe-dev <[hidden email]> wrote:
On Wed, Oct 21, 2015 at 11:14 AM, Tim Prince via cfe-dev <[hidden email]> wrote:
Hi everyone,

We've been working on upgrading our frontend from clang 3.4 to 3.7 and have run
into an issue where 'DeclRefExpr::refersToEnclosingVariableOrCapture' seems to be
returning the incorrect result. Here's a test case:

  int fn1() {
    const int i = 0;
    return [] { return i; }();
  }

Calling 'refersToEnclosingVariableOrCapture' on the 'DeclRefExpr' for 'i' within
the lambda body yields false. In clang 3.4, the equivalent 'refersToCapturedVariable'
method yields true. It looks this behavior was probably introduced by r224323,
which changed 'Sema::BuildDeclRefExpr' to determine this via 'tryCaptureVariable'.


That's an interesting case.  `i` is a compile-time constant expression there, so there's
no ODR-use of the variable, hence it's not captured.  I don't know what that should
imply for refersToEnclosingVariableOrCapture; the specification is a touch vague.

What it really means is "this DeclRefExpr is referring to a capture of this variable rather than to the variable itself". The "enclosing variable" part is for the "captured statement" feature, which models captures somewhat differently.
 
So with that background, I have a few questions:

- Is this code even valid? It seems like it shouldn't be since there's no
  capture-default or capture-list, but both clang and GCC accept it.

I believe it is, as only the value of the constant expression is used.
 
- Assuming the code is valid, is this a bug in clang?

I don't know.

It is; we don't yet implement the new "or e is a discarded-value expression" wording in [basic.def.odr]/2.
 
- If not, is there some other way to detect when a DeclRefExpr in a lambda body
  refers to a declaration from the enclosing scope?

One way is to keep track of the enclosing lambdas, and while that's something I do in code right now (out of tree) I'd prefer to get rid of that.

You could check the DeclContext of the variable.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Incorrect result from DeclRefExpr::refersToEnclosingVariableCapture?

Don Hinton via cfe-dev
On 10/21/2015 9:16 PM, Richard Smith via cfe-dev wrote:

> On Wed, Oct 21, 2015 at 4:08 PM, James Dennett via cfe-dev
> <[hidden email] <mailto:[hidden email]>> wrote:
>
>     On Wed, Oct 21, 2015 at 11:14 AM, Tim Prince via cfe-dev
>     <[hidden email] <mailto:[hidden email]>> wrote:
>
>         Hi everyone,
>
>         We've been working on upgrading our frontend from clang 3.4 to
>         3.7 and have run
>         into an issue where
>         'DeclRefExpr::refersToEnclosingVariableOrCapture' seems to be
>         returning the incorrect result. Here's a test case:
>
>            int fn1() {
>              const int i = 0;
>              return [] { return i; }();
>            }
>
>         Calling 'refersToEnclosingVariableOrCapture' on the
>         'DeclRefExpr' for 'i' within
>         the lambda body yields false. In clang 3.4, the equivalent
>         'refersToCapturedVariable'
>         method yields true. It looks this behavior was probably
>         introduced by r224323,
>         which changed 'Sema::BuildDeclRefExpr' to determine this via
>         'tryCaptureVariable'.
>
>
>     That's an interesting case.  `i` is a compile-time constant
>     expression there, so there's
>     no ODR-use of the variable, hence it's not captured.  I don't know
>     what that should
>     imply for refersToEnclosingVariableOrCapture; the specification is a
>     touch vague.
>
>
> What it really means is "this DeclRefExpr is referring to a capture of
> this variable rather than to the variable itself". The "enclosing
> variable" part is for the "captured statement" feature, which models
> captures somewhat differently.

Ok, so refersToEnclosingVariableOrCapture() is not a drop in substitute
for the former refersToEnclosingLocal().

>         So with that background, I have a few questions:
>
>         - Is this code even valid? It seems like it shouldn't be since
>         there's no
>            capture-default or capture-list, but both clang and GCC
>         accept it.
>
>
>     I believe it is, as only the value of the constant expression is used.
>
>         - Assuming the code is valid, is this a bug in clang?
>
>
>     I don't know.
>
>
> It is; we don't yet implement the new "or e is a discarded-value
> expression" wording in [basic.def.odr]/2.

Richard, can you elaborate?  It isn't clear to me what you think the bug
is.  Clang 3.7 does accept the code as presented.  According to your
description of refersToEnclosingVariableOrCapture(), the function
correctly returns false for this case (since the reference is to a
variable that is not captured).  The variable reference doesn't seem to
me to be a discarded-value expression since the value is used for the
function return value.

It would be useful to have a predicate for "this DeclRefExpr refers to a
local variable of an enclosing scope".

Tom.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev