Skip to content

Fix druntime -preview=nosharedaccess errors#23056

Open
atilaneves wants to merge 1 commit into
dlang:masterfrom
atilaneves:shared-codex
Open

Fix druntime -preview=nosharedaccess errors#23056
atilaneves wants to merge 1 commit into
dlang:masterfrom
atilaneves:shared-codex

Conversation

@atilaneves
Copy link
Copy Markdown
Contributor

@atilaneves atilaneves commented May 1, 2026

This makes druntime build and run its unittest target with -preview=nosharedaccess enabled.

@dlang-bot
Copy link
Copy Markdown
Contributor

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

Comment thread druntime/src/core/atomic.d Outdated
Comment thread memory.md Outdated
@atilaneves atilaneves force-pushed the shared-codex branch 6 times, most recently from bdaf3f7 to 7603e09 Compare May 4, 2026 11:26
Comment thread druntime/src/core/internal/newaa.d Outdated
@atilaneves atilaneves force-pushed the shared-codex branch 3 times, most recently from 91c4949 to b5c212a Compare May 4, 2026 16:07
@atilaneves atilaneves changed the title Fix druntime -preview=nosharedaccess errors Draft: Fix druntime -preview=nosharedaccess errors May 4, 2026
@atilaneves atilaneves changed the title Draft: Fix druntime -preview=nosharedaccess errors Fix druntime -preview=nosharedaccess errors May 4, 2026
@atilaneves atilaneves marked this pull request as draft May 4, 2026 17:46
@thewilsonator
Copy link
Copy Markdown
Contributor

Do ping when this is no longer draft

Comment thread druntime/src/core/internal/convert.d
Comment thread druntime/src/core/internal/hash.d Outdated
Comment thread druntime/src/core/internal/hash.d Outdated
Comment thread druntime/src/core/sync/condition.d Outdated
Comment thread druntime/src/core/sync/condition.d
Comment thread druntime/src/core/sync/mutex.d
Comment thread druntime/src/core/thread/fiber/base.d
Comment thread druntime/src/core/thread/fiber/base.d
Comment thread druntime/test/shared/src/finalize.d
@TurkeyMan
Copy link
Copy Markdown
Contributor

Thanks for moving on this! I added some comments.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@atilaneves atilaneves marked this pull request as ready for review May 18, 2026 18:45
@atilaneves
Copy link
Copy Markdown
Contributor Author

Do ping when this is no longer draft

ping

@TurkeyMan
Copy link
Copy Markdown
Contributor

TurkeyMan commented May 19, 2026

I don't think this is ready; the shared(bool[10]) nonsense exposed an implementation bug.


Repeated for visibility:

So...

import core.atomic : cas;

alias A = shared(bool)[10];
alias B = shared(bool[10]);

static assert(is(A == B));
static assert(A.stringof == "shared(bool[10])");
static assert(B.stringof == "shared(bool[10])");

That's surprising on impact... but I can see how it manifests. Is it correct though? Let's think this through.

It starts shared(int)* vs shared(int*); most certainly NOT the same thing.
And then shared(int)[] vs shared(int[]); should be the same as the pointer case.

These are obvious, because the unshared pointer/slice are a variable in its own right.
But... we have the static array case (which is inhibiting you from indexing it?), where that metadata is implicit, and denying you access to its address... which can't be right.

shared(int) i;
shared(int)* pi = &i; // obviously, we can handle the address of a shared thing!

// so, this should be legal:
shared(int[10]) s;
shared(int[10])* y = &s; // <- obviously acceptable
shared(int)[] x = s[];   // <- if it's not, then that's a bug; valid to take the address of a shared thing, 10 is just an int literal

shared(int)* p = x.ptr + 5; // <- clearly legal, x.ptr/length are not shared
shared(int)* q = &x[5];     // <- so, same operation? or does the compiler 'imagine' a value resolution before the `&`?

