Skip to content

add -safe switch#6097

Merged
andralex merged 1 commit intodlang:masterfrom
WalterBright:dash-safe
Sep 4, 2016
Merged

add -safe switch#6097
andralex merged 1 commit intodlang:masterfrom
WalterBright:dash-safe

Conversation

@WalterBright
Copy link
Member

The new return scope and other added @safe checking will require some updates to existing code, hence the need for this transitional switch.

@dlang-bot
Copy link
Contributor

@WalterBright, thanks for your PR! By analyzing the annotation information on this pull request, we identified @yebblies, @9rnsr and @mathias-lang-sociomantic to be potential reviewers. @yebblies: The PR was automatically assigned to you, please reassign it if you were identified mistakenly.

(The DLang Bot is under development. If you experience any issues, please open an issue at its repo.)

@wilzbach
Copy link
Contributor

What about the man page? AFAIK it's not updated automatically?

@WalterBright
Copy link
Member Author

done

@dnadlinger
Copy link
Contributor

Shouldn't this be -transition=<...>?

I'd also like to see a clear rationale and design before this is merged. The change log and doc entries are useless in their current form ("enhanced @safe checking"?).

@WalterBright
Copy link
Member Author

It will basically be difficult to continue developing DIP1000 without this switch. The switch will enable things that break too much existing code, like #5860

I'll probably have to tweak around just what it does based on experience.

@wilzbach
Copy link
Contributor

Shouldn't this be -transition=<...>?

I really like this -transition=.., because

  • it clearly conveys to the user what this switch is about (the word transition automatically implies that it will be enabled by default)
  • it's a nice grouping for experimental behavior
  • once the flag is removed, it will be moderately cheap to remove it completely and just let transition print a nice error message that helps the user ("Hey the transition you requested is over and now enabled by default)
  • we don't "burn" the flag yet (probably not so problematic in this case)

The switch will enable things that break too much existing code,

Btw in case you missed the announcement @skoppe is working on tooling to assess the caused "damage".

@andralex
Copy link
Member

What happened to the -dip convention?

@WalterBright
Copy link
Member Author

The -transition=???? seems much better.

This is blocking #5860

@WalterBright WalterBright added the Review:Blocking Other Work review and pulling should be a priority label Aug 31, 2016
)

$(LI $(LNAME2 dash_safe, Add -transition=safe switch.)
$(P Enables enhanced @safe checking, which will break some existing code.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please enumerate these enhancements? This is the change log after all.

Copy link
Member Author

Choose a reason for hiding this comment

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

What they will specifically be is unknown at the moment, because it depends on which of the @safe improvements will break significant amounts of code. The only thing that is clear is that we'll need this switch.

Copy link
Member

Choose a reason for hiding this comment

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

"@safe" should be in backquotes (anything not part of the English text should be adorned appropriately)

@andralex andralex merged commit 470121e into dlang:master Sep 4, 2016
andralex added a commit that referenced this pull request Sep 4, 2016
@andralex
Copy link
Member

andralex commented Sep 4, 2016

#6107

AndrejMitrovic added a commit that referenced this pull request Sep 4, 2016
@WalterBright WalterBright deleted the dash-safe branch September 4, 2016 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review:Blocking Other Work review and pulling should be a priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants