Need Feedback on this very preliminary code that implements a portion of generic lambdas.

First off, this patch is far from ready for committing - it does not include
enough tests - includes commented out code - and even some dead
code. I shall try to address all those issues in the future - but for now
I just wanted to put this out there for some early feedback (before I go
any deeper down a rabbit hole I need not have crawled into)

The patch implements the following (an incomplete list):

  • converting auto parameters to template parameters and creating a
    function call operator template
  • It does this by replacing the typesourceinfo of the parameter -
    Should I avoid doing this? Is there a way to transform the auto
    into a template type parameter, so that it subsequently behaves
    as a template type parameter, but remembers that it was
    once an auto (for error messages only, right?)
  • it makes TransformLambdaExpr and InstantiateFunctionDefinition
    generic lambda aware (to allow for transformation of nested lambdas)
    so that the
  • Regarding capturing for generic lambdas:
  1. A generic lambda (nested or not) will only have its captures
    computed/finalized when its enclosing context is non-dependent
    (and if an empty capture list, then no captures).
  2. When a generic lambda is invoked (i.e a specialization is instantiated)
    any variable from an outer scope that is odr-used, but not already
    captured by the lambda expression, results in an arror.
  3. If an outer lambda that is enclosed in a non-dependent context,
    can’t tell whether an inner lambda (with an implicit capture)
    or a tree of inner lambdas (all with implicit captures)
    might need to capture the variable for some
    instantiation of that inner lambda, the outer lambda will
    capture that variable (if it has a capture default).
  • this is implemented by hooking into ActOnCallExpr
    (is it enough to assume that the only cases where an inner
    lambda might potentially need to capture a variable is
    if it is used in a dependent function call (ctor, operator)
    of the inner lambda, in a non-unevaluated context?)
  • Within ActOnCallExpr, I dig into each argument, and as
    long as they are not in an unevaluated context (currently
    I only check for sizeof), and are from a scope outside
    the outer lambda, I capture the variable within the outer
    lambda (if both the inner and outer lambda have a cap default,
    and the outer lambda is enclosed by a non-dependent context).
  • The reason I do this in ActOnCallExpr is because until then
    I do not know if the function call is still dependent, perhaps
    there is a better place to do this?

I will include some code to illustrate various scenarios:

#define L_CAP
#define M_CAP
#define N_CAP
#define L_CALL
#define M_CALL
#define N_CALL
#define M_IS_GENERIC_IF_AUTO auto
#define TEST_CALL test(‘a’)

#define F_CALL f(x, selector)

void f(int, const int (&)[1]) { }
void f(const int&, const int (&)[2]) { }

void g(int, const int (&)[1]) { }
void g(int, const int (&)[2]) { }

template
void test(T t) {
const int x = 0;
const int y = 10;
auto L = [L_CAP](auto a) → void {
int selector[sizeof(t) == 1 ? 1 : 2]{};
F_CALL;
auto M = [M_CAP](M_IS_GENERIC_IF_AUTO b) → void {
int selector[sizeof(a) == 1 ? 1 : 2]{};
F_CALL;
auto N = [N_CAP](auto c) → void {
int selector[sizeof(c) == 1 ?
(sizeof(b) == 1 ? 1 : 2) : 2]{};
F_CALL;
};
N_CALL;
};
M_CALL;
};
L_CALL;
}

int main() {
TEST_CALL;
}

