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

Fix issue 20850 - Can't assign enum of Tuple #7489

Merged
merged 1 commit into from
May 22, 2020

Conversation

Biotronic
Copy link
Contributor

@Biotronic Biotronic commented May 20, 2020

areCompatibleTuples has to check OriginalType to account for enums. Also, swap is confused, so we're not using that for these assignments. Casting to OriginalType would work, but breaks @safe.

@Biotronic Biotronic requested a review from MetaLang as a code owner May 20, 2020 21:46
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @Biotronic! 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

Auto-close Bugzilla Severity Description
20850 normal Can't assign enum of Tuple

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 + phobos#7489"

@schveiguy
Copy link
Member

I actually just envisioned that assignment to the same type would be supported. For instance, a type that alias this' itself to a Tuple should be assignable to that type.

@Biotronic
Copy link
Contributor Author

Biotronic commented May 21, 2020

If you mean this should work, it does:

import std.typecons;

struct S {
    alias t this;
    Tuple!int t;
}

unittest {
    Tuple!int a;
    S s;
    a = s;
}

Assigning s = a; doesn't work, but I'm pretty sure that can't be fixed in Tuple.

Strangely, a = S(); also doesn't work. That seems like a DMD issue though, since the above code works.

I don't think there's a way to allow this to work:

import std.typecons;

enum T1 : Tuple!(int*) { a = T1(null) }
enum T2 : Tuple!(int*) { a = T2(null) }

unittest {
    T1 t1;
    T2 t2;
    // Allow assignment from T1 to T1:
    t1 = T1.a;
    // But not T2 to T1:
    static assert(!__traits(compiles, t1 = T2.a));
}

@schveiguy
Copy link
Member

Well, that's not what I expected. That seems like an odd difference between enum and alias this.

@schveiguy
Copy link
Member

Copy link
Member

@schveiguy schveiguy left a comment

Choose a reason for hiding this comment

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

This workaround is OK for now. But I don't love the extra isTuple inside the static if. I get why you need to do it, it's unfortunate we can't figure out how to make the enum work with the existing operator overloads.

@dlang-bot dlang-bot merged commit 4f7f442 into dlang:master May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants