std.traits: hasFunctionAttributes by wilzbach · Pull Request #4287 · dlang/phobos

wilzbach · 2016-05-07T16:29:43Z

As discussed in #4274 the std.traits API for querying after function attributes is not very user-friendly.

functionAttributes!((int a) { }) == (FunctionAttributes.pure_ | FunctionAttributes.nothrow_ | FunctionAttributes.nogc | FunctionAttributes.safe

wouldn't it be nicer, if we have a dedicated function for this?

hasFunctionAttributes!((int a) { }, "pure", "nothrow", "@nogc", "@safe");

Imho this makes it a lot more intuitive for the user to read and easier for us to maintain.
I heard about a DIP to cleanup and unify function attributes, so having a function allows us to stay flexible for the future ;-)
Btw the trait getFunctionAttributes already returns a string tuple.

Ping @ntrel

ntrel · 2016-05-09T15:35:04Z

std/traits.d

+See_Also:
+    $(LREF functionAttributes)
+*/
+template hasFunctionAttributes(func...)


(alias func, strAttributes...)

I tried this too, but it doesn't work when called with typeof(func)

Maybe add an overload for (Func, strAttributes...) then? That's best, otherwise at least rename func to args. (It's pretty annoying alias doesn't accept all types).

JackStouffer · 2016-05-09T15:42:28Z

@wilzbach Issue 14835 rears its ugly head once again.

ntrel · 2016-05-09T15:45:41Z

std/traits.d

+
+Params:
+    func = function to check
+    attributes = variadic number of function attributes


Need to state these are strings, not enums.

I added "... as strings" - do you prefer a different wording?

I think that's OK, thanks.

JackStouffer · 2016-05-09T17:26:37Z

std/traits.d

+    $(LREF functionAttributes)
+*/
+template hasFunctionAttributes(args...)
+    if (isCallable!(args[0]) && args.length > 0)


What about

if (isCallable!(args[0]) && args.length > 1 && allSatisfy!(isSomeString, args[1 .. $]))

args are just strings, but with a small wrapper this works well. thanks a lot!

wilzbach · 2016-05-10T19:54:01Z

As this exposes a public symbol, it needs the andralex tag ;-)

JackStouffer · 2016-05-10T20:35:14Z

std/traits.d

+See_Also:
+    $(LREF functionAttributes)
+*/
+/// ditto


oh that was just a relict from trying to have different overloads. Thanks for spotting that :)

wilzbach · 2016-05-29T17:54:41Z

In a similar manner, we could also provide a better API for ParameterStorageClassTuple. Again most of them, but not all need underscores.

JackStouffer · 2016-06-02T14:33:26Z

std/traits.d

    return res;
 }

+private enum bool isSomeStringInfer(alias T) = isSomeString!(typeof(T));


Private templates should never show up in public template constraints.

JackStouffer · 2016-06-02T20:38:20Z

std/traits.d

+         && allSatisfy!(isSomeString, typeof(args[1 .. $])))
+{
+    enum bool hasFunctionAttributes = {
+        import std.algorithm.searching: canFind;


space before :

rebased ;-)

wilzbach · 2016-06-02T20:39:59Z

I think you can use allSatisfy!(isSomeString, typeof(args[1..$])).

thanks @ntrel - that worked!
(isSomeStringInfer was an relict from the initial version when the function has two specializations)

JackStouffer · 2016-06-02T21:06:54Z

I don't see any other problems. LGTM

andralex · 2016-06-18T08:39:08Z

Breathtaking, thx.

Please add an example checking the attributes of a function instantiated from a template. That should show that attributes are deduced even if not introduced by the user. Use typeof(fncall(args)) as the first argument to hasFunctionAttributes.
Can this be generalized? I'm thinking if one adds some attribute @Crap(42) then subsequently hasFunctionAttribute!(fun, Crap(42)) should yield true.

andralex · 2016-06-18T08:39:53Z

I preapprove if (1) is addressed and (2) is at least not precluded by the current design

wilzbach · 2016-06-19T16:40:33Z

Please add an example checking the attributes of a function instantiated from a template. That should show that attributes are deduced even if not introduced by the user. Use typeof(fncall(args)) as the first argument to hasFunctionAttributes.

ehm isn't typeof(myFunc(bool)) == bool. I added an example to the ddoced unittest that shows that the attributes are deduced by the compiler.

In the huge tests there are also cases where attributes are inferred, e.g. @system and the delegates at the bottom are inferred too.

Can this be generalized? I'm thinking if one adds some attribute @crap(42) then subsequently hasFunctionAttribute!(fun, Crap(42)) should yield true.

Yes it could be, but the DLang specification makes an explicit difference between UDA and FunctionAttributes.
Moreover there is already hasUDA. I will have a look whether hasUDA can be improved ;-)