Now lets consider the following scenarios (if the #define is not
mentioned in that scenario, it is as above):
A) #define TEST_CALL test(‘h’); test(5);

  • no errors
    B) #define TEST_CALL test(5)
  • no errors
  • error if L is instantiated: e.g. #define L_CALL L(‘5’)
  • ok again if: #define L_CAP =
    C) #define TEST_CALL test(5)
    #define L_CAP =
    #define L_CALL L(‘j’)
  • no errors since M does not need to capture x
  • if #define N_CAP =, still no error.
  • error if M is instantiated: e.g. #define M_CALL M(‘a’)
  • since when N is examined, it contains an expression that
    depends on a dependent generic parameter, so therefore
    M contains an expression that depends on a dependent generic
    parameter, and so x needs to be captured by M
    but M has no default capture or explicit x capure.
  • this can be fixed by #define M_CAP = OR #define M_CAP x
  • if #define N_CAP =
    #define M_CALL M(‘a’)
    #define F_CALL sizeof(f(x, selector))
    Then no error, since x can not be odr-used
  • if #define N_CAP =
    #define M_CALL M(‘a’)
    #define F_CALL g(y, selector)
    Even though all overloads of g don’t require N to
    capture y, y will need to be captured preemptively by M
    (which will error, unless M has a default capture or explicitly
    captures y) but never by N, because when M has to finalize its
    captures, it can only see that N uses x in a
    dependent expression, and so it can’t know that all
    instantiations of N will never odr-use y.
    (although i guess it can by looking at each overload
    and knowing that ADL is disabled for primitives and
    recognizing that every call requires the lvalue-to-rvalue
    conversion, but this could get very hairy i imagine)
    D) #define TEST_CALL test(‘h’)
    #define L_CALL L(‘j’)
    #define M_IS_GENERIC_IF_AUTO char
  • no errors
  • if #define N_CALL N(‘k’) - still no errors
  • if #define N_CALL N(5) - error, N can not capture
  • if #define M_IS_GENERIC_IF_AUTO auto
    and #define M_CALL M(‘k’)
    and #define N_CALL N(‘t’) - still no errors
    but if instead above #define M_CALL M(5) - error N cant capture x
  1. Does ActOnCallExpr, cover dependent Constructor and Operator
    invocations? (sorry i’m being lazy here)

Additionally, It does not implement the following (but i will get to it):

  • return type deduction for generic lambdas
  • conversion operator for generic lambdas with no captures
  • comprehensive capturing of variadic packs within nested lambdas

Recognizing that this is a very rough draft, and that the code does need
significant refactoring, curating, testing and potential re-architecting
before it is ready for commit;
I’m hoping to solicit the following feedback:

  • general thoughts & discussion regarding the implementation strategy
    (and whether there are better ways to do what I am doing).
  • on the correctness of the capturing semantics as described above and
    their concordance with our standard wording.
  • should i try and break up the patch into smaller patches when
    submitting for actual commit?
  • should i continue working on this …

Any constructive feedback will be welcome!

Thanks.

initial-generic-lambda-no-conversion-operator-or-pack-expansions-4.patch (131 KB)

First off, this patch is far from ready for committing - it does not include
enough tests - includes commented out code - and even some dead
code. I shall try to address all those issues in the future - but for now
I just wanted to put this out there for some early feedback (before I go
any deeper down a rabbit hole I need not have crawled into)

Just a couple of thoughts below, I’ve not had time to study the patch itself yet.

The patch implements the following (an incomplete list):

  • converting auto parameters to template parameters and creating a
    function call operator template
  • It does this by replacing the typesourceinfo of the parameter -
    Should I avoid doing this? Is there a way to transform the auto
    into a template type parameter, so that it subsequently behaves
    as a template type parameter, but remembers that it was
    once an auto (for error messages only, right?)

Perhaps an AutoType whose deduced type is the template parameter would work? However, note this comment from SubstituteAutoTransform:

// If we’re building the type pattern to deduce against, don’t wrap the
// substituted type in an AutoType. Certain template deduction rules
// apply only when a template type parameter appears directly (and not if
// the parameter is found through desugaring). For instance:
// auto &&lref = lvalue;
// must transform into “rvalue reference to T” not “rvalue reference to
// auto type deduced as T” in order for [temp.deduct.call]p3 to apply.

If we wanted that to work, we’d need to teach some parts of template deduction to walk over the AutoType node.

