-
-
Notifications
You must be signed in to change notification settings - Fork 610
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
Fix issue20123 by allowing opUnaryRight to disable post-[inc|dec]rement #12301
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for your pull request and interest in making D better, @thewilsonator! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#12301" |
| void errorRet() | ||
| { | ||
| result = ErrorExp.get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know there is a method somewhere that does exactly this but I can't remember whats its called.
2b17cd0 to
9647cc2
Compare
9647cc2 to
f3206f3
Compare
| auto fdr = search_function(ad, Id.opUnaryRight); | ||
| if (!fdr) | ||
| return visit(cast(BinExp) pe); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from here on the function always returns an error. The control flow can be simplified to make that clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from here on the function always returns an error
correct.
The control flow can be simplified to make that clear.
How?
| /* TEST_OUTPUT: | ||
| --- | ||
| fail_compilation/issue20123.d(39): Error: can only `@disable` opUnaryRight | ||
| fail_compilation/issue20123.d(39): Error: can only `@disable` opUnaryRight |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A doesn't have disabled methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as in the only thing you are allowed to do with opUnaryRight is to @disable it. Perhaps that should be worded better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as in the only thing you are allowed to do with
opUnaryRightis to@disableit.
For the moment. Give it some days and people in the forums will demand it to be usable properly.
| fail_compilation/issue20123.d(39): Error: can only `@disable` opUnaryRight | ||
| fail_compilation/issue20123.d(39): Error: can only `@disable` opUnaryRight | ||
| fail_compilation/issue20123.d(40): Error: cannot be used because it is annotated with `@disable` | ||
| fail_compilation/issue20123.d(40): Error: cannot be used because it is annotated with `@disable` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WHAT cannot be used?
Also maybe add an errorSupplemental pointing to the disabled operator overload?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof. That needs work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments
|
@thewilsonator What is the status of this? |
|
waiting on input from @TurkeyMan |
|
ping @TurkeyMan |
cc @TurkeyMan