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.
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
RFC: Generic member access for dyn Error trait objects #2895
base: master
Are you sure you want to change the base?
RFC: Generic member access for dyn Error trait objects #2895
Changes from 7 commits
39fc23e
63d0539
6d9a212
c1fd1f6
f344bd9
9726cfe
9dd4113
49fc1d0
f2073ac
1fc15e9
460c523
c8b80ed
f16045d
e12ba35
9581f78
6143f6d
1bb8ca7
e82ac82
53431f5
6451b1c
f74f64f
75ef121
ac0f94e
8d55678
7f87544
bcb4823
2ea013d
248e4ca
ef2e47e
d055bbb
ac79814
41b589b
defeafe
48adce6
0d441ac
7b98760
407ce78
84c8bf7
fb02f91
61be66b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
would be better to cite the mechanism this uses in addition to/instead of the name
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 think that might be more detail than is necessary for the RFC, I've merged it with the "alternatives to std::backtrace" bullet and it has a docs.rs link
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 is a strong and attractive claim but to me needs expansion/defense. i.e. are there really no other reasons to have it?
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.
Not that I can think of, the only other purpose it has is downcasting, so if you wanted a type that could contain an open set of errors where the only thing you do with them is downcast them back to concrete types you'd just use
dyn Any
.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'd start the walkthrough with the motivating output, much better headline
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.
Do you mean move this to the top of the guide level explanation? or to motivation / summary?
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.
Since these two lines don't really do anything, I assume they're stand-ins for "extracting Location types from errors gathered via #[track_caller] or similar" mentioned previously? Is there a link to a proposal for how that might be done? Maybe it's already possible and I just haven't been watching
track_caller
close enough?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.
correct. It is already possible to do this with
track_caller
, so we could update this to includetrack_caller
. I don't recall if there was a specific reason I avoided using track_caller here but I don't mind updating this example.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.
Is this misplaced?
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.
no, but I wasn't really sure how to put a code snippet in a bullet list