How significant is the diagnostic impact of performing the substitution? Where, and how often, does it show up?

  • it makes TransformLambdaExpr and InstantiateFunctionDefinition
    generic lambda aware (to allow for transformation of nested lambdas)
    so that the
  • Regarding capturing for generic lambdas:
  1. A generic lambda (nested or not) will only have its captures
    computed/finalized when its enclosing context is non-dependent
    (and if an empty capture list, then no captures).
  2. When a generic lambda is invoked (i.e a specialization is instantiated)
    any variable from an outer scope that is odr-used, but not already
    captured by the lambda expression, results in an arror.
  3. If an outer lambda that is enclosed in a non-dependent context,
    can’t tell whether an inner lambda (with an implicit capture)
    or a tree of inner lambdas (all with implicit captures)
    might need to capture the variable for some
    instantiation of that inner lambda, the outer lambda will
    capture that variable (if it has a capture default).
  • this is implemented by hooking into ActOnCallExpr
    (is it enough to assume that the only cases where an inner
    lambda might potentially need to capture a variable is
    if it is used in a dependent function call (ctor, operator)
    of the inner lambda, in a non-unevaluated context?)

Per the proposal, you need captures in other situations:

struct S {
constexpr S() {}
S(const S&) { puts(“captured”); }
};
void f() {
constexpr S s {};
[=] (auto x) {
// captures ‘s’ (even though no instantiation can need it),
// because the full-expression depends on ‘x’. Therefore
// we must print “captured”.
(void)x, s;
};
}

  • Within ActOnCallExpr, I dig into each argument, and as
    long as they are not in an unevaluated context (currently
    I only check for sizeof), and are from a scope outside
    the outer lambda, I capture the variable within the outer
    lambda (if both the inner and outer lambda have a cap default,
    and the outer lambda is enclosed by a non-dependent context).
  • The reason I do this in ActOnCallExpr is because until then
    I do not know if the function call is still dependent, perhaps
    there is a better place to do this?

Here’s how I was imagining this working when we were in CWG:

The captures for a lambda are not finalized until the outermost context of the reaching scope is non-dependent. At that point, for each call to ActOnFinishFullExpr within the lambda, we check whether the full-expression is instantiation-dependent. If it is, we scan it for entities which it might need to capture, and capture all of them. There may be ways of optimizing this (maybe keep a list of the potentially-captured variables for each full-expression as we build it).

First off, this patch is far from ready for committing - it does not
include
enough tests - includes commented out code - and even some dead
code. I shall try to address all those issues in the future - but for now
I just wanted to put this out there for some early feedback (before I go
any deeper down a rabbit hole I need not have crawled into)

Just a couple of thoughts below, I've not had time to study the patch
itself yet.

The patch implements the following (an incomplete list):
  - converting auto parameters to template parameters and creating a
    function call operator template
    - It does this by replacing the typesourceinfo of the parameter -
      Should I avoid doing this? Is there a way to transform the auto
      into a template type parameter, so that it subsequently behaves
      as a template type parameter, but remembers that it was
      once an auto (for error messages only, right?)

Perhaps an AutoType whose deduced type is the template parameter would
work? However, note this comment from SubstituteAutoTransform:

      // If we're building the type pattern to deduce against, don't wrap
the
      // substituted type in an AutoType. Certain template deduction rules
      // apply only when a template type parameter appears directly (and
not if
      // the parameter is found through desugaring). For instance:
      // auto &&lref = lvalue;
      // must transform into "rvalue reference to T" not "rvalue reference
to
      // auto type deduced as T" in order for [temp.deduct.call]p3 to
apply.

If we wanted that to work, we'd need to teach some parts of template
deduction to walk over the AutoType node.

How significant is the diagnostic impact of performing the substitution?
Where, and how often, does it show up?

It shows up if deduction fails - for e.g:

auto L = (auto *a) -> void { };
L(1);

f:\clang-trunk\clang-trunk-fv\tests\test-lambda-captures-lab.cpp:77:3:
error: no matching function for call to object of type '<lambda at

f:\clang-trunk\clang-trunk-fv\tests\test-lambda-captures-lab.cpp:76:12>'
  L(1);
  ^
f:\clang-trunk\clang-trunk-fv\tests\test-lambda-captures-lab.cpp:76:12:
note: candidate template ignored: could not match 'type-parameter-0-0 *'
against 'int'
  auto L = (auto *a) -> void { };

I could live with that - but if we didn't want to - asides from considering
the approach
you presented, what are your thoughts about the following approach :
when emitting the error, check to see if it is a parameter of a generic
lambda
and sneak the auto back into the diagnositc - pros and cons?

  - it makes TransformLambdaExpr and InstantiateFunctionDefinition
    generic lambda aware (to allow for transformation of nested lambdas)
    so that the
  - Regarding capturing for generic lambdas:
    1) A generic lambda (nested or not) will only have its captures
       computed/finalized when its enclosing context is non-dependent
       (and if an empty capture list, then no captures).
    2) When a generic lambda is invoked (i.e a specialization is
instantiated)
       any variable from an outer scope that is odr-used, but not already
       captured by the lambda expression, results in an arror.
    3) If an outer lambda that is enclosed in a non-dependent context,
       can't tell whether an inner lambda (with an implicit capture)
       or a tree of inner lambdas (all with implicit captures)
       might need to capture the variable for some
       instantiation of that inner lambda, the outer lambda will
       capture that variable (if it has a capture default).
         - this is implemented by hooking into ActOnCallExpr
           (is it enough to assume that the only cases where an inner
             lambda might potentially need to capture a variable is
             if it is used in a dependent function call (ctor, operator)
             of the inner lambda, in a non-unevaluated context?)

Per the proposal, you need captures in other situations:

struct S {
  constexpr S() {}
  S(const S&) { puts("captured"); }
};
void f() {
  constexpr S s {};
  [=] (auto x) {
    // captures 's' (even though no instantiation can need it),
    // because the full-expression depends on 'x'. Therefore
    // we must print "captured".
    (void)x, s;
  };
}

          - Within ActOnCallExpr, I dig into each argument, and as

           long as they are not in an unevaluated context (currently
           I only check for sizeof), and are from a scope outside
           the outer lambda, I capture the variable within the outer
           lambda (if both the inner and outer lambda have a cap default,
           and the outer lambda is enclosed by a non-dependent context).
         - The reason I do this in ActOnCallExpr is because until then
           I do not know if the function call is still dependent, perhaps
           there is a better place to do this?

Here's how I was imagining this working when we were in CWG:

The captures for a lambda are not finalized until the outermost context of
the reaching scope is non-dependent. At that point, for each call to
ActOnFinishFullExpr within the lambda, we check whether the full-expression
is instantiation-dependent. If it is, we scan it for entities which it
might need to capture, and capture all of them. There may be ways of
optimizing this (maybe keep a list of the potentially-captured variables
for each full-expression as we build it).
*
*

Aah! ActOnFinishfullExpr was exactly the hook I was looking for - thanks!
So the new patch implements it as you describe (and it does do the
optimization
- I introduced an array of potential captures in the LambdaScopeInfo) so we
don't have to walk the expression tree each time. It seems to work for
your
case and my previous tests.

While we're on the topic, can i ask you to clarify a few capture and
constexpr
questions below:

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
      fooS(s); // 2
      fooInt(s); // 3
  };
}

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?

thanks!

initial-generic-lambda-uses-act-on-finnish-full-expr-2.patch (136 KB)

I don’t think we have enough context to work that out in the general case; the simplest way of encoding such context would be to wrap the template parameter in a type sugar node, which brings us back to the other approach.

OK, the relevant context is [basic.def.odr]p2 and p3.

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.

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.

This is equivalent to ‘s.operator int()’, which again odr-uses ‘s’, so requires a capture-default.

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