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

Feature: Member of Operator #16161

Closed
wants to merge 14 commits into from
Closed

Conversation

rikkimax
Copy link
Contributor

@rikkimax rikkimax commented Feb 5, 2024

This is a new feature, which I'm calling member of operator.
It is based upon Neat's Identifier Type.

In certain statements and expressions, there is a context, when seen with the operator it gets the member and rewrites the AST to use that instead.

Supports function calls, variable declarations, case statements for switch statements, and return expressions.
Dot expressions after it has not been implemented, although that should be done.

Why did I implement this? To prove that even I with limited knowledge can do it. It isn't a complex feature. It was less than 10 hours work with a bit of help from @dkorpel to point in the right direction!

My motivation is to make it be the mechanism to describe sum-type tags without a value type. It also applies to value type exceptions aka zero cost, to replace zero size struct support.

Test case which has all the examples in it:
master...rikkimax:dmd:member-of-property#diff-476a4c5f24136a55eb521b2a39faad10ce01dc3075af4f4ceceb82acd22242c3

But the difficult one that is worth copying Enum varF2 = identity(:Middle);.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @rikkimax! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

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

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 run digger -- build "master + dmd#16161"

@thewilsonator thewilsonator added Needs Approval 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 New Language Feature labels Feb 5, 2024
@thewilsonator thewilsonator added the WIP Work In Progress - not ready for review or pulling label Feb 6, 2024
@thewilsonator thewilsonator removed the WIP Work In Progress - not ready for review or pulling label Feb 14, 2024
Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

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

How much of the test case needs to actually be runnable? I suspect that most of it could be made compilable.

}

static assert(!__traits(compiles, { const fail = :max; }));
static assert(!__traits(compiles, { Context fail = :max.min; }));
Copy link
Contributor

Choose a reason for hiding this comment

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

silly question: why does this fail? this breaks my metal model of : as shorthand for typeof(lhs) (or typeof(return) as appropriate).

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like it might be more useful to have a token which represents the expected type, so you can call its constructor:
fun(_(5), _.some_member).

assert(0);
}

static assert(!__traits(compiles, { const fail = :max; }));
Copy link
Contributor

@thewilsonator thewilsonator Feb 14, 2024

Choose a reason for hiding this comment

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

should also check for auto, immutable, static, shared, __gshared (any other type redundant modifier I'm forgetting?).
Also should check for initialisation at

  • module scope,
  • class/struct scope
  • (other scopes I may be forgetting?)

@RazvanN7
Copy link
Contributor

@rikkimax What are your plans with regards to this feature? This looks like something that would require a DIP.

@rikkimax
Copy link
Contributor Author

The plan is to basically let this bitrot.

It exists to prove that it can be done without introducing a lot of complexity in the compiler.

@RazvanN7
Copy link
Contributor

Ok to close? There is no practical difference in keeping this open, apart from constantly appearing in the PR queue.

@dkorpel dkorpel added the Phantom Zone Has value/information for future work, but closed for now label Mar 13, 2024
@rikkimax rikkimax closed this Mar 13, 2024
@WalterBright
Copy link
Member

This feature can be used to “type-tag” entries in sumtypes, to differentiate identically typed entries, such as (:centimeters, int | :meters, int).

An alias can do that, too:

alias centimeters = int;
alias meters = int;

@schveiguy
Copy link
Member

Is there a drawback syntactically for using :Member? Like for ternary operations, named parameters, or array initializers?

Otherwise, I love this idea, and have loved it since I used it in Swift.

@rikkimax
Copy link
Contributor Author

Changelog added.

@rikkimax
Copy link
Contributor Author

Is there a drawback syntactically for using :Member? Like for ternary operations, named parameters, or array initializers?

Otherwise, I love this idea, and have loved it since I used it in Swift.

https://dlang.org/spec/arrays.html#ArrayInitializer

You need the identifier to proceed the :.

It only applies to the first term of an expression, so as far as I know there are no downsides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Approval 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 New Language Feature Phantom Zone Has value/information for future work, but closed for now Walter Bright
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants