[PATCH] for feedback - avoid use of the "__invoke" magic string in Lambdas

[Faisal Vali]

On Sun, May 5, 2013 at 12:38 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Sat, May 4, 2013 at 8:46 AM, Faisal Vali <faisalv at gmail.com> wrote:
> > I just wanted to float this patch out for some feedback - It's not ready
> for
> > commit.
> > There is no functionality change - so no tests included.
> >
> > This patch addresses the following:
> >   1) It replaces the use of the "__invoke" magic name for the static
> > invoker, with a property within
> >     the lambda class  - I thought this would be a less brittle approach
> and
> > potentially more efficient
> > -   though it takes up more space in the LambdaDefinitionData - how big a
> > deal is that?
> >       - also with my current change can i just drop the name for this
> > function or do i need the
> >         "super-secret" name?
>
> I'm a little confused - it seems all the change is to change the name
> "__invoke" to "__$#lambda_invoker" - is there any particular reason to
> believe the latter is substantially safer than the former? The
> language specification reserves both identifiers for use by the
> implementation equally.
>
>
The name change is secondary to simply storing the static invoker as a
property within the LambdaDefinitionData, which is what I'd really like
feedback on.
I suppose __invoke is as good a name as any - i thought by making it more
cryptic, it might be safer - but really I was hoping we might not need to
name this function at all?



> >   2) Also I intend to put some utility functions in Lambda.h (I have
> > included some samples - these functions currently might overlap in
> > functionality - in my initial generic lambda implementation they were
> > scattered amongst various files, and i thought it might be better to
> > centralize them a little) - here are my questions:
> >      - do you prefer to place utility functions within CXXMethodDecl or
> Sema
> > (ugh - the recompiles
> >         suck with changes!) if they make sense there?
> >         That is do you prefer IsLambdaCallOperator(MD) or
> > MD->IsLambdaCallOperator() ?
>
> I'm a little confused by this too - there seem to be utility functions
> that aren't called/dead-code, though I haven't looked closely.
>


Yeah - except for the first one, they are all dead code as of now - only
the first one is relevant to the patch (which i'm hoping to drop).
I don't intend to submit those for commit-review just yet.


> Could you at least separate out your cleanup changes from the semantic
> ones so it's easier to see what's what?
>

Once I get some direction on which is preferred: storing a pointer to the
lambda static invoker within the class (my proposed solution), versus
giving it a name and looking it up within the class (status quo) - I will
re-submit a patch with just code, relevant to that fix.

Faisal Vali
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130505/59523872/attachment.html>