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

rustc_typeck: correctly compute Substs for Res::SelfCtor. #61896

Merged
merged 1 commit into from
Jun 19, 2019

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Jun 16, 2019

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 16, 2019
impl A<bool> {
const B: A<u8> = Self(0);
//~^ ERROR mismatched types
//~| ERROR mismatched types
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to add the const B: Self = Self(0); case also for some redundancy?

I don't know what an interesting test for A<&'static T> would be... I have:

impl A<&'static u8> {
    fn f() -> Self {
        let x = 0;
        Self(&x)
    }
}

it fails with "cannot return value referencing local variable x".

Copy link
Member Author

Choose a reason for hiding this comment

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

You can't return it. Try Self(&x); - that should still error, even if you're not returning it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah good call; it doesn't error:

struct A<T>(T);

impl A<&'static u8> {
    fn f() {
        let x = 0;
        Self(&x);
    }
}

so you can add that to the test file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Decided to check your branch out to help speed things up... but Self(&x); for impl A<&'static u8> { still does not error.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does error, but it needs to be in a separate test because it doesn't borrow-check A::f if A::B doesn't type-check (we should probably remove this limitation and keep going? cc @estebank)

@petrochenkov
Copy link
Contributor

r=me with tests requested by Centril

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 16, 2019
@eddyb
Copy link
Member Author

eddyb commented Jun 18, 2019

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Jun 18, 2019

📌 Commit dedf2ed has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 18, 2019
@eddyb eddyb added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 18, 2019
@Centril Centril added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 18, 2019
Centril added a commit to Centril/rust that referenced this pull request Jun 18, 2019
…nkov

rustc_typeck: correctly compute `Substs` for `Res::SelfCtor`.

Fixes rust-lang#61882.

r? @petrochenkov cc @varkor
Centril added a commit to Centril/rust that referenced this pull request Jun 18, 2019
…nkov

rustc_typeck: correctly compute `Substs` for `Res::SelfCtor`.

Fixes rust-lang#61882.

r? @petrochenkov cc @varkor
bors added a commit that referenced this pull request Jun 19, 2019
Rollup of 11 pull requests

Successful merges:

 - #61505 (Only show methods that appear in `impl` blocks in the Implementors sections of trait doc pages)
 - #61701 (move stray run-pass const tests into const/ folder)
 - #61748 (Tweak transparent enums and unions diagnostic spans)
 - #61802 (Make MaybeUninit #[repr(transparent)])
 - #61839 (ci: Add a script for generating CPU usage graphs)
 - #61842 (Remove unnecessary lift calls)
 - #61843 (Turn down the myriad-closures test)
 - #61896 (rustc_typeck: correctly compute `Substs` for `Res::SelfCtor`.)
 - #61898 (syntax: Factor out common fields from `SyntaxExtension` variants)
 - #61938 (create an issue for miri even in status test-fail)
 - #61941 (Preserve generator and yield source for error messages)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Jun 19, 2019
Rollup of 11 pull requests

Successful merges:

 - #61505 (Only show methods that appear in `impl` blocks in the Implementors sections of trait doc pages)
 - #61701 (move stray run-pass const tests into const/ folder)
 - #61748 (Tweak transparent enums and unions diagnostic spans)
 - #61802 (Make MaybeUninit #[repr(transparent)])
 - #61839 (ci: Add a script for generating CPU usage graphs)
 - #61842 (Remove unnecessary lift calls)
 - #61843 (Turn down the myriad-closures test)
 - #61896 (rustc_typeck: correctly compute `Substs` for `Res::SelfCtor`.)
 - #61898 (syntax: Factor out common fields from `SyntaxExtension` variants)
 - #61938 (create an issue for miri even in status test-fail)
 - #61941 (Preserve generator and yield source for error messages)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Jun 19, 2019
Rollup of 11 pull requests

Successful merges:

 - #61505 (Only show methods that appear in `impl` blocks in the Implementors sections of trait doc pages)
 - #61701 (move stray run-pass const tests into const/ folder)
 - #61748 (Tweak transparent enums and unions diagnostic spans)
 - #61802 (Make MaybeUninit #[repr(transparent)])
 - #61839 (ci: Add a script for generating CPU usage graphs)
 - #61842 (Remove unnecessary lift calls)
 - #61843 (Turn down the myriad-closures test)
 - #61896 (rustc_typeck: correctly compute `Substs` for `Res::SelfCtor`.)
 - #61898 (syntax: Factor out common fields from `SyntaxExtension` variants)
 - #61938 (create an issue for miri even in status test-fail)
 - #61941 (Preserve generator and yield source for error messages)

Failed merges:

r? @ghost
@bors bors merged commit dedf2ed into rust-lang:master Jun 19, 2019
@eddyb eddyb deleted the correct-self-ctor branch June 19, 2019 14:37
@pnkfelix
Copy link
Member

discussed at T-compiler meeting. accepting for beta-backport.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 20, 2019
@pietroalbini
Copy link
Member

@eddyb there are conflicts applying this to the beta branch. Is this partial diff for src/librustc_typeck/check/mod.rs correct?

@@ -5255,12 +5209,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
 
         let tcx = self.tcx;
 
-        let res = match self.rewrite_self_ctor(res, span) {
-            Ok(res) => res,
-            Err(ErrorReported) => return (tcx.types.err, res),
-        };
         let path_segs = match res {
-            Res::Local(_) | Res::Upvar(..) => Vec::new(),
+            Res::Local(_) | Res::Upvar(..) | Res::SelfCtor(_) => vec![],
             Res::Def(kind, def_id) =>
                 AstConv::def_ids_for_value_path_segments(self, segments, self_ty, kind, def_id),
             _ => bug!("instantiate_value_path on {:?}", res),

@eddyb
Copy link
Member Author

eddyb commented Jun 23, 2019

@pietroalbini Yes, that seems correct. I could take a look at a backported PR, or open a backport PR myself if you prefer that.

@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 23, 2019
bors added a commit that referenced this pull request Jun 26, 2019
[beta] Rollup backports

Rolled up:

* [beta] Comment out dev key #61700

Cherry picked:

* Dont ICE on an attempt to use GAT without feature gate #61118
* Fix cfg(test) build for x86_64-fortanix-unknown-sgx #61503
* Handle index out of bound errors during const eval without panic #61598
* Hygienize macros in the standard library #61629
* Fix ICE involving mut references #61947
* rustc_typeck: correctly compute `Substs` for `Res::SelfCtor`. #61896
* Revert "Set test flag when rustdoc is running with --test option" #61199
* create a "provisional cache" to restore performance in the case of cycles #61754

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

With a generic struct, Self(x) works where x is of the wrong type argument
7 participants