Skip to content

Conversation

@mihails-strasuns
Copy link

Trivial tweak to prevent reviewer confusion like in #6183 (comment)

Dicebot added 2 commits October 17, 2016 22:53
Original name was confusing in absence of "transition"
context and could lead into thinking it enables @safe checks.
@WalterBright
Copy link
Member

-transition=safe does not work without -dip25. I don't understand this reversion, anyway, because nobody uses -dip25.

@mihails-strasuns
Copy link
Author

This flag must not enable any new semantics, only print informational messages when depredations can't be used reliably. If you want to hide safety changes behind new flag, it will have to be a different one.

See also http://forum.dlang.org/post/ntvv1d$r9b$1@digitalmars.com

Note that PR targets stable branch which doesn't contain any new semantics enabled by this switch now.

@WalterBright
Copy link
Member

-transition=safe does not enable new semantics for -dip25, it enables a superset of them. It does not change -dip25.

@MartinNowak
Copy link
Member

MartinNowak commented Oct 19, 2016

@Dicebot point is that any -transition switch should only print additional messages to highlight semantic changes.
Instead -transition=safe now enables new semantics (-dip25 and more).

@mihails-strasuns
Copy link
Author

Correct. Your DIP1000 PR will have to be changed to use -scope or -preview=safe or something like that. -transition switch has a very specific reserved meaning.

Instead -transition=safe now enables new semantics (-dip25 and more).

Note that is was all cleaned in stable branch so the change will only make impact on master.

@mihails-strasuns
Copy link
Author

Ping

@WalterBright
Copy link
Member

I don't see the value in the rename. Why is vsafe less confusing?

Regardless of the name of the switch, it still needs to enable dip25 internally. It is not designed to work without dip25.

@mihails-strasuns
Copy link
Author

  1. DIP25 is becoming the default in next release so this is irrelevant (see http://forum.dlang.org/post/ntvv1d$r9b$1@digitalmars.com)
  2. Pick a different flag, all -transition ones serve a different purpose
  3. Name was confusing because it has lead reader reader to think it somehow enables @safe at least once (LinkedIn topic). New name is more consistent with vtls, serving similar purpose.

@MartinNowak MartinNowak merged commit 44fe0e1 into dlang:stable Oct 24, 2016
@MartinNowak
Copy link
Member

It's less confusing b/c it's not related to somehow turning on/off @safe.

@WalterBright
Copy link
Member

  1. DIP25 is becoming the default in next release so this is irrelevant

But it's not the default now and so now there's an unknown state where -transition=safe is thrown and -dip25 is not. This state was not designed for, there are no test cases for it, and should not exist.

@WalterBright
Copy link
Member

I don't particularly mind the vsafe change, and I would have pulled this but for the -dip25 change. It's another reason why independent changes should be in separate PRs.

@mihails-strasuns
Copy link
Author

so now there's an unknown state where -transition=safe is thrown and -dip25 is not

Not in stable branch. All places affected by -transition=safe here were adjusted in already merged #6183 - and I would recommend making dip25 the default in master already if @MartinNowak doesn't mind.

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.

3 participants