Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UDAs for function arguments #7576

Merged
merged 2 commits into from
Jun 30, 2018

Conversation

wilzbach
Copy link
Member

@wilzbach wilzbach commented Jan 2, 2018

#4783 revived from the dead.
See dlang/DIPs#105 for a respective DIP.

Official spelling is "user-defined attributes".

@dlang-bot
Copy link
Contributor

dlang-bot commented Jan 2, 2018

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#7576"

@wilzbach
Copy link
Member Author

wilzbach commented Jan 2, 2018

Jenkins failure looks unrelated: dlang/ci#105

{
this.storageClass = storageClass;
this.type = type;
this.ident = ident;
this.defaultArg = defaultArg;
this.userAttribDecl = userAttribDecl;
Copy link
Member Author

@wilzbach wilzbach Jan 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RazvanN7 do you have any plans on removing the giant code duplication in astbase?

@jacob-carlborg
Copy link
Contributor

Will this work for template parameters?

@wilzbach
Copy link
Member Author

wilzbach commented Jan 2, 2018

Will this work for template parameters?

Not this version.
AFAICT is the proposed DIP also only about parameter attributes (and attaching attributes to enums).
It does work with template d parameters - I just added test cases for that.


void wrongAttr1(@safe int);
void wrongAttr2(@safe void function());
void wrongAttr3(@safe void delegate());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That isn't going to be confusing at all :D

Copy link
Contributor

@skl131313 skl131313 Jan 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are invalid attributes, so it shouldn't be too big of a problem.

}

/************************************************/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably could use a test with a tuple as well

@jacob-carlborg
Copy link
Contributor

It does work with template d parameters - I just added test cases for that.

@wilzbach that do you mean with "template d parameters"?


extern (D) alias ForeachDg = int delegate(size_t idx, Parameter param);

final extern (D) this(StorageClass storageClass, Type type, Identifier ident, Expression defaultArg)
final extern (D) this(StorageClass storageClass, Type type, Identifier ident, Expression defaultArg, UserAttributeDeclaration userAttribDecl)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expression defaultArg, UserAttributeDeclaration userAttribDecl = null)

Giving this a default value would save on a lot of code edits that just add a null as the last parameter. Not sure if default values are being avoided here on purpose or not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT is the prevalent coding style in the DMD source code to prefer explicitness.

@John-Colvin
Copy link
Contributor

I'm trying to get a hold on the scope of this PR.
Can I do this?

auto a(int x)(void function(@(x) int) f);
auto b = a((@(2) int){ });

@skl131313
Copy link
Contributor

@John-Colvin
Not I don't think that's possible with the current implementation, also not sure if that is desired.

@wilzbach wilzbach force-pushed the uda-for-function-arguments branch 3 times, most recently from 4679f23 to 1656137 Compare May 7, 2018 23:35
@wilzbach
Copy link
Member Author

wilzbach commented May 8, 2018

Finally got around submitting the dlang.org PR: dlang/dlang.org#2363

static assert(is(ParameterUDA!(3, test14x)[0] == Test14UDA4!"3"));
}

void test15x(@(20) void delegate(int) @safe dg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the parameters on delegates, are UDAs on those supported? Either way I think it's worth having a test case, showing either that it works or that it doesn't work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the tests below.

@IgorStepanov
Copy link
Contributor

Ping. What status of that?

@jacob-carlborg
Copy link
Contributor

Ping. What status of that?

I asked for some more tests, which I don't think has been added. Not sure what else.

@wilzbach ping.

@wilzbach wilzbach force-pushed the uda-for-function-arguments branch 3 times, most recently from 3a69876 to c012027 Compare June 27, 2018 11:37
@wilzbach
Copy link
Member Author

I asked for some more tests, which I don't think has been added.

Sorry. I haven't found much time for this, but now managed to add these tests. See test15y and test15z.

Not sure what else.

Same here.

{
static assert([__traits(getAttributes, a)] == [22]);
}
---
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps include an example of how to get the UDAs of a parameter from outside of a function. Or should that be left as an exercise for the reader 😃.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a bit more complicated and I think we will provide an interface in std.traits anyhow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

@jacob-carlborg
Copy link
Contributor

Tests for the following could be added:

  • Lambdas, both with and without parentheses: alias a = @(3) b => 1 + 2 and alias b = (@(3) c, @(4) d) => 1 + 2 and usage like: map!(@(3) a => 1 + 2)
  • Using a delegate with inferred parameter type: foo((@(3) a) { return 1 + 2; })
  • Template parameter: void foo(@(3) T)(T t)

@wilzbach
Copy link
Member Author

Lambdas, both with and without parentheses: alias a = @(3) b => 1 + 2 and alias b = (@(3) c, @(4) d) => 1 + 2 and usage like: map!(@(3) a => 1 + 2)

Only the latter was supported (i.e. with parentheses). I added support for alias a = @(3) b => 1 + 2.
Though I'm not sure that alias a = @(3) b => 1 + 2 will be used in practice.
I added the tests to test19, but I'm not sure whether there's currently a way to query the attributes.

Using a delegate with inferred parameter type: foo((@(3) a) { return 1 + 2; })

Added to test20.

Template parameter: void foo(@(3) T)(T t)

That's not supported as of now. I'm not sure it's that useful, but if people run into use cases for it, it could always be added?
(I still added it to fail_compilation)

@jacob-carlborg
Copy link
Contributor

That's not supported as of now

I’m not asking for adding new functionality, just to add tests that shows it’s either compiles or fails to compile.

@wilzbach wilzbach closed this Jun 30, 2018
@wilzbach wilzbach reopened this Jun 30, 2018
@dlang-bot dlang-bot merged commit d1e803f into dlang:master Jun 30, 2018
@nordlow
Copy link
Contributor

nordlow commented Jul 1, 2018

Cool. I wonder if Andrei will have use of this in his complexity library...

I notified him and he answered that this PR indeed will make it more powerful!

@wilzbach wilzbach deleted the uda-for-function-arguments branch July 1, 2018 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.