fix issue 20655 - attribute inference accepts unsafe union access as …

[aG0aep6G]

…@safe

[dlang-bot]

Thanks for your pull request and interest in making D better, @aG0aep6G! 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
✓	20655	regression	[REG: 2.072] attribute inference accepts unsafe union access as @safe

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 "stable + dmd#10884"

[aG0aep6G]

This is blocked by issue 20661, which is triggered more easily with this PR, but is also present without.

[schveiguy]

I just ran into this issue, while building code for my dconf 2020 talk. What is the status of this?

[aG0aep6G]

What is the status of this?

Still blocked by issue 20661.

[RazvanN7]

I have created: https://github.com/dlang/dmd/pull/12207 . Hope it unblocks this.

[RazvanN7]

@aG0aep6G #12207 was merged. Maybe you can have another look at this?

[aG0aep6G]

@aG0aep6G #12207 was merged. Maybe you can have another look at this?

That fix is in stable, but it's not in master yet. I'm targeting master here. Let me know if you want me to switch to stable. Otherwise, we'll have to wait until your fix hits master.

[RazvanN7]

Since this PR fixes a regression, procedure dictates that it should target stable.

[aG0aep6G]

now targeting stable

[RazvanN7]

This PR seems to break phobos.

[aG0aep6G]

This PR seems to break phobos.

A reduced version of what's failing:

enum isAssignable(alias T) = false;

struct Unique
{
    alias ValueType = typeof({ return a; }()); /* Error: circular `typeof` definition */
    static if (isAssignable!ValueType) {}
    int* a;
}

I'm not sure that's supposed to work. With int a;, the code is rejected with "Error: need this for a of type int". I don't see why it should work with a pointer.

[RazvanN7]

Plot twist. This compiles:

enum isAssignable(alias T) = false;

struct Unique
{
    int a;
    alias ValueType = typeof({ return a; }()); /* Error: circular `typeof` definition */
    static if (isAssignable!ValueType) {}

}

This shows that the code should probably compile with both int and int*.

[aG0aep6G]

If typeof({ return a; }()) must work, then I'm afraid this issue is getting too complex for me to fix.

[RazvanN7]

Maybe you can file this as a bug report and I will take a look at it hopefully next week.

[aG0aep6G]

Maybe you can file this as a bug report and I will take a look at it hopefully next week.

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

Note that fixing it doesn't necessarily unblock me here. If we consider issue 21680 a rejects-valid bug (which I think you're leaning towards), then I'm blocked here by my lack of DMD expertise, not by that issue.

[RazvanN7]

Thanks! I'm hoping that fixing that issue will make the phobos compile with this patch. I'll have to investigate further.

[RazvanN7]

@aG0aep6G could you please rebase this?

[aG0aep6G]

@aG0aep6G could you please rebase this?

rebased

[ibuclaw]

@aG0aep6G still interested in getting this over the finish line?

[aG0aep6G]

@aG0aep6G still interested in getting this over the finish line?

no

[dkorpel]

Rebooted here: https://github.com/dlang/dmd/pull/14827