wilzbach · 2016-08-28T15:40:04Z

I preapprove if (1) is addressed and (2) is at least not precluded by the current design
..
isn't typeof(myFunc(bool)) == bool. I added an example to the ddoced unittest that shows that the attributes are deduced by the compiler.
..
Yes it could be, but the DLang specification makes an explicit difference between UDA and FunctionAttributes.

@9il could you please point out why you marked this as "needs work"? I don't see what is blocking on my side & it has already been preapproved.
As usual to avoid unnecessary conflicts & work, I will add the changelog entry afterwards.

9il · 2016-08-28T16:21:28Z

@9il could you please point out why you marked this as "needs work"? I don't see what is blocking on my side & it has already been preapproved.

auto tester is failing

dlang-bot · 2016-08-28T16:53:18Z

@wilzbach, thanks for your PR! By analyzing the annotation information on this pull request, we identified @9rnsr, @sinfu and @WalterBright to be potential reviewers. @9rnsr: The PR was automatically assigned to you, please reassign it if you were identified mistakenly.

(The DLang Bot is under development. If you experience any issues, please open an issue at its repo.)

wilzbach · 2016-08-28T16:56:54Z

auto tester is failing

Oh sorry - it looks like DMD got smarter and can now automatically infer @safe for free functions (it doesn't do this for structs yet).

wilzbach · 2016-08-31T12:20:13Z

std/traits.d

+        int safeF() @safe { return 0; }
+
+        int pureF() pure { return 0; }
+    }


CodeCov is failing because DMD counts all these functions as uncovered.
(there's a NG thread discussion about not counting unittest blocks, but tl:dr it won't happen)

wilzbach · 2016-12-10T19:54:18Z

I found this PR -> rebased + squashed it -> it's alive :)

I preapprove if (1) is addressed and (2) is at least not precluded by the current design

For clarity here again my answers to Andrei's points:

(1): Please add an example checking the attributes of a function instantiated from a template. That should show that attributes are deduced even if not introduced by the user. Use typeof(fncall(args)) as the first argument to hasFunctionAttributes.

As mentioned above I added:

bool myFunc(T)(T b)  { return !b;  }
static assert(hasFunctionAttributes!(myFunc!bool, "@safe", "pure", "@nogc", "nothrow"));

However typeof(myFunc(true)) doesn't work because it is bool.

(2) Can this be generalized? I'm thinking if one adds some attribute @Crap(42) then subsequently hasFunctionAttribute!(fun, Crap(42)) should yield true.

Yes it could be, but the DLang specification makes an explicit difference between UDA and FunctionAttributes.
Moreover there is already hasUDA.

wilzbach · 2017-01-04T12:42:53Z

I had to rebase this PR because I added a changelog entry (btw there were tabs in the changelog file).

Is anything else blocking this?
(Andrei preapproved the name)

andralex · 2017-01-04T13:10:47Z

std/traits.d

+        {
+            if (!funcAttribs.canFind(attribute))
+            {
+                b = false;


Eh, that's overly complicated. Why not just return false from here?

andralex · 2017-01-04T13:11:00Z

std/traits.d

+                break;
+            }
+        }
+        return b;


(and true from here)

andralex · 2017-01-04T13:11:40Z

std/traits.d

+{
+    real func(real x) pure nothrow @safe
+    {
+        return x;


In order to avoid false coverage reports, you may want to just declare the function without implementing it.

andralex · 2017-01-04T13:11:51Z

std/traits.d

+{
+    struct S
+    {
+        int noF() { return 0; }


same here - all are not covered

andralex · 2017-01-04T13:12:17Z

std/traits.d

+    struct S2
+    {
+        int pure_const() const pure { return 0; }
+        int pure_sharedconst() const shared pure { return 0; }


these, too - declare and don't define

wilzbach · 2017-01-04T13:44:16Z

std/traits.d

    }
    static assert(hasFunctionAttributes!(myFunc!bool, "@safe", "pure", "@nogc", "nothrow"));
-    static assert(!hasFunctionAttributes!(myFunc!bool, "@trusted"));
+    static assert(!hasFunctionAttributes!(myFunc!bool, "shared"));


This change is just to make the public unittest a bit more interesting and not use @trusted twice.

wilzbach · 2017-01-04T13:47:00Z

Eh, that's overly complicated. Why not just return false from here?

When this PR initially was submitted a DMD bug prevented this (it wouldn't recognize this properly and warn with "unreachable code"). However, it seems that this has been fixed. Thanks! :)

In order to avoid false coverage reports, you may want to just declare the function without implementing it.

Ok, I did this for the manually annotated functions. However, the other functions are supposed to check whether automatic attribute inference works correctly. Thus, I added a few dummy calls.

andralex · 2017-01-04T14:15:56Z

std/traits.d

+        import std.algorithm.searching : canFind;
+        import std.range : only;
+        enum funcAttribs = only(__traits(getFunctionAttributes, args[0]));
+        foreach (attribute; args[1..$])


spaces around .. (should be detected by the style checker...)

wilzbach · 2017-01-04T14:32:03Z

spaces around .. (should be detected by the style checker...)

Yeah this is on my backlog. Btw while I like standardization it's not uniformly considered so, see e.g. #4999

wilzbach · 2017-01-04T14:34:12Z

spaces around .. (should be detected by the style checker...)

I added the whitespace, rebased and armed the new merge-bot.

JackStouffer · 2017-01-04T15:26:29Z

Auto-merge toggled on

ntrel reviewed May 9, 2016
View reviewed changes

wilzbach force-pushed the traits_has_function_attributes branch 2 times, most recently from f8db352 to 6e0aadd Compare May 9, 2016 16:24

JackStouffer reviewed May 9, 2016
View reviewed changes

wilzbach force-pushed the traits_has_function_attributes branch from 6e0aadd to 4169827 Compare May 10, 2016 19:53

JackStouffer reviewed May 10, 2016
View reviewed changes

wilzbach force-pushed the traits_has_function_attributes branch from 4169827 to 556bfd1 Compare May 10, 2016 20:46

DmitryOlshansky added the @andralex label May 11, 2016

JackStouffer reviewed Jun 2, 2016
View reviewed changes

wilzbach force-pushed the traits_has_function_attributes branch from 556bfd1 to dcb8ba4 Compare June 2, 2016 20:30

JackStouffer reviewed Jun 2, 2016
View reviewed changes

wilzbach force-pushed the traits_has_function_attributes branch from dcb8ba4 to 1254ac5 Compare June 2, 2016 20:43

andralex removed the @andralex label Jun 18, 2016

wilzbach force-pushed the traits_has_function_attributes branch from 1254ac5 to 2565349 Compare June 19, 2016 16:33

9il added Review:Needs Work Severity:Enhancement labels Jul 29, 2016

wilzbach removed the Review:Needs Work label Aug 28, 2016

wilzbach force-pushed the traits_has_function_attributes branch from 2565349 to 87cadb4 Compare August 28, 2016 16:53

dlang-bot assigned 9rnsr Aug 28, 2016

wilzbach reviewed Aug 31, 2016
View reviewed changes

wilzbach unassigned 9rnsr Dec 10, 2016

wilzbach force-pushed the traits_has_function_attributes branch from 87cadb4 to 936380b Compare December 10, 2016 19:54

JackStouffer approved these changes Dec 21, 2016

View reviewed changes

wilzbach added this to the 2.073.0-b0 milestone Dec 29, 2016

wilzbach force-pushed the traits_has_function_attributes branch from 936380b to 371089d Compare January 4, 2017 12:39

andralex requested changes Jan 4, 2017

View reviewed changes

wilzbach force-pushed the traits_has_function_attributes branch from c885f25 to ac81a9c Compare January 4, 2017 13:43

wilzbach commented Jan 4, 2017

View reviewed changes

andralex approved these changes Jan 4, 2017

View reviewed changes

std.traits: hasFunctionAttributes

970dff3

wilzbach force-pushed the traits_has_function_attributes branch from ac81a9c to 970dff3 Compare January 4, 2017 14:26

wilzbach added the Merge:auto-merge label Jan 4, 2017

JackStouffer merged commit f94a9e9 into dlang:master Jan 4, 2017

wilzbach deleted the traits_has_function_attributes branch February 19, 2017 23:44

Uh oh!

Conversation

wilzbach commented May 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JackStouffer commented May 9, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented May 10, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented May 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Jun 2, 2016

Uh oh!

JackStouffer commented Jun 2, 2016

Uh oh!

andralex commented Jun 18, 2016

Uh oh!

andralex commented Jun 18, 2016

Uh oh!

wilzbach commented Jun 19, 2016

Uh oh!

wilzbach commented Aug 28, 2016

Uh oh!

9il commented Aug 28, 2016

Uh oh!

dlang-bot commented Aug 28, 2016

Uh oh!

wilzbach commented Aug 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Dec 10, 2016

Uh oh!

wilzbach commented Jan 4, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Jan 4, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Jan 4, 2017

Uh oh!

wilzbach commented Jan 4, 2017

Uh oh!