Skip to content

[scope] add @trusted and 'scope' annotations to std.array#5082

Merged
andralex merged 1 commit intodlang:masterfrom
WalterBright:scope-array
Feb 11, 2017
Merged

[scope] add @trusted and 'scope' annotations to std.array#5082
andralex merged 1 commit intodlang:masterfrom
WalterBright:scope-array

Conversation

@WalterBright
Copy link
Member

The @trusted is documented in the comments. The need for scope is there for join because the use of sep is convoluted enough the relatively simple inference engine in the compiler cannot infer it, so a little help is needed.

@WalterBright
Copy link
Member Author

With this change, and the other pending scope PRs, std.array will now compile its unit tests with -dip1000.

the input.
*/
ElementType!String[] array(String)(String str) if (isNarrowString!String)
@trusted ElementType!String[] array(String)(scope String str) if (isNarrowString!String)
Copy link
Contributor

@JackStouffer JackStouffer Feb 2, 2017

Choose a reason for hiding this comment

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

Applying @trusted to a template is really dangerous. What if toUTF32 becomes unsafe in the future? What if the template constraints are relaxed?

* is unsafe - converting a dstring[] to a dchar[], for example.
* Our special knowledge of toUTF32 is needed to know the cast is safe.
*/
return cast(typeof(return)) str.toUTF32;
Copy link
Contributor

Choose a reason for hiding this comment

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

The whole trusted thing can be avoided by

  1. Changing this line to return toUTFImpl!(dchar[])(str);
  2. Changing std.utf.toUTFImpl to be package rather than private

Copy link
Member

Choose a reason for hiding this comment

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

  1. Wrapping this cast in a @trusted lambda.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no need for the cast using my solution.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, you're right. Just thought I'd mention it.

Copy link
Member Author

Choose a reason for hiding this comment

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

toUTF32

Throws an Exception on invalid UTF. toUTFIml does not. I.e. the behavior is different.

trusted lambda

While that will work, it kinda is superfluous. The reason the cast is safe is because of special knowledge of how toUTF32 is implemented. Any changes should reflect knowledge of the whole, it isn't just about an unsafe cast.

Copy link
Member

Choose a reason for hiding this comment

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

This is indeed fragile but I'll allow it in the interest of moving forward.

@WalterBright
Copy link
Member Author

@andralex
Copy link
Member

Auto-merge toggled on

@andralex andralex dismissed JackStouffer’s stale review February 11, 2017 09:35

With apologies. @JackStouffer would you mind following up with a PR? Walter has a long backlog. Thanks!

@andralex
Copy link
Member

@WalterBright @CyberShadow what is the problem with the doc builder here? Thx.

@andralex andralex merged commit aca4b58 into dlang:master Feb 11, 2017
@CyberShadow
Copy link
Member

@WalterBright @CyberShadow what is the problem with the doc builder here? Thx.

Same problem as dlang/dmd#4745 (comment) (and everywhere else)...

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