Readd track_caller to Result::from_residual by yaahc · Pull Request #91752 · rus...

yaahc commented on Dec 10, 2021 •

This is a followup on #87401 in and an attempt to move the issue towards resolution.

As part of the overhaul of the Try trait we removed the ability for errors to grab location information during propagation via ? with the builtin std::result::Result. The previously linked issue has a fair bit of discussion into the reasons for and against the usage of #[track_caller] on the FromResidual impl on Result that I will do my best to summarize.

• #87401 (comment): Difficulties with using non std::result::Result like types
• #87401 (comment): Inconsistency with functionality provided for recoverable (Result) and non-recoverable errors (panic), where panic provides a location and Result does not, pushing some users towards using panic

Against

• #84277 (comment): concern that this will bloat callers that never use this data

Personally, I want to quantify the performance / bloat impact of re-adding this attribute, and fully evaluate the pros and cons before deciding if I need to switch eyre to have a custom Result type, which would also mean I need try_trait_v2 to be stabilized, cc @scottmcm. If the performance impact is minor enough in the general case I think I would prefer that the default Result type has the ability to track location information for consistency with panic error reporting, and leave it to applications that need particularly high performance to handle the micro optimizations of introducing their own efficient custom Result type or matching manually.

Alternatively, I wonder if the performance penalty on code that doesn't use the location information on FromResidual could be mitigated via new optimizations.