// and then these cases:
shared(int)* r = &s[5];  // obviously legal, and valid code can arrive here as shown above
shared(int)* s = &y[5];  // ditto

So, I think I'm convinced that is(shared(bool)[10] == shared(bool[10])), because the distinction between terms is purely metadata; but the bug is that you can't index a shared static-array.
You wouldn't be able to index a shared(T[]) (the pointer itself is shared), but you CAN index a shared(T[N]); the pointer is implicit information, so can't carry any notion of shared-ness.

Needs a language fix, then remove this mess :)

@TurkeyMan
Copy link
Copy Markdown
Contributor

TurkeyMan commented May 19, 2026

Also, the problem with class is substantial. Outside the fact that I think D got class totally wrong and it never never should have been a pointer implicitly, that's a serious problem here.

How do we distinguish a shared(class*) from a shared(class)*? (repeat for const and every other attribute!)
We can't, and we have to assume the conservative former expression, even though the latter expression is what the user wants 99 times out of 100.

With this in mind; shared(class) is a mostly useless concept, and the fact those sync primitives are classes (!!) is not workable. They need to be re-implemented as struct's. Probably new names as struct need to be created, and the class names deprecated. Functions that handle those sync primitives need overloads for the new type :/

We also DESPERATELY need struct inheritence! We've needed it forever, we need it more than ever.
Any argument that goes "but D avoided object slicing"; a problem you read about in the first 5-minutes of education, and no C++ programmer has EVER had a problem with in their career in reality... but our reality is that the cure is so much worse than the disease.

@rikkimax
Copy link
Copy Markdown
Contributor

A lot of these problems originate in a single decision, trying to force shared an already questionable in what it "proves" into representing atomics.

There is a reason why I have long held and suggested that we needed a dedicated atomic storage class.

The atomic nature of a cell does not reflect the atomic nature of any contained cells that are stored via a pointer.

Just keep in mind those sync classes, which support synchronized won't be supported as structs.
I tried to get approval to get synchronized converted to be operator overload based, as part of properly making the monitor a fully user defined field. But that was not approved.

@WalterBright
Copy link
Copy Markdown
Member

We also DESPERATELY need struct inheritence!

We already have it:

struct S
{
    A a;
    alias a this;
    ...
}

is equivalent to:

struct A { ... }
struct S : A
{
     ...
}

I've thought about replacing the former syntax with the latter many times.

@WalterBright
Copy link
Copy Markdown
Member

I'm confused. I do not see what the "mess" is. It appears to me that you are conflating a static array with a dynamic array. They are not the same at all. You correctly note that const behaves the same way. Is const broken? No. I don't see any issue with a mutable pointer to const, nor a mutable pointer to shared.

@WalterBright
Copy link
Copy Markdown
Member

Please do not bring up different issues within a PR. Each distinct issue should have its own Issue submission. It's fine to link to an issue here, but in order to track and resolve issues they need to be in the Issues, not comments in unrelated PRs.

@Herringway
Copy link
Copy Markdown
Contributor

We also DESPERATELY need struct inheritence!

We already have it:

struct S
{
    A a;
    alias a this;
    ...
}

is equivalent to:

struct A { ... }
struct S : A
{
     ...
}

I've thought about replacing the former syntax with the latter many times.

I prefer explicit composition. I've tripped over too many landmines using alias this, and it doesn't function at all in reverse - the only time I'd actually WANT to use the feature.

struct Foo {
  int a;
}
struct Bar {
  Foo f;
  alias this = f;
}
Bar fun(int z) {
  return Foo(z); // nope, that's an error
}

But this is a bit of a dead end. C'est la vie.

As for shared(T)[x] being equivalent to shared T[x], it makes sense to me since static arrays are effectively head-immutable, which would implicitly convert to head-const shared. From there, we have a head-shared array with shared members, so the whole thing is just shared, similar to how shared(shared(int)[]) == shared int[].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants