Skip to content
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

Unable to get Span without consuming Pair #223

Closed
sunjay opened this issue Apr 9, 2018 · 4 comments
Closed

Unable to get Span without consuming Pair #223

sunjay opened this issue Apr 9, 2018 · 4 comments

Comments

@sunjay
Copy link
Contributor

sunjay commented Apr 9, 2018

If getting a Span consumes a Pair, I have to choose between getting that Span and getting its inner Pairs. This is counter intuitive given that the two don't really seem related. Sometimes I want to look further into the contents of a pair and also get its position. Can we add a span(&self) method (or something) to get the Span of a Pair and still allow you to get its inner Pairs afterwards?

I would be happy to create a PR for this. 😄

@sunjay sunjay changed the title Unable to get Span without consuming pair Unable to get Span without consuming Pair Apr 9, 2018
@dragostis
Copy link
Contributor

dragostis commented Apr 9, 2018

I guess it does make sense for into_span to be refactored to as_span(&self). However, the current API let's you do what you want by simply cloning the Pair which is a cheap operation.

@sunjay
Copy link
Contributor Author

sunjay commented Apr 10, 2018

@dragostis The PR for this is ready: #224 😄 🎉

@frehberg
Copy link

Dito, the function "Pair::into_inner(self,..)" is consuming the pair. I would like to see a function in API just borrowing the pair, something like "Pair::into_inner(&self, ..)"

@CAD97
Copy link
Contributor

CAD97 commented Dec 13, 2018

This issue was about into_span, which got as_span, which actually does save an Arc clone. However, pair.clone().into_inner() is the exact process that a non-consuming Pair to inner Pairs would do, so adding a function just for that seems not helpful. The API today encourages the consuming option, which is cheaper, if the outside Pair is unneeded afterwards, which results in better code (or at the very least, less optimization work for the compiler).

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

No branches or pull requests

4 participants