fix issue 18149 - Add a compiler trait to detect if a function is @disable #7569

ghost · 2017-12-31T15:20:02Z

Although there's not much concrete application (since a call to a @disable function is already statically checked) for now there is no way to know if a function is @disable.

dlang-bot · 2017-12-31T15:20:04Z

Thanks for your pull request, @bbasile! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

smaller, focused PRs are easier to review than big ones
try not to mix up refactoring or style changes with bug fixes or feature enhancements
provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close	Bugzilla	Description
✓	18149	Add a compiler trait to detect if a function is @disable

JinShil · 2018-01-01T13:15:36Z

Is there any particular reason why @disable can't be added to the getAttributes trait?

UPDATE: I see that once a function is marked with @disable, getAttributes cannot be used on it. Adding @disable to getAttributes would introduce a breaking change (and may not even be appropriate; I'm still unsure about it).

ghost · 2018-01-01T15:35:04Z

Adding @disable to getAttributes would introduce a breaking change (and may not even be

Yes. This and also it'd be for the getFunctionAttributes verb (the one you ask for is for the UDAs). There's a lot of code in std.typecons that relies on them and @disable would not being usable in the same fashion anyway (typically all the wrappers things).

JinShil · 2018-01-01T23:44:50Z

I'm itching to approve this, but before then it needs a changelog entry in this PR and a documentation update at https://github.com/dlang/dlang.org.

ghost · 2018-01-02T06:17:40Z

@JinShil, a changelog entry is not needed. An issue is referenced so it would appear even without.

JinShil · 2018-01-02T06:28:21Z

@JinShil, a changelog entry is not needed. An issue is referenced so it would appear even without.

Yes, I know, and I generally agree, but I've been asked to add changelog entries for certain Bugzilla issues, even though they appear in list of fixed issues. The following are examples:

This is a new feature and I think it is appropriate to do some advertising for it.

PetarKirov · 2018-01-02T09:01:09Z

We have had examples of both adding a dedicated changelog entry and relying on Bugzilla (e.g. https://dlang.org/changelog/2.078.0.html#bugfix-list - isFuture). IMO we should add changelog entries for all enhancement requests which allow expressing new things in the language, that were not easily doable before.

@RazvanN7 can you add a changelog entry for isFuture?

ghost · 2018-01-02T16:32:53Z

Okay i 've just added the changelog entry and the good news is that this has revealed that the trait is much more useful than i thought.

JinShil

Thanks @bbasile. I'm going to leave this open for a little while to give others an opportunity to weigh in.

wilzbach · 2018-01-03T00:11:09Z

I'm going to leave this open for a little while to give others an opportunity to weigh in.

Yes this is a language change and thus needs approval from either @WalterBright or @andralex.

WalterBright · 2018-01-05T09:31:40Z

src/dmd/declaration.d

@@ -417,6 +417,11 @@ extern (C++) abstract class Declaration : Dsymbol
        return (storage_class & STCdeprecated) != 0;
    }

+    final bool isDisabled()


Add const pure nothrow @nogc @safe

WalterBright · 2018-01-05T09:32:06Z

changelog/disabled-trait.dd

+using `__trait(compiles)`, which was less than ideal since `false` could be
+returned for other reasons.
+
+Now, in your metaprogramming code, `@disable` functions can be detected accurately,


Remove your

WalterBright

see my comments

JinShil · 2018-01-06T05:23:07Z

@WalterBright Are you willing to explicitly mark this "Approved"?

JinShil · 2018-01-08T23:48:12Z

@bbasile Please rebase. It won't elicit any reviews unless it's passing.

wilzbach · 2018-01-08T23:53:44Z

@bbasile Please rebase. It won't elicit any reviews unless it's passing.

What do you mean by this? I don't see any merge conflicts and only auto-tester has one spurious failure, but auto-tester automatically invalidates and restarts all jobs if at any repo a PR is merged to master.

@WalterBright needs to dismiss his review (I could do that as well, but I'm not sure he explicitly approved the addition).

JinShil · 2018-01-08T23:58:08Z

What do you mean by this?

It appears this code is using STCdisabled instead of the recently refactored STC.disabled. See the failure. That is why I assumed it needed to be rebased.

wilzbach · 2018-01-09T00:02:54Z

It appears this code is using STCdisabled instead of the recently refactored STC.disabled. See the failure. That is why I assumed it needed to be rebased.

Ah I see. Though we can easily do that and reduce the latency / "ping-pong" time ;-)

wilzbach · 2018-01-09T00:03:07Z

(I just rebased this PR)

ghost · 2018-01-11T16:36:27Z

Looks like there's a regression under OSX 32 bits, that happened between the 2018-01-10 22:21:41 and the 2018-01-10 11:23:11.

JackStouffer · 2018-01-11T16:55:16Z

Looks like there's a regression under OSX 32 bits

Can we please stop spending precious dev time and stalling PRs to support this dead platform?

wilzbach · 2018-01-11T16:59:14Z

Oh that's interesting. It looks similar to what @marler8997 is experiencing.
Yeah we definitely should switch that machine to do 64_64 testing only. CC @braddr
Alternatively we can also try to ping Brad here: https://github.com/braddr/d-tester/issues

marler8997 · 2018-01-11T17:03:43Z

Looking at the logs it is the same issue. I was able to reproduce on my macbook pro and got a stack trace. Note that this seg fault only occurs because the autotester is using dmd version 2.068.2, I was able to fix the problem on my macbook by using the next version, 2.069.0.

Here's the stack trace, maybe we should consider patching dmd 2.068.2?

Obj::term(char const*) + 2285
obj_end(Library*, File*) + 37
tryMain(unsigned long, char const**) + 12066
_start + 203
start + 33

marler8997 · 2018-01-11T17:10:00Z

P.S. I worked around the issue by adding some printf statements to a certain locations in the code :) It was hours of painstaking trial and error to massage the code to not cause the segfault. Tedious work, but no one else is going to do it for me.

braddr · 2018-01-11T20:31:25Z

reminder: I do not define the supported platforms. I only implement them and remind people when they want them changed for whatever reason that those changes must go through the community and leadership team. I'm part of the former but not the latter. So, go, get approval for changing the supported platform set.

…sabled

…sabled

ghost · 2018-01-13T07:50:48Z

@wilzbach, circle ci here too doesn't work anymore, although it used to. I did rebase or amend several times yesterday and it's not triggered anymore (as for semaphore).

wilzbach · 2018-01-13T07:55:08Z

circle ci here too doesn't work anymore

Interesting that that only happens with your account, but it seems like closing & reopening worked here...

ghost · 2018-01-13T07:57:16Z

Recently i revoked circle-ci from my account...but i don't think this could be the cause.

wilzbach · 2018-01-13T07:59:06Z

but i don't think this could be the cause.

It could also be related to the partial GitHub outage a couple of days ago:

https://status.github.com/messages

ghost requested a review from WalterBright as a code owner December 31, 2017 15:20

dlang-bot added the Enhancement label Dec 31, 2017

ghost changed the title ~~fix issue 18149 - Add a compiler trait to detect if a function is @disabled~~ fix issue 18149 - Add a compiler trait to detect if a function is @disable Jan 1, 2018

JinShil added Needs Changelog A changelog entry needs to be added to /changelog Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org labels Jan 1, 2018

ghost mentioned this pull request Jan 2, 2018

document possible new isDisabled __traits dlang/dlang.org#2042

Merged

JinShil removed the Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org label Jan 2, 2018

JinShil removed the Needs Changelog A changelog entry needs to be added to /changelog label Jan 3, 2018

JinShil approved these changes Jan 3, 2018

View reviewed changes

wilzbach added the Needs Approval label Jan 3, 2018

WalterBright reviewed Jan 5, 2018

View reviewed changes

WalterBright requested changes Jan 5, 2018

View reviewed changes

ghost mentioned this pull request Jan 5, 2018

[Trivial] add func attribs to class Declaration final helpers #7607

Merged

dlang-bot added the Needs Work label Jan 9, 2018

dlang-bot added Needs Work and removed Needs Work labels Jan 9, 2018

fix issue 18149 - Add a compiler trait to detect if a function is @di…

43ef5ce

…sabled

dlang-bot removed the Needs Work label Jan 12, 2018

wilzbach closed this Jan 13, 2018

wilzbach reopened this Jan 13, 2018

andralex approved these changes Jan 18, 2018

View reviewed changes

andralex added auto-merge and removed Needs Approval labels Jan 18, 2018

dlang-bot merged commit c68ae00 into dlang:master Jan 18, 2018

ghost deleted the issue-18149 branch April 23, 2018 13:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix issue 18149 - Add a compiler trait to detect if a function is @disable #7569

fix issue 18149 - Add a compiler trait to detect if a function is @disable #7569

ghost commented Dec 31, 2017 •

edited by ghost

Loading

dlang-bot commented Dec 31, 2017 •

edited

Loading

JinShil commented Jan 1, 2018 •

edited

Loading

ghost commented Jan 1, 2018

JinShil commented Jan 1, 2018

ghost commented Jan 2, 2018

JinShil commented Jan 2, 2018

PetarKirov commented Jan 2, 2018

ghost commented Jan 2, 2018

JinShil left a comment

wilzbach commented Jan 3, 2018

WalterBright Jan 5, 2018

WalterBright Jan 5, 2018

WalterBright left a comment

JinShil commented Jan 6, 2018

JinShil commented Jan 8, 2018

wilzbach commented Jan 8, 2018

JinShil commented Jan 8, 2018

wilzbach commented Jan 9, 2018

wilzbach commented Jan 9, 2018

ghost commented Jan 11, 2018 •

edited by ghost

Loading

JackStouffer commented Jan 11, 2018

wilzbach commented Jan 11, 2018

marler8997 commented Jan 11, 2018

marler8997 commented Jan 11, 2018 •

edited

Loading

braddr commented Jan 11, 2018

ghost commented Jan 13, 2018

wilzbach commented Jan 13, 2018

ghost commented Jan 13, 2018

wilzbach commented Jan 13, 2018

fix issue 18149 - Add a compiler trait to detect if a function is @disable #7569

fix issue 18149 - Add a compiler trait to detect if a function is @disable #7569

Conversation

ghost commented Dec 31, 2017 • edited by ghost Loading

dlang-bot commented Dec 31, 2017 • edited Loading

Bugzilla references

JinShil commented Jan 1, 2018 • edited Loading

ghost commented Jan 1, 2018

JinShil commented Jan 1, 2018

ghost commented Jan 2, 2018

JinShil commented Jan 2, 2018

PetarKirov commented Jan 2, 2018

ghost commented Jan 2, 2018

JinShil left a comment

Choose a reason for hiding this comment

wilzbach commented Jan 3, 2018

WalterBright Jan 5, 2018

Choose a reason for hiding this comment

WalterBright Jan 5, 2018

Choose a reason for hiding this comment

WalterBright left a comment

Choose a reason for hiding this comment

JinShil commented Jan 6, 2018

JinShil commented Jan 8, 2018

wilzbach commented Jan 8, 2018

JinShil commented Jan 8, 2018

wilzbach commented Jan 9, 2018

wilzbach commented Jan 9, 2018

ghost commented Jan 11, 2018 • edited by ghost Loading

JackStouffer commented Jan 11, 2018

wilzbach commented Jan 11, 2018

marler8997 commented Jan 11, 2018

marler8997 commented Jan 11, 2018 • edited Loading

braddr commented Jan 11, 2018

ghost commented Jan 13, 2018

wilzbach commented Jan 13, 2018

ghost commented Jan 13, 2018

wilzbach commented Jan 13, 2018

ghost commented Dec 31, 2017 •

edited by ghost

Loading

dlang-bot commented Dec 31, 2017 •

edited

Loading

JinShil commented Jan 1, 2018 •

edited

Loading

ghost commented Jan 11, 2018 •

edited by ghost

Loading

marler8997 commented Jan 11, 2018 •

edited

Loading