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

Check the Self-type of inherent associated constants #58353

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@matthewjasper
Copy link
Contributor

matthewjasper commented Feb 10, 2019

@matthewjasper matthewjasper force-pushed the matthewjasper:typeck-pattern-constants branch 2 times, most recently from c468033 to a7ae7a2 Feb 10, 2019

@matthewjasper matthewjasper changed the title Handle user type annotations on range patterns Check the Self-type of inherent associated constants Feb 11, 2019

@matthewjasper

This comment has been minimized.

Copy link
Contributor Author

matthewjasper commented Feb 11, 2019

Removed the overlap with #58371

@@ -2236,7 +2236,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
hir_id, def_id, substs, user_self_ty, self.tag(),
);

if !substs.is_noop() {
if !substs.is_noop() || user_self_ty.map_or(false, |ty| {
ty.has_free_regions() || ty.has_projections() || ty.has_infer_types()

This comment has been minimized.

@arielb1

arielb1 Feb 12, 2019

Contributor

~~Is there some justification for why these checks are enough?~~Found it

If there is, could you write it in a comment?

This comment has been minimized.

@arielb1

arielb1 Feb 12, 2019

Contributor

Even if these checks are enough, shouldn't they be run on the pair (substs, ty)? i.e.

let types = (substs, ty); // yes there are TypeFoldable impls for pairs and options
if types.has_free_regions() || types.has_projections() || types.has_infer_types() {
    // ...
}

This comment has been minimized.

@arielb1

arielb1 Feb 12, 2019

Contributor

Nah I've seen a comment at

// If the type given by the user has free regions, save it for
// later, since NLL would like to enforce those. Also pass in
// types that involve projections, since those can resolve to
// `'static` bounds (modulo #54940, which hopefully will be
// fixed by the time you see this comment, dear reader,
// although I have my doubts). Also pass in types with inference
// types, because they may be repeated. Other sorts of things
// are already sufficiently enforced with erased regions. =)
if ty.has_free_regions() || ty.has_projections() || ty.has_infer_types() {

Maybe move the check there it to a separate function (needs_user_type_annotation), while still doing the types trick (where T: TypeFoldable<'tcx> on that function, and then call it)?

@arielb1

This comment has been minimized.

Copy link
Contributor

arielb1 commented Feb 12, 2019

r=me with the needs_user_type_annotation function

@matthewjasper matthewjasper force-pushed the matthewjasper:typeck-pattern-constants branch from a7ae7a2 to 283ffcf Feb 13, 2019

@arielb1

This comment has been minimized.

Copy link
Contributor

arielb1 commented Feb 14, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 14, 2019

📌 Commit 283ffcf has been approved by arielb1

Centril added a commit to Centril/rust that referenced this pull request Feb 14, 2019

Rollup merge of rust-lang#58353 - matthewjasper:typeck-pattern-consta…
…nts, r=arielb1

Check the Self-type of inherent associated constants

r? @arielb1

bors added a commit that referenced this pull request Feb 14, 2019

Auto merge of #58470 - Centril:rollup, r=Centril
Rollup of 9 pull requests

Successful merges:

 - #57981 (Fix #57730)
 - #58196 (Add specific feature gate error for const-unstable features)
 - #58293 (Remove code for updating copyright years in generate-deriving-span-tests)
 - #58306 (Don't default on std crate when manipulating browser history)
 - #58353 (Check the Self-type of inherent associated constants)
 - #58429 (fix Box::into_unique effecitvely transmuting to a raw ptr)
 - #58433 (Update which libcore/liballoc tests Miri ignores, and document why)
 - #58438 (Use posix_spawn_file_actions_addchdir_np when possible)
 - #58448 (rustdoc: mask `compiler_builtins` docs)

Failed merges:

r? @ghost

bors added a commit that referenced this pull request Feb 15, 2019

Auto merge of #58470 - Centril:rollup, r=Centril
Rollup of 9 pull requests

Successful merges:

 - #57981 (Fix #57730)
 - #58196 (Add specific feature gate error for const-unstable features)
 - #58293 (Remove code for updating copyright years in generate-deriving-span-tests)
 - #58306 (Don't default on std crate when manipulating browser history)
 - #58353 (Check the Self-type of inherent associated constants)
 - #58429 (fix Box::into_unique effecitvely transmuting to a raw ptr)
 - #58433 (Update which libcore/liballoc tests Miri ignores, and document why)
 - #58438 (Use posix_spawn_file_actions_addchdir_np when possible)
 - #58448 (rustdoc: mask `compiler_builtins` docs)

Failed merges:

r? @ghost

bors added a commit that referenced this pull request Feb 15, 2019

Auto merge of #58470 - Centril:rollup, r=Centril
Rollup of 9 pull requests

Successful merges:

 - #57981 (Fix #57730)
 - #58196 (Add specific feature gate error for const-unstable features)
 - #58293 (Remove code for updating copyright years in generate-deriving-span-tests)
 - #58306 (Don't default on std crate when manipulating browser history)
 - #58353 (Check the Self-type of inherent associated constants)
 - #58429 (fix Box::into_unique effecitvely transmuting to a raw ptr)
 - #58433 (Update which libcore/liballoc tests Miri ignores, and document why)
 - #58438 (Use posix_spawn_file_actions_addchdir_np when possible)
 - #58448 (rustdoc: mask `compiler_builtins` docs)

Failed merges:

r? @ghost

bors added a commit that referenced this pull request Feb 15, 2019

Auto merge of #58470 - Centril:rollup, r=Centril
Rollup of 9 pull requests

Successful merges:

 - #57981 (Fix #57730)
 - #58196 (Add specific feature gate error for const-unstable features)
 - #58293 (Remove code for updating copyright years in generate-deriving-span-tests)
 - #58306 (Don't default on std crate when manipulating browser history)
 - #58353 (Check the Self-type of inherent associated constants)
 - #58429 (fix Box::into_unique effecitvely transmuting to a raw ptr)
 - #58433 (Update which libcore/liballoc tests Miri ignores, and document why)
 - #58438 (Use posix_spawn_file_actions_addchdir_np when possible)
 - #58448 (rustdoc: mask `compiler_builtins` docs)

Failed merges:

r? @ghost
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 15, 2019

⌛️ Testing commit 283ffcf with merge df4ed96...

bors added a commit that referenced this pull request Feb 15, 2019

Auto merge of #58353 - matthewjasper:typeck-pattern-constants, r=arielb1
Check the Self-type of inherent associated constants

r? @arielb1
@pietroalbini

This comment has been minimized.

Copy link
Member

pietroalbini commented Feb 15, 2019

@bors treeclosed=1000

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 15, 2019

💔 Test failed - checks-travis

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Feb 15, 2019

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.

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)

@pietroalbini

This comment has been minimized.

Copy link
Member

pietroalbini commented Feb 15, 2019

@bors retry -- apparently we can't clone the repo anymore on macOS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment