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

Don't crash in item_path when an item lacks a name #34561

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@arielb1
Copy link
Contributor

arielb1 commented Jun 29, 2016

Fixes #34027

We may want to use a better name than <unnamed>, but I just cherry-picked my old commit.

r? @brson

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jun 29, 2016

warning Warning warning

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!
@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jun 29, 2016

Backport-wise the protocol is typically to send a PR to master, tag it beta-nominated, and then cherry-pick it backwards once it's landed. (just FYI).

Also just to clarify, this is a cherry-pick of f6068ea from #33622, right?

@arielb1

This comment has been minimized.

Copy link
Contributor Author

arielb1 commented Jun 29, 2016

@alexcrichton

This already landed in master as part of a larger PR, with the motivation of unbreaking logging so I could debug some issue, so I can't really open a new one. This is indeed an unmodified cherry-pick of f6068ea.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jun 29, 2016

Ok cool, sounds good to me!

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jun 29, 2016

cc @rust-lang/compiler about backporting

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jun 29, 2016

sgtm

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 3, 2016

With a lack of r+, the fact that this is already on stable, and the fact that the stable 1.10 release is to be built tomorrow, this is relatively unlikely to be backported at this point. If it's critical to include, however, we can make it work.

@arielb1

This comment has been minimized.

Copy link
Contributor Author

arielb1 commented Jul 3, 2016

We want to include this - the regression is very annoying when it hits. Could @eddyb r+ this?

@arielb1

This comment has been minimized.

Copy link
Contributor Author

arielb1 commented Jul 3, 2016

This is indeed a regression to 1.9 (current stable). In that case, I don't think we should backport.

@arielb1 arielb1 closed this Jul 3, 2016

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 4, 2016

Ok, removing the beta-nominated tag from this as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.