Issue 22300 - Allow shared <-> unshared reinterpreting casts during CTFE

[MoonlightSentinel]

CTFE evaluation is single threaded only, hence it's safe to discard the
shared qualifier in a reinterpreting cast.

This change allows -checkaction=context to use a reinterpreting cast
for compile and runtime - which removes the problematic workaround
that caused the regression.

CC @Geod24

[dlang-bot]

Thanks for your pull request and interest in making D better, @MoonlightSentinel! 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
✗	22300	regression	[REG 2.098-rc.2] `-checkaction=context` of a `shared` type with an `opCast` fails to compile

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#13203"

[thewilsonator]

cc @UplinkCoder

[UplinkCoder]

@MoonlightSentinel CTFE must not expose the undefined behavior.

[UplinkCoder]

@MoonlightSentinel I'd like to see the usecase first before coming to a decision is this is okay to do.

[MoonlightSentinel]

There's no undefined behavior involved. Casting away shared is simply a way for the programmer to declare that there won't be any race conditions when accessing the underlying storage (e.g. because it's guarded by proper synchronization primitives).

[MoonlightSentinel]

@MoonlightSentinel I'd like to see the usecase first before coming to a decision is this is okay to do.

It's intended as a fallback for code that would normally use atomicLoad but should also work during CTFE.

[UplinkCoder]

@MoonlightSentinel If you intended to have core.atomic "just work" at compile time I'd prefer to register them as builit-ins.
That way have the option of faulting if we statically detect a possible race.

[MoonlightSentinel]

@MoonlightSentinel If you intended to have core.atomic "just work" at compile time I'd prefer to register them as builit-ins.

That doesn't work for types not supported by atomicLoad.

[UplinkCoder]

I dislike the idea of allowing shared to be cast away at CTFE without having had time to think about it properly.
Can we -preview-flag this?

[MoonlightSentinel]

Certain constructs not working in CTFE is good if the results are not portable (rely on UB as you mentioned before). But removing shared in an environment known to be single threaded cannot result in UB.

Note that this PR retains the restriction for const / immutable

[UplinkCoder]

@MoonlightSentinel I'll merge it if it's behind a preview.

[MoonlightSentinel]

(Also note that this is already supported for basic integral types)

[UplinkCoder]

(Also note that this is already supported for basic integral types)

That's a concerning.

[ljmf00]

I don't think this is a problem. CTFE variables are evaluated in a single-threaded environment. Also, I think resolving this and other reinterpret cast issues are a good way forward to make CTFE less restrictive.

[Geod24]

I hate special cases, but it does have proven utility and no foreseeable downside.

[Geod24]

Oh, I just realized why you CCed me. Forgot about that issue. And yeah, we disabled ldc-master testing in our project over it.

[MoonlightSentinel]

I hate special cases, but it does have proven utility and no foreseeable downside.

Me neither. CTFE could even safely support mutable <-> immutable because it keeps track whether the referenced value is actually mutable and hence rejects invalid modifications.

But that's out of scope for this PR.