From 04c8191719761dd8617fb84a86fe114ed7ac6eab Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 1 Apr 2020 13:19:11 +0200 Subject: [PATCH 01/14] Explain how to work with subrepos --- CONTRIBUTING.md | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2843944b2e181..69ce9dae5aa9b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -188,7 +188,53 @@ with one another are rolled up. Speaking of tests, Rust has a comprehensive test suite. More information about it can be found [here][rctd]. -### External Dependencies +### External Dependencies (subrepo) + +As a developer to this repository, you don't have to treat the following external projects +differently from other crates that are directly in this repo: + +* none so far, see https://github.com/rust-lang/rust/issues/70651 for more info + +They are just regular files and directories. This is in contrast to `submodule` dependencies +(see below for those). + +If you want to synchronize or otherwise work with subrepos, install the `git subrepo` command via +instructions found at https://github.com/ingydotnet/git-subrepo + +#### Synchronizing a subrepo + +There are two synchronization directions: `subrepo push` and `subrepo pull`. Both operations create +a synchronization commit in the rustc repo. +This commit is very important in order to make future synchronizations work. +Do not rebase this commit under any circumstances. +Prefer to merge in case of conflicts or redo the operation if you really need to rebase. + +A `git subrepo push src/tools/clippy` +takes all the changes that +happened to the copy in this repo and creates commits on the remote repo that match the local +changes (so every local commit that touched the subrepo causes a commit on the remote repo). + +A `git subrepo pull src/tools/clippy` takes all changes since the last `subrepo pull` from the clippy +repo and creates a single commit in the rustc repo with all the changes. + +#### Creating a new subrepo dependency + +If you want to create a new subrepo dependency from an existing repository, call (from this +repository's root directory!!) + +``` +git subrepo clone https://github.com/rust-lang/rust-clippy.git src/tools/clippy +``` + +This will create a new commit, which you may not rebase under any circumstances! Delete the commit +and redo the operation if you need to rebase. + +Now you're done, the `src/tools/clippy` directory behaves as if clippy were part of the rustc +monorepo, so no one but you (or others that synchronize subrepos) needs to have `git subrepo` +installed. + + +### External Dependencies (submodules) Currently building Rust will also build the following external projects: @@ -221,7 +267,6 @@ before the PR is merged. Rust's build system builds a number of tools that make use of the internals of the compiler. This includes -[Clippy](https://github.com/rust-lang/rust-clippy), [RLS](https://github.com/rust-lang/rls) and [rustfmt](https://github.com/rust-lang/rustfmt). If these tools break because of your changes, you may run into a sort of "chicken and egg" From 56d0c81c750fa3cbe63f20bb30570cf084b9b98c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 1 Apr 2020 14:53:10 +0200 Subject: [PATCH 02/14] s/subrepo/subtree/ --- CONTRIBUTING.md | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 69ce9dae5aa9b..fb1d43c87112c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -188,7 +188,7 @@ with one another are rolled up. Speaking of tests, Rust has a comprehensive test suite. More information about it can be found [here][rctd]. -### External Dependencies (subrepo) +### External Dependencies (subtree) As a developer to this repository, you don't have to treat the following external projects differently from other crates that are directly in this repo: @@ -198,39 +198,39 @@ differently from other crates that are directly in this repo: They are just regular files and directories. This is in contrast to `submodule` dependencies (see below for those). -If you want to synchronize or otherwise work with subrepos, install the `git subrepo` command via -instructions found at https://github.com/ingydotnet/git-subrepo +If you want to synchronize or otherwise work with subtrees, install the `git subtree` command via +instructions found at https://github.com/ingydotnet/git-subtree -#### Synchronizing a subrepo +#### Synchronizing a subtree -There are two synchronization directions: `subrepo push` and `subrepo pull`. Both operations create -a synchronization commit in the rustc repo. -This commit is very important in order to make future synchronizations work. -Do not rebase this commit under any circumstances. -Prefer to merge in case of conflicts or redo the operation if you really need to rebase. +There are two synchronization directions: `subtree push` and `subtree pull`. -A `git subrepo push src/tools/clippy` +A `git subtree push -P src/tools/clippy` takes all the changes that happened to the copy in this repo and creates commits on the remote repo that match the local -changes (so every local commit that touched the subrepo causes a commit on the remote repo). +changes (so every local commit that touched the subtree causes a commit on the remote repo). -A `git subrepo pull src/tools/clippy` takes all changes since the last `subrepo pull` from the clippy +A `git subtree pull -P src/tools/clippy` takes all changes since the last `subtree pull` from the clippy repo and creates a single commit in the rustc repo with all the changes. -#### Creating a new subrepo dependency +You always need to specifiy the `-P` prefix to the subtree directory. If you specify the wrong directory +you'll get very fun merges that try to push the wrong directory to the remote repository. Luckily you +can just abort this without any consequences. -If you want to create a new subrepo dependency from an existing repository, call (from this +#### Creating a new subtree dependency + +If you want to create a new subtree dependency from an existing repository, call (from this repository's root directory!!) ``` -git subrepo clone https://github.com/rust-lang/rust-clippy.git src/tools/clippy +git subtree add -P src/tools/clippy https://github.com/rust-lang/rust-clippy.git master ``` This will create a new commit, which you may not rebase under any circumstances! Delete the commit and redo the operation if you need to rebase. Now you're done, the `src/tools/clippy` directory behaves as if clippy were part of the rustc -monorepo, so no one but you (or others that synchronize subrepos) needs to have `git subrepo` +monorepo, so no one but you (or others that synchronize subtrees) needs to have `git subtree` installed. From 521eb0df67bd2b822d7dbe5f4613a1e9beb98e1c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 1 Apr 2020 16:18:25 +0200 Subject: [PATCH 03/14] Git subtree is upstream, no need to install --- CONTRIBUTING.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fb1d43c87112c..be31cc26da531 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -198,9 +198,6 @@ differently from other crates that are directly in this repo: They are just regular files and directories. This is in contrast to `submodule` dependencies (see below for those). -If you want to synchronize or otherwise work with subtrees, install the `git subtree` command via -instructions found at https://github.com/ingydotnet/git-subtree - #### Synchronizing a subtree There are two synchronization directions: `subtree push` and `subtree pull`. @@ -230,8 +227,7 @@ This will create a new commit, which you may not rebase under any circumstances! and redo the operation if you need to rebase. Now you're done, the `src/tools/clippy` directory behaves as if clippy were part of the rustc -monorepo, so no one but you (or others that synchronize subtrees) needs to have `git subtree` -installed. +monorepo, so no one but you (or others that synchronize subtrees) needs actually use `git subtree`. ### External Dependencies (submodules) From af553528ade187a0f85bb3fa9ffb22420c630238 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 1 Apr 2020 17:33:20 +0200 Subject: [PATCH 04/14] Explain that you have to specify path and repository during subrepo synchronizations --- CONTRIBUTING.md | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index be31cc26da531..156ebdb0795c6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -196,23 +196,31 @@ differently from other crates that are directly in this repo: * none so far, see https://github.com/rust-lang/rust/issues/70651 for more info They are just regular files and directories. This is in contrast to `submodule` dependencies -(see below for those). +(see below for those). Only tool authors will actually use any operations here. #### Synchronizing a subtree There are two synchronization directions: `subtree push` and `subtree pull`. -A `git subtree push -P src/tools/clippy` +`git subtree push -P src/tools/clippy https://github.com/rust-lang/rust-clippy.git` + takes all the changes that happened to the copy in this repo and creates commits on the remote repo that match the local changes (so every local commit that touched the subtree causes a commit on the remote repo). -A `git subtree pull -P src/tools/clippy` takes all changes since the last `subtree pull` from the clippy -repo and creates a single commit in the rustc repo with all the changes. - -You always need to specifiy the `-P` prefix to the subtree directory. If you specify the wrong directory -you'll get very fun merges that try to push the wrong directory to the remote repository. Luckily you -can just abort this without any consequences. +`git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy.git` +takes all changes since the last `subtree pull` from the clippy +repo and adds these commits to the rustc repo + a merge commit with the existing changes. +It is recommended that you always do a push before a pull, so that the merge works without conflicts. +While definitely possible to resolve conflicts during a pull, you may have to redo the conflict +resolution if your PR doesn't get merged fast enough and there are new conflicts. Do not try to +rebase the result of a `git subtree pull`, rebasing merge commits is a bad idea in general. + +You always need to specify the `-P` prefix to the subtree directory and the corresponding remote +repository. If you specify the wrong directory or repository +you'll get very fun merges that try to push the wrong directory to the wrong remote repository. +Luckily you can just abort this without any consequences and try again. It is usually fairly obvious +that this is happening because you suddenly get thousands of commits that want to be synchronized. #### Creating a new subtree dependency From 59cfb8035c387bcf7297c08b78dd099dec2dd7f4 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 1 Apr 2020 17:45:47 +0200 Subject: [PATCH 05/14] Triple backticks --- CONTRIBUTING.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 156ebdb0795c6..486129ac4fada 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -202,13 +202,19 @@ They are just regular files and directories. This is in contrast to `submodule` There are two synchronization directions: `subtree push` and `subtree pull`. -`git subtree push -P src/tools/clippy https://github.com/rust-lang/rust-clippy.git` +``` +git subtree push -P src/tools/clippy https://github.com/rust-lang/rust-clippy.git +``` takes all the changes that happened to the copy in this repo and creates commits on the remote repo that match the local changes (so every local commit that touched the subtree causes a commit on the remote repo). -`git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy.git` +``` +git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy.git +``` + + takes all changes since the last `subtree pull` from the clippy repo and adds these commits to the rustc repo + a merge commit with the existing changes. It is recommended that you always do a push before a pull, so that the merge works without conflicts. From df91b40b43e6fb631f09a36d0d305585822eeb74 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 2 Apr 2020 17:20:56 +0200 Subject: [PATCH 06/14] Address review comments --- CONTRIBUTING.md | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 486129ac4fada..fee16a1764da5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -203,21 +203,27 @@ They are just regular files and directories. This is in contrast to `submodule` There are two synchronization directions: `subtree push` and `subtree pull`. ``` -git subtree push -P src/tools/clippy https://github.com/rust-lang/rust-clippy.git +git subtree push -P src/tools/clippy git@github.com:your-github-name/rust-clippy rustup ``` takes all the changes that happened to the copy in this repo and creates commits on the remote repo that match the local -changes (so every local commit that touched the subtree causes a commit on the remote repo). +changes. Every local commit that touched the subtree causes a commit on the remote repo, but is +modified to move the files from the specified directory to the tool repo root. + +Make sure to not pick the `master` branch, so you can open a normal PR to the tool to merge that +subrepo push. ``` -git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy.git +git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy master ``` +takes all changes since the last `subtree pull` from the tool repo +repo and adds these commits to the rustc repo + a merge commit that moves the tool changes into +the specified directory in the rust repository. -takes all changes since the last `subtree pull` from the clippy -repo and adds these commits to the rustc repo + a merge commit with the existing changes. -It is recommended that you always do a push before a pull, so that the merge works without conflicts. +It is recommended that you always do a push first and get that merged to the tool master branch. +Then, when you do a pull, the merge works without conflicts. While definitely possible to resolve conflicts during a pull, you may have to redo the conflict resolution if your PR doesn't get merged fast enough and there are new conflicts. Do not try to rebase the result of a `git subtree pull`, rebasing merge commits is a bad idea in general. From 7928c45c062b330f4654faaeedac3722bb71ee3a Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 2 Apr 2020 18:33:48 +0200 Subject: [PATCH 07/14] Address reviews --- CONTRIBUTING.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fee16a1764da5..051f5af7bc105 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -203,7 +203,7 @@ They are just regular files and directories. This is in contrast to `submodule` There are two synchronization directions: `subtree push` and `subtree pull`. ``` -git subtree push -P src/tools/clippy git@github.com:your-github-name/rust-clippy rustup +git subtree push -P src/tools/clippy git@github.com:your-github-name/rust-clippy sync-from-rust ``` takes all the changes that @@ -211,8 +211,8 @@ happened to the copy in this repo and creates commits on the remote repo that ma changes. Every local commit that touched the subtree causes a commit on the remote repo, but is modified to move the files from the specified directory to the tool repo root. -Make sure to not pick the `master` branch, so you can open a normal PR to the tool to merge that -subrepo push. +Make sure to not pick the `master` branch on the tool repo, so you can open a normal PR to the tool +to merge that subrepo push. ``` git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy master @@ -224,20 +224,21 @@ the specified directory in the rust repository. It is recommended that you always do a push first and get that merged to the tool master branch. Then, when you do a pull, the merge works without conflicts. -While definitely possible to resolve conflicts during a pull, you may have to redo the conflict +While it's definitely possible to resolve conflicts during a pull, you may have to redo the conflict resolution if your PR doesn't get merged fast enough and there are new conflicts. Do not try to rebase the result of a `git subtree pull`, rebasing merge commits is a bad idea in general. You always need to specify the `-P` prefix to the subtree directory and the corresponding remote repository. If you specify the wrong directory or repository you'll get very fun merges that try to push the wrong directory to the wrong remote repository. -Luckily you can just abort this without any consequences and try again. It is usually fairly obvious +Luckily you can just abort this without any consequences by throwing away either the pulled commits +in rustc or the pushed branch on the remote and try again. It is usually fairly obvious that this is happening because you suddenly get thousands of commits that want to be synchronized. #### Creating a new subtree dependency If you want to create a new subtree dependency from an existing repository, call (from this -repository's root directory!!) +repository's root directory!) ``` git subtree add -P src/tools/clippy https://github.com/rust-lang/rust-clippy.git master @@ -247,7 +248,7 @@ This will create a new commit, which you may not rebase under any circumstances! and redo the operation if you need to rebase. Now you're done, the `src/tools/clippy` directory behaves as if clippy were part of the rustc -monorepo, so no one but you (or others that synchronize subtrees) needs actually use `git subtree`. +monorepo, so no one but you (or others that synchronize subtrees) actually needs to use `git subtree`. ### External Dependencies (submodules) From 795fa0a006bed5526e762a8bb11284da96ddf57d Mon Sep 17 00:00:00 2001 From: marmeladema Date: Sun, 12 Apr 2020 21:22:30 +0100 Subject: [PATCH 08/14] Remove useless calls to `assemble_extension_candidates_for_traits_in_scope` Calls to `assemble_extension_candidates_for_traits_in_scope` with `DUMMY_HIR_ID` as `expr_hir_id` are useless because the first thing that this function does is to return `Ok(())` in this case. --- src/librustc_typeck/check/method/probe.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 7e7d84c199676..0b1aff7e806cf 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -1513,7 +1513,6 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { ); pcx.allow_similar_names = true; pcx.assemble_inherent_candidates(); - pcx.assemble_extension_candidates_for_traits_in_scope(hir::DUMMY_HIR_ID)?; let method_names = pcx.candidate_method_names(); pcx.allow_similar_names = false; @@ -1523,10 +1522,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { pcx.reset(); pcx.method_name = Some(method_name); pcx.assemble_inherent_candidates(); - pcx.assemble_extension_candidates_for_traits_in_scope(hir::DUMMY_HIR_ID) - .map_or(None, |_| { - pcx.pick_core().and_then(|pick| pick.ok()).map(|pick| pick.item) - }) + pcx.pick_core().and_then(|pick| pick.ok()).map(|pick| pick.item) }) .collect(); From 830e4fde0f11c8e8cf177729efac356429ffb0eb Mon Sep 17 00:00:00 2001 From: marmeladema Date: Mon, 13 Apr 2020 11:12:57 +0100 Subject: [PATCH 09/14] Remove usage of `DUMMY_HIR_ID` in some visitors --- src/librustc_lint/builtin.rs | 8 ++++---- src/librustc_lint/types.rs | 10 +++++----- src/librustc_privacy/lib.rs | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 910d53880f22d..627a438c2c3b1 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1354,7 +1354,7 @@ declare_lint! { } pub struct UnnameableTestItems { - boundary: hir::HirId, // HirId of the item under which things are not nameable + boundary: Option, // HirId of the item under which things are not nameable items_nameable: bool, } @@ -1362,7 +1362,7 @@ impl_lint_pass!(UnnameableTestItems => [UNNAMEABLE_TEST_ITEMS]); impl UnnameableTestItems { pub fn new() -> Self { - Self { boundary: hir::DUMMY_HIR_ID, items_nameable: true } + Self { boundary: None, items_nameable: true } } } @@ -1372,7 +1372,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnnameableTestItems { if let hir::ItemKind::Mod(..) = it.kind { } else { self.items_nameable = false; - self.boundary = it.hir_id; + self.boundary = Some(it.hir_id); } return; } @@ -1385,7 +1385,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnnameableTestItems { } fn check_item_post(&mut self, _cx: &LateContext<'_, '_>, it: &hir::Item<'_>) { - if !self.items_nameable && self.boundary == it.hir_id { + if !self.items_nameable && self.boundary == Some(it.hir_id) { self.items_nameable = true; } } diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index aa805a2f2dbc0..ee2ed8826ba12 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -43,14 +43,14 @@ declare_lint! { #[derive(Copy, Clone)] pub struct TypeLimits { /// Id of the last visited negated expression - negated_expr_id: hir::HirId, + negated_expr_id: Option, } impl_lint_pass!(TypeLimits => [UNUSED_COMPARISONS, OVERFLOWING_LITERALS]); impl TypeLimits { pub fn new() -> TypeLimits { - TypeLimits { negated_expr_id: hir::DUMMY_HIR_ID } + TypeLimits { negated_expr_id: None } } } @@ -244,7 +244,7 @@ fn lint_int_literal<'a, 'tcx>( let int_type = t.normalize(cx.sess().target.ptr_width); let (min, max) = int_ty_range(int_type); let max = max as u128; - let negative = type_limits.negated_expr_id == e.hir_id; + let negative = type_limits.negated_expr_id == Some(e.hir_id); // Detect literal value out of range [min, max] inclusive // avoiding use of -min to prevent overflow/panic @@ -397,8 +397,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypeLimits { match e.kind { hir::ExprKind::Unary(hir::UnOp::UnNeg, ref expr) => { // propagate negation, if the negation itself isn't negated - if self.negated_expr_id != e.hir_id { - self.negated_expr_id = expr.hir_id; + if self.negated_expr_id != Some(e.hir_id) { + self.negated_expr_id = Some(expr.hir_id); } } hir::ExprKind::Binary(binop, ref l, ref r) => { diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index a6d880667adf2..e02e97be19db5 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1012,7 +1012,7 @@ impl DefIdVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'_, 'tcx> { struct NamePrivacyVisitor<'a, 'tcx> { tcx: TyCtxt<'tcx>, tables: &'a ty::TypeckTables<'tcx>, - current_item: hir::HirId, + current_item: Option, empty_tables: &'a ty::TypeckTables<'tcx>, } @@ -1028,7 +1028,7 @@ impl<'a, 'tcx> NamePrivacyVisitor<'a, 'tcx> { ) { // definition of the field let ident = Ident::new(kw::Invalid, use_ctxt); - let current_hir = self.current_item; + let current_hir = self.current_item.unwrap(); let def_id = self.tcx.adjust_ident_and_get_scope(ident, def.did, current_hir).1; if !def.is_enum() && !field.vis.is_accessible_from(def_id, self.tcx) { let label = if in_update_syntax { @@ -1074,7 +1074,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NamePrivacyVisitor<'a, 'tcx> { } fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { - let orig_current_item = mem::replace(&mut self.current_item, item.hir_id); + let orig_current_item = mem::replace(&mut self.current_item, Some(item.hir_id)); let orig_tables = mem::replace(&mut self.tables, item_tables(self.tcx, item.hir_id, self.empty_tables)); intravisit::walk_item(self, item); @@ -2059,7 +2059,7 @@ fn check_mod_privacy(tcx: TyCtxt<'_>, module_def_id: DefId) { let mut visitor = NamePrivacyVisitor { tcx, tables: &empty_tables, - current_item: hir::DUMMY_HIR_ID, + current_item: None, empty_tables: &empty_tables, }; let (module, span, hir_id) = tcx.hir().get_module(module_def_id); From 95fb7bf1081850873a7db513a1087e5cc00897b5 Mon Sep 17 00:00:00 2001 From: marmeladema Date: Sun, 12 Apr 2020 22:49:34 +0100 Subject: [PATCH 10/14] Just `unwrap()` instead of `unwrap_or(DUMMY_HIR_ID)`. A valid hir id should always be returned in this case. --- src/librustc_infer/infer/error_reporting/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_infer/infer/error_reporting/mod.rs b/src/librustc_infer/infer/error_reporting/mod.rs index db81ceea43f01..4189570a0da58 100644 --- a/src/librustc_infer/infer/error_reporting/mod.rs +++ b/src/librustc_infer/infer/error_reporting/mod.rs @@ -191,7 +191,7 @@ fn msg_span_from_early_bound_and_free_regions( let sm = tcx.sess.source_map(); let scope = region.free_region_binding_scope(tcx); - let node = tcx.hir().as_local_hir_id(scope).unwrap_or(hir::DUMMY_HIR_ID); + let node = tcx.hir().as_local_hir_id(scope).unwrap(); let tag = match tcx.hir().find(node) { Some(Node::Block(_)) | Some(Node::Expr(_)) => "body", Some(Node::Item(it)) => item_scope_tag(&it), From e08ad7ae58b4b94e29a71dedf36d73ed7f93dd79 Mon Sep 17 00:00:00 2001 From: marmeladema Date: Mon, 13 Apr 2020 11:09:25 +0100 Subject: [PATCH 11/14] Use `CRATE_HIR_ID` instead of `DUMMY_HIR_ID` when appropriate. Those usage ends up forwarded to a `ObligationClause` which uses `CRATE_HIR_ID` for dummy value as seen in `ObligationClause::dummy`. --- src/librustc_traits/implied_outlives_bounds.rs | 2 +- src/librustc_ty/ty.rs | 2 +- src/librustc_typeck/check/method/probe.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_traits/implied_outlives_bounds.rs b/src/librustc_traits/implied_outlives_bounds.rs index 33ecbe72a8c4f..6db2e557fea69 100644 --- a/src/librustc_traits/implied_outlives_bounds.rs +++ b/src/librustc_traits/implied_outlives_bounds.rs @@ -62,7 +62,7 @@ fn compute_implied_outlives_bounds<'tcx>( // unresolved inference variables here anyway, but there might be // during typeck under some circumstances.) let obligations = - wf::obligations(infcx, param_env, hir::DUMMY_HIR_ID, ty, DUMMY_SP).unwrap_or(vec![]); + wf::obligations(infcx, param_env, hir::CRATE_HIR_ID, ty, DUMMY_SP).unwrap_or(vec![]); // N.B., all of these predicates *ought* to be easily proven // true. In fact, their correctness is (mostly) implied by diff --git a/src/librustc_ty/ty.rs b/src/librustc_ty/ty.rs index aefe61f60b87a..4cb7b7d8fda24 100644 --- a/src/librustc_ty/ty.rs +++ b/src/librustc_ty/ty.rs @@ -265,7 +265,7 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> { let unnormalized_env = ty::ParamEnv::new(tcx.intern_predicates(&predicates), traits::Reveal::UserFacing, None); - let body_id = tcx.hir().as_local_hir_id(def_id).map_or(hir::DUMMY_HIR_ID, |id| { + let body_id = tcx.hir().as_local_hir_id(def_id).map_or(hir::CRATE_HIR_ID, |id| { tcx.hir().maybe_body_owned_by(id).map_or(id, |body| body.hir_id) }); let cause = traits::ObligationCause::misc(tcx.def_span(def_id), body_id); diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 0b1aff7e806cf..a77d8981d028c 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -452,7 +452,7 @@ fn method_autoderef_steps<'tcx>( tcx.infer_ctxt().enter_with_canonical(DUMMY_SP, &goal, |ref infcx, goal, inference_vars| { let ParamEnvAnd { param_env, value: self_ty } = goal; - let mut autoderef = Autoderef::new(infcx, param_env, hir::DUMMY_HIR_ID, DUMMY_SP, self_ty) + let mut autoderef = Autoderef::new(infcx, param_env, hir::CRATE_HIR_ID, DUMMY_SP, self_ty) .include_raw_pointers() .silence_errors(); let mut reached_raw_pointer = false; From 38bfba659e2ae9423c9d782806db11e23187f3f6 Mon Sep 17 00:00:00 2001 From: Samrat Man Singh Date: Mon, 13 Apr 2020 22:14:08 +0530 Subject: [PATCH 12/14] Add test case for type aliasing `impl Sized` --- .../type-alias-impl-trait-sized.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 src/test/ui/type-alias-impl-trait/type-alias-impl-trait-sized.rs diff --git a/src/test/ui/type-alias-impl-trait/type-alias-impl-trait-sized.rs b/src/test/ui/type-alias-impl-trait/type-alias-impl-trait-sized.rs new file mode 100644 index 0000000000000..c54df664243e7 --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/type-alias-impl-trait-sized.rs @@ -0,0 +1,17 @@ +// check-pass + +#![feature(type_alias_impl_trait)] + +type A = impl Sized; +fn f1() -> A { 0 } + +type B = impl ?Sized; +fn f2() -> &'static B { &[0] } + +type C = impl ?Sized + 'static; +fn f3() -> &'static C { &[0] } + +type D = impl ?Sized; +fn f4() -> &'static D { &1 } + +fn main() {} From d0409ead2f44306fd54b12ed0ac83a4f5ee44910 Mon Sep 17 00:00:00 2001 From: marmeladema Date: Mon, 13 Apr 2020 17:57:39 +0100 Subject: [PATCH 13/14] Remove usage of `DUMMY_HIR_ID` in `FnCtxt::get_conversion_methods` --- src/librustc_typeck/check/demand.rs | 10 +++------- src/librustc_typeck/check/mod.rs | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 369bb183bcd73..781d25099fd31 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -217,14 +217,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { span: Span, expected: Ty<'tcx>, checked_ty: Ty<'tcx>, + hir_id: hir::HirId, ) -> Vec { - let mut methods = self.probe_for_return_type( - span, - probe::Mode::MethodCall, - expected, - checked_ty, - hir::DUMMY_HIR_ID, - ); + let mut methods = + self.probe_for_return_type(span, probe::Mode::MethodCall, expected, checked_ty, hir_id); methods.retain(|m| { self.has_no_input_arg(m) && self diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 4b5953b5e958c..5857ccf589710 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5000,7 +5000,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } else if !self.check_for_cast(err, expr, found, expected) { let is_struct_pat_shorthand_field = self.is_hir_id_from_struct_pattern_shorthand_field(expr.hir_id, expr.span); - let methods = self.get_conversion_methods(expr.span, expected, found); + let methods = self.get_conversion_methods(expr.span, expected, found, expr.hir_id); if let Ok(expr_text) = self.sess().source_map().span_to_snippet(expr.span) { let mut suggestions = iter::repeat(&expr_text) .zip(methods.iter()) From 50eb39757e471ed50ab34862af776f044c88222d Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Mon, 13 Apr 2020 22:32:40 +0200 Subject: [PATCH 14/14] allow const generics in const fn --- src/librustc_ast_passes/ast_validation.rs | 25 +------------------ .../const-fn-with-const-param.rs | 4 +-- .../const-fn-with-const-param.stderr | 17 ++----------- 3 files changed, 5 insertions(+), 41 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 9563325fe329e..395fd7460850f 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -561,28 +561,6 @@ impl<'a> AstValidator<'a> { } } - /// We currently do not permit const generics in `const fn`, - /// as this is tantamount to allowing compile-time dependent typing. - /// - /// FIXME(const_generics): Is this really true / necessary? Discuss with @varkor. - /// At any rate, the restriction feels too syntactic. Consider moving it to e.g. typeck. - fn check_const_fn_const_generic(&self, span: Span, sig: &FnSig, generics: &Generics) { - if let Const::Yes(const_span) = sig.header.constness { - // Look for const generics and error if we find any. - for param in &generics.params { - if let GenericParamKind::Const { .. } = param.kind { - self.err_handler() - .struct_span_err( - span, - "const parameters are not permitted in const functions", - ) - .span_label(const_span, "`const` because of this") - .emit(); - } - } - } - } - fn check_item_named(&self, ident: Ident, kind: &str) { if ident.name != kw::Underscore { return; @@ -966,9 +944,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> { .emit(); } } - ItemKind::Fn(def, ref sig, ref generics, ref body) => { + ItemKind::Fn(def, _, _, ref body) => { self.check_defaultness(item.span, def); - self.check_const_fn_const_generic(item.span, sig, generics); if body.is_none() { let msg = "free function without a body"; diff --git a/src/test/ui/const-generics/const-fn-with-const-param.rs b/src/test/ui/const-generics/const-fn-with-const-param.rs index e9e236be55630..3d8b77bcf7b47 100644 --- a/src/test/ui/const-generics/const-fn-with-const-param.rs +++ b/src/test/ui/const-generics/const-fn-with-const-param.rs @@ -1,11 +1,11 @@ +// run-pass #![feature(const_generics)] //~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash const fn const_u32_identity() -> u32 { - //~^ ERROR const parameters are not permitted in const functions X } fn main() { - println!("{:?}", const_u32_identity::<18>()); + assert_eq!(const_u32_identity::<18>(), 18); } diff --git a/src/test/ui/const-generics/const-fn-with-const-param.stderr b/src/test/ui/const-generics/const-fn-with-const-param.stderr index e944b02101e8c..64b9c18a8f525 100644 --- a/src/test/ui/const-generics/const-fn-with-const-param.stderr +++ b/src/test/ui/const-generics/const-fn-with-const-param.stderr @@ -1,23 +1,10 @@ -error: const parameters are not permitted in const functions - --> $DIR/const-fn-with-const-param.rs:4:1 - | -LL | const fn const_u32_identity() -> u32 { - | ^---- - | | - | _`const` because of this - | | -LL | | -LL | | X -LL | | } - | |_^ - warning: the feature `const_generics` is incomplete and may cause the compiler to crash - --> $DIR/const-fn-with-const-param.rs:1:12 + --> $DIR/const-fn-with-const-param.rs:2:12 | LL | #![feature(const_generics)] | ^^^^^^^^^^^^^^ | = note: `#[warn(incomplete_features)]` on by default -error: aborting due to previous error; 1 warning emitted +warning: 1 warning emitted