-
-
Notifications
You must be signed in to change notification settings - Fork 747
make buildNormalizedPath() @safe #6357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for your pull request, @WalterBright! Bugzilla referencesYour 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 locallyIf 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 + phobos#6357" |
std/path.d
Outdated
| result = asNormalizedPath(result).array; | ||
| return cast(typeof(return)) result; | ||
| auto result = asNormalizedPath(chained).array; // .array returns a copy, so we know it is unique | ||
| return () @trusted { return assumeUnique(result); } (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize that this has become an accepted way to do @trusted. But I think it actually goes against the idea behind it.
The @trusted snippet doesn't ensure safety by itself. It relies on the rest of the code not messing with result. That means, you can trigger undefined behavior by editing @safe code. Exactly what should not be possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
6c64fc6 to
aefb700
Compare
| return cast(typeof(return)) result; | ||
| auto result = asNormalizedPath(chained); | ||
| // .array returns a copy, so we know it is unique | ||
| return () @trusted { return assumeUnique(result.array); } (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. If .array returns a unique array, how come the compiler can't infer it? I would have thought that it should be inferred as pure, and the compiler would have been able to implicitly convert it to immutable without a @trusted escape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pure isn't enough to infer the result is immutable.
aefb700 to
49216a2
Compare
Shrink the @trusted to the smallest bit of the function that needs it. The rest is to follow the convention that
resultis for the return value.