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

Add const generics to ty (and transitive dependencies) #58583

Merged
merged 34 commits into from Mar 7, 2019

Conversation

Projects
None yet
8 participants
@varkor
Copy link
Member

varkor commented Feb 20, 2019

Split out from #53645. This work is a collaborative effort with @yodaldevoid.

There are a number of stubs. Some I plan to leave for the next PRs (e.g. infer and rustdoc). Others I can either fix up in this PR, or as follow ups (which would avoid the time-consuming rebasing).

It was a little hard to split this up, as so much depends on ty and friends. Apologies for the large diff.

r? @eddyb

Show resolved Hide resolved src/librustc/ty/subst.rs
Show resolved Hide resolved src/librustc/ty/subst.rs
Show resolved Hide resolved src/test/ui/const-generics/const-expression-parameter.rs Outdated
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 21, 2019

☔️ The latest upstream changes (presumably #58056) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk
Copy link
Contributor

oli-obk left a comment

I think a bunch of the Evaluated handlings are "wrong" (in the sense that we could panic or delay_span_bug in the unevaluated case instead of just producing some value or skipping the value), but it's probably going to lead to weird diagnostics at worst and can be addressed together with the folding LazyConst back into ty::Const

can you rebase the PR? I'll try to re-review it fast before it bitrots again.

r? @oli-obk

@@ -585,6 +585,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
ty::LazyConst::Evaluated(c) => c,
};
match val.val {
ConstValue::Param(_) => Err(EvalErrorKind::TooGeneric.into()),

This comment has been minimized.

@oli-obk

oli-obk Mar 5, 2019

Contributor

Shouldn't we try to resolve this via self.monomorphize?

debug!(" => type length={}", type_length);
let const_length = instance.substs.consts()
.filter_map(|ct| {
if let ty::LazyConst::Evaluated(ct) = ct {

This comment has been minimized.

@oli-obk

oli-obk Mar 5, 2019

Contributor

It feels wrong to filter out parameters. Can you explain what's going on here?

This comment has been minimized.

@varkor

varkor Mar 5, 2019

Author Member

This was mainly intended to be a conservative extension of the type length limit check to also take account of the types in consts. This might not actually be necessary at all; it's hard to know without concrete examples of lengthy type-checking. I think LazyConst::Unevaluated was filtered out because getting a type from it wasn't immediately obvious. I've included it with type_of for now, but I'm not actually sure whether type_of on Unevaluated will ever actually return a concrete type?

varkor and others added some commits Feb 20, 2019

Add Const generic param to ty
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Add ParamConst
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Add ConstVid
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Add InferConst
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Add type_flags helper methods to consts
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Use non_erasable_generics for codegen
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Take const generics into account when monomorphising
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Add const kind and UnpackedKind::Const
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Add ConstValue::Param and ConstValue::Infer
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Add type constraints from const parameters
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Update diagnostics to include const parameters
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Implement Hash for new types
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Stub methods in infer
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Take const into account in context
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Add const type flags
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Pretty printing for const generics
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Make a lazy const from a const param
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Add ast_const_to_const
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Refactor compare_method
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Implement wfcheck for const parameters
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Implement collect for const parameters
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Stub rustdoc const generics implementations
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
Add HAS_CT_INFER
Co-Authored-By: Gabriel Smith <yodaldevoid@users.noreply.github.com>
@Centril

This comment has been minimized.

Copy link
Contributor

Centril commented Mar 6, 2019

@bors retry

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 6, 2019

⌛️ Testing commit de4478a with merge 8df8213...

bors added a commit that referenced this pull request Mar 6, 2019

Auto merge of #58583 - varkor:const-generics-ty, r=oli-obk
Add const generics to ty (and transitive dependencies)

Split out from #53645. This work is a collaborative effort with @yodaldevoid.

There are a number of stubs. Some I plan to leave for the next PRs (e.g. `infer` and `rustdoc`). Others I can either fix up in this PR, or as follow ups (which would avoid the time-consuming rebasing).

It was a little hard to split this up, as so much depends on ty and friends. Apologies for the large diff.

r? @eddyb
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 6, 2019

💔 Test failed - checks-travis

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Mar 6, 2019

The job i686-gnu of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[02:57:14] [RUSTC-TIMING] parking_lot_core test:false 3.258
[02:57:14]    Compiling parking_lot v0.7.1
[02:57:16] [RUSTC-TIMING] parking_lot test:false 1.772
[02:57:16]    Compiling rustdoc v0.0.0 (/checkout/src/librustdoc)
The job exceeded the maximum time limit for jobs, and has been terminated.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Centril

This comment has been minimized.

Copy link
Contributor

Centril commented Mar 6, 2019

@bors retry

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 6, 2019

⌛️ Testing commit de4478a with merge 2a9ce18...

bors added a commit that referenced this pull request Mar 6, 2019

Auto merge of #58583 - varkor:const-generics-ty, r=oli-obk
Add const generics to ty (and transitive dependencies)

Split out from #53645. This work is a collaborative effort with @yodaldevoid.

There are a number of stubs. Some I plan to leave for the next PRs (e.g. `infer` and `rustdoc`). Others I can either fix up in this PR, or as follow ups (which would avoid the time-consuming rebasing).

It was a little hard to split this up, as so much depends on ty and friends. Apologies for the large diff.

r? @eddyb
@pietroalbini

This comment has been minimized.

Copy link
Member

pietroalbini commented Mar 6, 2019

@bors retry

You're not gonna land anyway, so...

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 6, 2019

⌛️ Testing commit de4478a with merge 947aa2e...

bors added a commit that referenced this pull request Mar 6, 2019

Auto merge of #58583 - varkor:const-generics-ty, r=oli-obk
Add const generics to ty (and transitive dependencies)

Split out from #53645. This work is a collaborative effort with @yodaldevoid.

There are a number of stubs. Some I plan to leave for the next PRs (e.g. `infer` and `rustdoc`). Others I can either fix up in this PR, or as follow ups (which would avoid the time-consuming rebasing).

It was a little hard to split this up, as so much depends on ty and friends. Apologies for the large diff.

r? @eddyb
@pietroalbini

This comment has been minimized.

Copy link
Member

pietroalbini commented Mar 6, 2019

@bors retry

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 7, 2019

⌛️ Testing commit de4478a with merge 88f755f...

bors added a commit that referenced this pull request Mar 7, 2019

Auto merge of #58583 - varkor:const-generics-ty, r=oli-obk
Add const generics to ty (and transitive dependencies)

Split out from #53645. This work is a collaborative effort with @yodaldevoid.

There are a number of stubs. Some I plan to leave for the next PRs (e.g. `infer` and `rustdoc`). Others I can either fix up in this PR, or as follow ups (which would avoid the time-consuming rebasing).

It was a little hard to split this up, as so much depends on ty and friends. Apologies for the large diff.

r? @eddyb
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 7, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk
Pushing 88f755f to master...

@bors bors added the merged-by-bors label Mar 7, 2019

@bors bors merged commit de4478a into rust-lang:master Mar 7, 2019

1 check passed

homu Test successful
Details
@@ -1113,14 +1124,16 @@ fn create_mono_items_for_default_impls<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
continue;
}

if tcx.generics_of(method.def_id).own_counts().types != 0 {
let counts = tcx.generics_of(method.def_id).own_counts();
if counts.types + counts.consts != 0 {

This comment has been minimized.

@eddyb

eddyb Mar 13, 2019

Member

Shouldn't there be a method for this on ty::Generics?
If there is one but it recurses, you can make a version that doesn't and prefix its name with own_.

if type_length > type_length_limit {
// We include the const length in the type length, as it's better
// to be overly conservative.
if type_length + const_length > type_length_limit {

This comment has been minimized.

@eddyb

eddyb Mar 13, 2019

Member

I think this is unnecessary, .types() / .consts() shouldn't be used on the substs, instead the substs should be uniformly "walked" somehow, only ignoring lifetimes.

.flat_map(|ct| {
let ty = match ct {
ty::LazyConst::Evaluated(ct) => ct.ty,
ty::LazyConst::Unevaluated(def_id, _) => tcx.type_of(*def_id),

This comment has been minimized.

@eddyb

eddyb Mar 13, 2019

Member

I don't think the type matters as much as the unevaluated substs here.

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.