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.

Sign up for GitHub

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

Jump to bottom

remove some const arg in ty dep path boilerplate #74404

Merged
merged 3 commits into from
Jul 23, 2020
Merged

remove some const arg in ty dep path boilerplate #74404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ebc0c15
remove some const arg in ty dep path boilerplate
lcnr Jul 16, 2020
38e9e62
`try_update` -> `try_upgrade`
lcnr Jul 17, 2020
4bffe8f
fetch -> lookup
lcnr Jul 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/librustc_middle/ty/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1617,12 +1617,33 @@ pub struct WithOptConstParam<T> {

impl<T> WithOptConstParam<T> {
/// Creates a new `WithOptConstParam` setting `const_param_did` to `None`.
#[inline(always)]
pub fn unknown(did: T) -> WithOptConstParam<T> {
Copy link
Contributor

@tesuji tesuji Jul 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think the general guideline is not to inline for generic function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm... this function is only instantiated for WithOptConstParam<DefId> and WithOptConstParam<LocalDefId>.

Would be kind of sad if this is a problem :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait, isn't this only for code in the standard library as inline causes that to get included in every CGU? I don't think that policy counts for compiler internals afaik 🤔

Might be misremembering something here though

WithOptConstParam { did, const_param_did: None }
}
}

impl WithOptConstParam<LocalDefId> {
/// Returns `Some((did, param_did))` if `def_id` is a const argument,
/// `None` otherwise.
#[inline(always)]
pub fn try_lookup(did: LocalDefId, tcx: TyCtxt<'_>) -> Option<(LocalDefId, DefId)> {
tcx.opt_const_param_of(did).map(|param_did| (did, param_did))
}

/// In case `self` is unknown but `self.did` is a const argument, this returns
/// a `WithOptConstParam` with the correct `const_param_did`.
#[inline(always)]
pub fn try_upgrade(self, tcx: TyCtxt<'_>) -> Option<WithOptConstParam<LocalDefId>> {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, so if we wanted this name to be more explicit, it could be try_upgrade_to_const_arg, right?
Then the other one could be try_lookup_as_const_arg.

Alternatively, try_upgrade_with_const_param and try_lookup_const_param_for.

if self.const_param_did.is_none() {
if let const_param_did @ Some(_) = tcx.opt_const_param_of(self.did) {
return Some(WithOptConstParam { did: self.did, const_param_did });
}
}

None
}

pub fn to_global(self) -> WithOptConstParam<DefId> {
WithOptConstParam { did: self.did.to_def_id(), const_param_did: self.const_param_did }
}
Expand Down
14 changes: 7 additions & 7 deletions src/librustc_mir/borrow_check/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,13 @@ const DEREF_PROJECTION: &[PlaceElem<'_>; 1] = &[ProjectionElem::Deref];

pub fn provide(providers: &mut Providers) {
*providers = Providers {
mir_borrowck: |tcx, did| mir_borrowck(tcx, ty::WithOptConstParam::unknown(did)),
mir_borrowck: |tcx, did| {
if let Some(def) = ty::WithOptConstParam::try_lookup(did, tcx) {
tcx.mir_borrowck_const_arg(def)
} else {
mir_borrowck(tcx, ty::WithOptConstParam::unknown(did))
}
},
mir_borrowck_const_arg: |tcx, (did, param_did)| {
mir_borrowck(tcx, ty::WithOptConstParam { did, const_param_did: Some(param_did) })
},
Expand All @@ -100,12 +106,6 @@ fn mir_borrowck<'tcx>(
tcx: TyCtxt<'tcx>,
def: ty::WithOptConstParam<LocalDefId>,
) -> &'tcx BorrowCheckResult<'tcx> {
if def.const_param_did.is_none() {
if let Some(param_did) = tcx.opt_const_param_of(def.did) {
return tcx.mir_borrowck_const_arg((def.did, param_did));
}
}

let (input_body, promoted) = tcx.mir_validated(def);
debug!("run query mir_borrowck: {}", tcx.def_path_str(def.did.to_def_id()));

Expand Down
12 changes: 5 additions & 7 deletions src/librustc_mir/transform/check_unsafety.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,11 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers {
unsafety_check_result: |tcx, def_id| {
unsafety_check_result(tcx, ty::WithOptConstParam::unknown(def_id))
if let Some(def) = ty::WithOptConstParam::try_lookup(def_id, tcx) {
tcx.unsafety_check_result_for_const_arg(def)
} else {
unsafety_check_result(tcx, ty::WithOptConstParam::unknown(def_id))
}
},
unsafety_check_result_for_const_arg: |tcx, (did, param_did)| {
unsafety_check_result(
Expand Down Expand Up @@ -499,12 +503,6 @@ fn unsafety_check_result<'tcx>(
tcx: TyCtxt<'tcx>,
def: ty::WithOptConstParam<LocalDefId>,
) -> &'tcx UnsafetyCheckResult {
if def.const_param_did.is_none() {
if let Some(param_did) = tcx.opt_const_param_of(def.did) {
return tcx.unsafety_check_result_for_const_arg((def.did, param_did));
}
}

debug!("unsafety_violations({:?})", def);

// N.B., this borrow is valid because all the consumers of
Expand Down
53 changes: 21 additions & 32 deletions src/librustc_mir/transform/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,13 @@ pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers {
mir_keys,
mir_const,
mir_const_qualif: |tcx, did| {
mir_const_qualif(tcx, ty::WithOptConstParam::unknown(did.expect_local()))
mir_const_qualif: |tcx, def_id| {
let def_id = def_id.expect_local();
if let Some(def) = ty::WithOptConstParam::try_lookup(def_id, tcx) {
tcx.mir_const_qualif_const_arg(def)
} else {
mir_const_qualif(tcx, ty::WithOptConstParam::unknown(def_id))
}
},
mir_const_qualif_const_arg: |tcx, (did, param_did)| {
mir_const_qualif(tcx, ty::WithOptConstParam { did, const_param_did: Some(param_did) })
Expand All @@ -60,7 +65,12 @@ pub(crate) fn provide(providers: &mut Providers) {
optimized_mir_of_const_arg,
is_mir_available,
promoted_mir: |tcx, def_id| {
promoted_mir(tcx, ty::WithOptConstParam::unknown(def_id.expect_local()))
let def_id = def_id.expect_local();
if let Some(def) = ty::WithOptConstParam::try_lookup(def_id, tcx) {
tcx.promoted_mir_of_const_arg(def)
} else {
promoted_mir(tcx, ty::WithOptConstParam::unknown(def_id))
}
},
promoted_mir_of_const_arg: |tcx, (did, param_did)| {
promoted_mir(tcx, ty::WithOptConstParam { did, const_param_did: Some(param_did) })
Expand Down Expand Up @@ -221,12 +231,6 @@ pub fn run_passes(
}

fn mir_const_qualif(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> ConstQualifs {
if def.const_param_did.is_none() {
if let Some(param_did) = tcx.opt_const_param_of(def.did) {
return tcx.mir_const_qualif_const_arg((def.did, param_did));
}
}

let const_kind = tcx.hir().body_const_context(def.did);

// No need to const-check a non-const `fn`.
Expand Down Expand Up @@ -266,10 +270,8 @@ fn mir_const<'tcx>(
tcx: TyCtxt<'tcx>,
def: ty::WithOptConstParam<LocalDefId>,
) -> &'tcx Steal<Body<'tcx>> {
if def.const_param_did.is_none() {
if let const_param_did @ Some(_) = tcx.opt_const_param_of(def.did) {
return tcx.mir_const(ty::WithOptConstParam { const_param_did, ..def });
}
if let Some(def) = def.try_upgrade(tcx) {
return tcx.mir_const(def);
}

// Unsafety check uses the raw mir, so make sure it is run.
Expand Down Expand Up @@ -312,10 +314,8 @@ fn mir_validated(
tcx: TyCtxt<'tcx>,
def: ty::WithOptConstParam<LocalDefId>,
) -> (&'tcx Steal<Body<'tcx>>, &'tcx Steal<IndexVec<Promoted, Body<'tcx>>>) {
if def.const_param_did.is_none() {
if let const_param_did @ Some(_) = tcx.opt_const_param_of(def.did) {
return tcx.mir_validated(ty::WithOptConstParam { const_param_did, ..def });
}
if let Some(def) = def.try_upgrade(tcx) {
return tcx.mir_validated(def);
}

// Ensure that we compute the `mir_const_qualif` for constants at
Expand Down Expand Up @@ -357,13 +357,8 @@ fn mir_drops_elaborated_and_const_checked<'tcx>(
tcx: TyCtxt<'tcx>,
def: ty::WithOptConstParam<LocalDefId>,
) -> &'tcx Steal<Body<'tcx>> {
if def.const_param_did.is_none() {
if let const_param_did @ Some(_) = tcx.opt_const_param_of(def.did) {
return tcx.mir_drops_elaborated_and_const_checked(ty::WithOptConstParam {
const_param_did,
..def
});
}
if let Some(def) = def.try_upgrade(tcx) {
return tcx.mir_drops_elaborated_and_const_checked(def);
}

// (Mir-)Borrowck uses `mir_validated`, so we have to force it to
Expand Down Expand Up @@ -490,8 +485,8 @@ fn run_optimization_passes<'tcx>(

fn optimized_mir<'tcx>(tcx: TyCtxt<'tcx>, did: DefId) -> &'tcx Body<'tcx> {
let did = did.expect_local();
if let Some(param_did) = tcx.opt_const_param_of(did) {
tcx.optimized_mir_of_const_arg((did, param_did))
if let Some(def) = ty::WithOptConstParam::try_lookup(did, tcx) {
tcx.optimized_mir_of_const_arg(def)
} else {
tcx.arena.alloc(inner_optimized_mir(tcx, ty::WithOptConstParam::unknown(did)))
}
Expand Down Expand Up @@ -528,12 +523,6 @@ fn promoted_mir<'tcx>(
tcx: TyCtxt<'tcx>,
def: ty::WithOptConstParam<LocalDefId>,
) -> &'tcx IndexVec<Promoted, Body<'tcx>> {
if def.const_param_did.is_none() {
if let Some(param_did) = tcx.opt_const_param_of(def.did) {
return tcx.promoted_mir_of_const_arg((def.did, param_did));
}
}

if tcx.is_constructor(def.did.to_def_id()) {
return tcx.arena.alloc(IndexVec::new());
}
Expand Down
6 changes: 2 additions & 4 deletions src/librustc_mir_build/build/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@ crate fn mir_built<'tcx>(
tcx: TyCtxt<'tcx>,
def: ty::WithOptConstParam<LocalDefId>,
) -> &'tcx ty::steal::Steal<Body<'tcx>> {
if def.const_param_did.is_none() {
if let const_param_did @ Some(_) = tcx.opt_const_param_of(def.did) {
return tcx.mir_built(ty::WithOptConstParam { const_param_did, ..def });
}
if let Some(def) = def.try_upgrade(tcx) {
return tcx.mir_built(def);
}

tcx.alloc_steal_mir(mir_build(tcx, def))
Expand Down