Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Apr 27, 2014

https://issues.dlang.org/show_bug.cgi?id=12668

This requires dlang/dmd#3506 before it will pass the autotester.

@ghost
Copy link
Author

ghost commented Apr 27, 2014

I've left the demangleFunctionAttributes intact in case we ever need to use it in the future. But if we want to do it we could remove it.

@ghost
Copy link
Author

ghost commented Apr 27, 2014

Ah I've just realized I can use a CTFE function with a static foreach.

Edit: Updated to use CTFE.

@ghost ghost closed this Apr 27, 2014
@ghost ghost reopened this Apr 27, 2014
@monarchdodra
Copy link
Collaborator

Did you notice it's failing?

@ghost
Copy link
Author

ghost commented Apr 29, 2014

This requires dlang/dmd#3506 before it will pass the autotester.

@monarchdodra
Copy link
Collaborator

This requires dlang/dmd#3506 before it will pass the autotester.

Derp. I saw it the first time, and then forgot about it :/

@ghost
Copy link
Author

ghost commented Apr 29, 2014

Anyway no reason to keep this open while it's waiting for the DMD pull.

DMD pull was merged. Awaiting review & green on this one.

@ghost ghost closed this Apr 29, 2014
@ghost ghost reopened this May 5, 2014
@monarchdodra
Copy link
Collaborator

Seems like it is now failing at the unittest stage.

@ghost
Copy link
Author

ghost commented May 6, 2014

Gonna investigate. Strange because it worked a few days ago locally.. something must have changed.

@ghost
Copy link
Author

ghost commented May 6, 2014

It appears the old FunctionTypeOf trick was in there to support opCall. I've left it in as a special-case, but I'll have to investigate some more to see if I can support this directly with the getFunctionAttributes trait. In the meantime the tests should start passing now.

property = 1 << 3, /// ditto
trusted = 1 << 4, /// ditto
safe = 1 << 5, /// ditto
nogc = 1 << 6, /// ditto
Copy link
Author

Choose a reason for hiding this comment

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

The reason why I've left nogc here instead of moved it somewhere else (which would have made the order nicer with [trusted, safe, system]), is because I want to keep backwards compatibility if any code ever stored nogc as a uint field somewhere (it's might be rare, but I don't want to introduce hard to track bugs).

@ghost
Copy link
Author

ghost commented May 8, 2014

I need to fixup getFunctionAttributes w.r.t. @System first, let me close this for a while.

@ghost ghost closed this May 8, 2014
@ghost ghost reopened this May 8, 2014
@ghost
Copy link
Author

ghost commented May 8, 2014

Updated. Hopefully it should pass now, locally it works.

@ghost
Copy link
Author

ghost commented May 8, 2014

Ah interesting, there's some DerivedFunctionType template magic in std.typecons. I'll have to see what can be fixed here.

@ghost
Copy link
Author

ghost commented May 8, 2014

I've added a workaround.

@9rnsr: You probably wrote that DerivedFunctionType code. Do you think it's ok that I had to remove @system when applying the attribute (see the diff)? If not then maybe I could change how SetFunctionAttributes works instead so existing code continues to work.

@monarchdodra
Copy link
Collaborator

LGTM, but the any subtlety here could have gone over my head.

In particular: Is @system an actual attribute? I know you can "mark" a function as @system, but I always figured it was an override to prevent a template from being inferred @safe...

I guess the question is really for the getFunctionAttributes implementation...

@ghost
Copy link
Author

ghost commented May 11, 2014

In particular: Is @System an actual attribute?

Sure, it just happens to be the default. Let's ping @9rnsr though.

@monarchdodra
Copy link
Collaborator

Sure, it just happens to be the default. Let's ping @9rnsr though.

Hum...

struct S
{
  void foo()         {}
  void bar() @system {}
}

void main()
{
    pragma(msg, typeof(S.foo).stringof);
    pragma(msg, typeof(S.bar).stringof);
}
void()
@system void()

Now I'm even more confused. Seems that @system is an attribute that may or may not exist, but we don't really care either way, since the only thing that matters is rather "the abscense of @safe/@trusted"?

Given the above test, it seems inconsistent that your tests pass:

    static assert(functionAttributes!(S.noF) == FA.system); 
    static assert(functionAttributes!(typeof(S.noF)) == FA.system); 

I think there is an inconsistency here between what the actual attributes are, and what attributes get mangled in the name?

@ghost
Copy link
Author

ghost commented May 12, 2014

@monarchdodra
Copy link
Collaborator

I see...

@mihails-strasuns
Copy link

Auto-merge toggled on

@mihails-strasuns
Copy link

Looks good and latest auto-tester results look green

mihails-strasuns pushed a commit that referenced this pull request Jul 29, 2014
Issue 12668 -  std.traits.functionAttributes should use the new getFunctionAttributes trait.
@mihails-strasuns mihails-strasuns merged commit e709fee into dlang:master Jul 29, 2014
@ghost
Copy link
Author

ghost commented Jul 30, 2014

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants