Skip to content

Comments

add restrictiveshared preview flags (hidden).#10117

Closed
thewilsonator wants to merge 1 commit intodlang:masterfrom
thewilsonator:preview-shared
Closed

add restrictiveshared preview flags (hidden).#10117
thewilsonator wants to merge 1 commit intodlang:masterfrom
thewilsonator:preview-shared

Conversation

@thewilsonator
Copy link
Contributor

This is currently hidden because it currently induces no change. It serves to section off changes related to shared and thread safety.

@thewilsonator thewilsonator requested a review from ibuclaw as a code owner July 1, 2019 06:58
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @thewilsonator! 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 annotated coverage diff directly on GitHub with CodeCov's browser extension
  • 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 fetch digger
dub run digger -- build "master + dmd#10117"

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

Can't we add the flag when there is an actual PR for it?

@WalterBright
Copy link
Member

We've never been calling it restrictive shared, more like atomic shared.

Copy link
Member

@WalterBright WalterBright left a comment

Choose a reason for hiding this comment

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

I'd call it atomicshared or just atomic.

@WalterBright
Copy link
Member

Can't we add the flag when there is an actual PR for it?

There'll be likely several PRs for it, as one big one will be too big to review. Progress is currently stalled by nobody willing to look at #10097

At this rate, it'll never happen :-(

@thewilsonator
Copy link
Contributor Author

We've never been calling it restrictive shared, more like atomic shared.

You've not been calling it restrictive. Possibly because the plan you have is not the plan the rest of the community has which has little to do with atomics other than as a basic building block (although it was noted that core.atomic is missing the weak forms of cmpxchg which we may or may not need to add, I'm not an atomic expert).

I don't really care what the preview flag is called provided it is clearly about shared, I didn't go with "shared", because we already have shared (and this flag is under -preview=) its just completely useless, but I suppose thats fine too.

"newshared", "nothorriblybrokenshared" are all fine with me, but I went with "restrictiveshared" because shared will become more restrictive (as in less operations are allowed on it).

You can paint the bikeshed any colour you like, so long as its "shared".

@thewilsonator
Copy link
Contributor Author

There'll be likely several PRs for it,

Definitely.

as one big one will be too big to review.

It'll be a miracle if one PR manages to get the semantics correct in one shot irrespective of its size or difficulty to review.

Progress is currently stalled by nobody willing to look at #10097

Thats a backend PR, practically no-one is qualified to review those. Also, thats for what you want to do. What the rest of us are doing is not thus handicapped.

At this rate, it'll never happen :-(

Oh, it will. Just not necessarily what you had in mind.

@TurkeyMan
Copy link
Contributor

I'd call it atomicshared or just atomic.

shared and atomic intrinsics are not related tasks.

@UplinkCoder
Copy link
Member

I need this compiler switch, can we please merge it?

@UplinkCoder
Copy link
Member

@WalterBright #10142 could a remove the static if, if this is merged.

@thewilsonator
Copy link
Contributor Author

Superseded by #10142

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.

6 participants