Fix associated types in copy implementations #38152

arielb1 · 2016-12-04T00:05:17Z

Fixes an ICE and an error in checking copy implementations.

mrhota · 2016-12-04T22:02:57Z

src/librustc_typeck/coherence/builtin.rs

+}
+
+fn visit_implementation_of_coerce_unsized<'a, 'tcx>(
+    tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId,


nit: maybe separate each arg on its own line?

I'd say just rustfmt this new file and be done with it.

mrhota · 2016-12-04T22:13:35Z

curiously, the test will ICE immediately with Stable rustc on play.rust-lang.org, but will timeout with beta and nightly.

nikomatsakis · 2016-12-09T15:43:25Z

src/librustc/ty/util.rs

+                    for variant in &adt.variants {
+                        for field in &variant.fields {
+                            if !field_implements_copy(field) {
+                                return Err(CopyImplementationError::InfrigingVariant(


Pre-existing, but it seems like it would be nice to give both the variant name and the field name -- can we do that instead?

That's a good idea.

nikomatsakis · 2016-12-09T15:44:12Z

src/librustc_typeck/coherence/builtin.rs

+}
+
+fn visit_implementation_of_coerce_unsized<'a, 'tcx>(
+    tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId,


I'd say just rustfmt this new file and be done with it.

nikomatsakis · 2016-12-09T15:44:44Z

src/librustc_typeck/coherence/builtin.rs

+    }
+    let method_def_id = items[0];
+
+    let self_type = tcx.item_type(impl_did);


Is this file all code collected from other places, or did you make changes to it? I couldn't tell.

Code moved from coherence::mod.

arielb1 · 2016-12-09T16:55:57Z

How do you rustfmt?

nikomatsakis · 2016-12-10T01:24:30Z

@arielb1 cargo install rustfmt, then run it on the file I think

bors · 2016-12-21T10:38:29Z

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

nikomatsakis · 2016-12-27T22:22:59Z

@arielb1 if you don't feel like running rustfmt, no biggy, but let's rebase :)

nikomatsakis · 2017-01-03T19:00:40Z

@bors r+

bors · 2017-01-03T19:00:43Z

📌 Commit 14efebc has been approved by nikomatsakis

arielb1 · 2017-01-03T19:50:10Z

I was just going to rustfmt the new file and add better error reporting.

@bors r-

no functional changes

Fixes rust-lang#33187.

Fixes rust-lang#34377.

nikomatsakis · 2017-01-03T21:59:19Z

src/test/compile-fail/E0204.rs

+    Bar(
+        &'a mut bool //~ this field does not implement `Copy`
+    ),
+    Baz,


can we make this a ui test? I'd like to see precisely where the span is, since it's critical to the message

Span the affected fields instead of reporting the field/variant name.

nikomatsakis · 2017-01-03T22:07:08Z

src/test/ui/span/E0204.stderr

+  --> $DIR/E0204.rs:27:6
+   |
+23 |     Bar { x: Vec<u32> },
+   |           ----------- this field does not implement `Copy`


looks great :)

nikomatsakis · 2017-01-03T22:07:18Z

@bors r+

bors · 2017-01-03T22:07:19Z

📌 Commit 4cab293 has been approved by nikomatsakis

bors · 2017-01-03T23:04:22Z

⌛ Testing commit 4cab293 with merge be89c6b...

bors · 2017-01-03T23:51:17Z

💔 Test failed - status-appveyor

this makes error messages consistent across architectures

arielb1 · 2017-01-04T09:56:03Z

@bors r=nikomatsakis

bors · 2017-01-04T09:56:03Z

📌 Commit 5fad51e has been approved by nikomatsakis

bors · 2017-01-04T11:25:17Z

⌛ Testing commit 5fad51e with merge 5e8f802...

bors · 2017-01-04T14:25:50Z

💔 Test failed - status-appveyor

arielb1 · 2017-01-04T15:47:42Z

Spurious timeout in test net::tcp::tests::close_read_wakes_up #31657

@bors retry

bors · 2017-01-04T17:05:16Z

⌛ Testing commit 5fad51e with merge 4f1a16e...

bors · 2017-01-04T17:10:34Z

💔 Test failed - status-travis

bluss · 2017-01-04T17:39:33Z

Network Error:

fatal: clone of 'https://github.com/rust-lang/llvm.git' into submodule path '/Users/travis/build/rust-lang/rust/src/llvm' failed

Investigate making the buildbots network-access-free #35002

@bors retry

bors · 2017-01-04T19:26:07Z

⌛ Testing commit 5fad51e with merge 60812a8...

bors · 2017-01-04T20:04:25Z

⌛ Testing commit 5fad51e with merge 7bd015d...

bors · 2017-01-04T22:08:12Z

⌛ Testing commit 5fad51e with merge 2b62184...

bors · 2017-01-04T23:04:46Z

💔 Test failed - status-travis

arielb1 · 2017-01-04T23:14:17Z

Spurious segfault in tests for musl #38618

@bors retry

bors · 2017-01-05T03:07:07Z

⌛ Testing commit 5fad51e with merge 90618ce...

bors · 2017-01-05T03:11:05Z

💔 Test failed - status-travis

arielb1 · 2017-01-05T12:18:53Z

The command "if [ "$ALLOW_PR" = "" ] && [ "$TRAVIS_BRANCH" != "auto" ]; then
    echo skipping, not a full build;
elif [ "$TRAVIS_OS_NAME" = "osx" ]; then
    git submodule update --init &&
    src/ci/run.sh;
else
    git submodule update --init &&
    src/ci/docker/run.sh $IMAGE;
fi
" exited with 129.

cc @alexcrichton

@bors retry

bors · 2017-01-05T13:05:10Z

⌛ Testing commit 5fad51e with merge 74e5b7d...

@nikomatsakis

Fix associated types in copy implementations Fixes an ICE and an error in checking copy implementations. r? @nikomatsakis

bors · 2017-01-05T15:05:18Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 74e5b7d to master...

rust-highfive assigned nikomatsakis Dec 4, 2016

mrhota reviewed Dec 4, 2016

View reviewed changes

nikomatsakis requested changes Dec 9, 2016

View reviewed changes

arielb1 force-pushed the special-copy branch from d662ceb to 14efebc Compare December 30, 2016 21:15

nikomatsakis approved these changes Jan 3, 2017

View reviewed changes

arielb1 added 5 commits January 3, 2017 21:50

coherence: move the builtin trait checks to their own module

f8a2f98

no functional changes

coherence: code cleanup

7309bab

coherence: check builtin impls after the specialization graph is ready

d938ba4

Fixes rust-lang#33187.

normalize field types in copy implementations

243e45a

Fixes rust-lang#34377.

rustfmt coherence::builtin

e41920a

arielb1 force-pushed the special-copy branch from 14efebc to e41920a Compare January 3, 2017 19:51

nikomatsakis reviewed Jan 3, 2017

View reviewed changes

simplify Copy implementation error reporting

4cab293

Span the affected fields instead of reporting the field/variant name.

arielb1 force-pushed the special-copy branch from 8ec9c83 to 4cab293 Compare January 3, 2017 22:04

nikomatsakis reviewed Jan 3, 2017

View reviewed changes

typeck::coherence::builtin - sort impls in the DefId order

5fad51e

this makes error messages consistent across architectures

bors added a commit that referenced this pull request Jan 5, 2017

Auto merge of #38152 - arielb1:special-copy, r=nikomatsakis

74e5b7d

Fix associated types in copy implementations Fixes an ICE and an error in checking copy implementations. r? @nikomatsakis

bors merged commit 5fad51e into rust-lang:master Jan 5, 2017

This was referenced Jan 5, 2017

Show span for trait that doesn't implement Copy #37493

Closed

[11/n] Separate ty::Tables into one per each body. #38813

Merged

arielb1 mentioned this pull request Jan 5, 2017

Provide better error reporting for unimplemented traits #19950

Closed

Fix associated types in copy implementations #38152

Fix associated types in copy implementations #38152

Conversation

arielb1 commented Dec 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrhota commented Dec 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielb1 commented Dec 9, 2016

nikomatsakis commented Dec 10, 2016

bors commented Dec 21, 2016

nikomatsakis commented Dec 27, 2016

nikomatsakis commented Jan 3, 2017

bors commented Jan 3, 2017

arielb1 commented Jan 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Jan 3, 2017

bors commented Jan 3, 2017

bors commented Jan 3, 2017

bors commented Jan 3, 2017

arielb1 commented Jan 4, 2017

bors commented Jan 4, 2017

bors commented Jan 4, 2017

bors commented Jan 4, 2017

arielb1 commented Jan 4, 2017

bors commented Jan 4, 2017

bors commented Jan 4, 2017

bluss commented Jan 4, 2017 • edited by arielb1 Loading

bors commented Jan 4, 2017

bors commented Jan 4, 2017

bors commented Jan 4, 2017

bors commented Jan 4, 2017

arielb1 commented Jan 4, 2017

bors commented Jan 5, 2017

bors commented Jan 5, 2017

arielb1 commented Jan 5, 2017

bors commented Jan 5, 2017

bors commented Jan 5, 2017

bluss commented Jan 4, 2017 •

edited by arielb1

Loading