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

Rustup to rustc 1.16.0-nightly (24055d0f2 2017-01-31) #1505

Merged
merged 4 commits into from
Feb 4, 2017

Conversation

Mrmaxmeier
Copy link
Contributor

@Mrmaxmeier Mrmaxmeier commented Feb 3, 2017

@@ -275,9 +278,6 @@ impl<'a, 'tcx> Visitor<'tcx> for RefVisitor<'a, 'tcx> {

fn visit_ty(&mut self, ty: &'tcx Ty) {
match ty.node {
TyRptr(None, _) => {
self.record(&None);
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be TyRptr(ref lt, _) if lt.is_elided() =>

@@ -230,6 +230,9 @@ impl<'v, 't> RefVisitor<'v, 't> {
if let Some(ref lt) = *lifetime {
if &*lt.name.as_str() == "'static" {
self.lts.push(RefLt::Static);
} else if lt.name.as_u32() == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's lt.is_elided()

@@ -142,7 +142,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LintWithoutLintPass {


fn is_lint_ref_type(ty: &Ty) -> bool {
if let TyRptr(Some(_), MutTy { ty: ref inner, mutbl: MutImmutable }) = ty.node {
if let TyRptr(_, MutTy { ty: ref inner, mutbl: MutImmutable }) = ty.node {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs a !is_elided check on the lifetime.

@Mrmaxmeier Mrmaxmeier force-pushed the rustup-2017-01-31 branch 2 times, most recently from b3387b5 to c59abc3 Compare February 3, 2017 13:14
@oli-obk
Copy link
Contributor

oli-obk commented Feb 4, 2017

@eddyb: do you have any hints as to how lifetime elision changed in the following snippet?

fn trait_obj_elided2<'a>(_arg: &'a Drop) -> &'a str { unimplemented!() }

@eddyb
Copy link
Member

eddyb commented Feb 4, 2017

Whar are you talking about, there's no elision there.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 4, 2017

right. That was badly formulated. The lifetimes represented by rustc have changed, but I can't figure out how your code changes would even affect this code snippet.

@eddyb
Copy link
Member

eddyb commented Feb 4, 2017

Did something user-observed change, or just the compiler-internal representation?

If the latter, all lifetimes (in types) are explicit now.
So you have to check .is_elided() which returns false if the user wrote it.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 4, 2017

@eddyb: just compiler-internal representation changes. if the &'a Drop is passed to an intravisit::Visitor via visit_ty, it will encounter an elided lifetime (in visit_lifetime), which it did not encounter before.

@eddyb
Copy link
Member

eddyb commented Feb 4, 2017

Yeah, almost everything looking at a lifetime needs to ask .is_elided().

@Mrmaxmeier
Copy link
Contributor Author

I'm not sure if its possible to distinguish between these two cases without more context:

fn trait_obj_elided<'a>(_arg: &'a WithLifetime) -> &'a str;
fn trait_obj_elided2<'a>(_arg: &'a Drop) -> &'a str;

Both args are essentially the same:

TyRptr(
    lifetime('a, not elided),
    TyTraitObject( PolyTraitRef( .. ), lifetime(elided) )
)

@oli-obk
Copy link
Contributor

oli-obk commented Feb 4, 2017

I'm not sure if its possible to distinguish between these two cases without more context:

we don't need to, we just need to ignore the TyTraitObject lifetime in case it is elided. I took the liberty of pushing the patch directly to this PR.

@oli-obk oli-obk merged commit 49bf976 into rust-lang:master Feb 4, 2017
@Mrmaxmeier Mrmaxmeier deleted the rustup-2017-01-31 branch February 4, 2017 12:38
@mcarton
Copy link
Member

mcarton commented Feb 4, 2017

@oli-obk Could you publish a git tag when you publish a new version?

@oli-obk
Copy link
Contributor

oli-obk commented Feb 6, 2017

I tried to, it didn't accept leaving the message empty, so I wanted to check what you usually wrote into it, then I forgot about it...

Do you think it would make sense to stop filling in the CHANGELOG, and instead use the tagging feature to fill in the same information but in a structured manner?

@mcarton
Copy link
Member

mcarton commented Feb 6, 2017

Do you think it would make sense to stop filling in the CHANGELOG, and instead use the tagging feature to fill in the same information but in a structured manner?

Only signed tags need a message. I usually just write the version number.
I usually like to have a changelog that groups all versions in one document, because that makes it easy to find when something happened. It's also easy to add something in the changelog for an old version if we have forgotten something then. Tags are not that convenient.
But maybe we could just copy&paste the corresponding section from the changelog in the tag instead.

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

Successfully merging this pull request may close these issues.

breakage incoming: TyCtxt::map is now called TyCtxt::hir
4 participants