-
-
Notifications
You must be signed in to change notification settings - Fork 747
std.net.curl: fix for -dip1000 #5052
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
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Technically speaking, isn't this fine because you're returning a reference to GC handled memory rather than a reference to the struct itself?
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.
As far as the compiler is concerned, you could have used
mallocto create the associative array (even though currently that's far from straightforward). The only thing that it cares about is that you're returning a reference to one of its members.scopeandreturnare orthogonal to@nogcAFAIU.The strange thing is that it looks like the inferred
scopeofthisis transitive, because you're not returning a reference to a field, but a reference to the object pointed by the field. @WalterBright can you shed some light on this?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.
Right, that's my issue with this. Because the AA isn't part of the struct and it's GC-ed this is perfectly safe.
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.
The root of an associative array is a pointer, and marking with
returntells the compiler that the lifetime of that pointer is tied to the lifetime of the struct. The lifetime of a pointer is as long as the pointer exists, which must be a subset of how long the value contained in the pointer is live.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.
But isn't the pointer to the AA not tied to the lifetime of the struct at all? The lifetime of the AA is as long as the GC doesn't collect it, right?
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.
This ties it to the lifetime of the struct.
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.
The point I'm trying to make is that there's no reason that I can see to do that, and I don't understand why we can't let the AA outlive the struct, seeing as how it will always be managed by the GC and therefore doesn't present a safety risk.
Adding
returnhere does not make this code any safer as far as I can tell.