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

RFC on long series to free libsyntax of vecs_implicitly_copyable #5143

Merged
merged 37 commits into from
Mar 2, 2013

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Feb 27, 2013

Good morning,

It's taken a long time, but I finally am almost done freeing libsyntax of vecs_implicitly_copyable in this pull request, but I'm running into some issues. I've confirmed that all but the last commit (which only disables vecs_implicitly_copyable pass the check tests. The last commit errors with this message, which makes no sense to me:

/Users/erickt/rust/rust/src/libcore/num/f32.rs:35:37: 35:43 error: expected `,` but found `=`
/Users/erickt/rust/rust/src/libcore/num/f32.rs:35         pub pure fn $name($( $arg : $arg_ty ),*) -> $rv {
                                                                                       ^~~~~~

and this stack trace:

#1  0x00000001000b059b in sys::begin_unwind_::_a923ca4ae164c::_06 ()
#2  0x00000001000b0542 in sys::begin_unwind::anon::anon::expr_fn_13876 ()
#3  0x00000001000048a1 in sys::begin_unwind::_8ec273289fc0adc0::_06 ()
#4  0x00000001005df999 in diagnostic::__extensions__::meth_7941::span_fatal::_efdf2d14612d79ec::_06 ()
#5  0x0000000100682d48 in parse::parser::__extensions__::meth_16938::fatal::_8aa3239426747a3::_06 ()
#6  0x00000001006850b8 in parse::common::__extensions__::meth_17005::expect::_d3604ec6c7698d5f::_06 ()
#7  0x00000001006b59f1 in parse::common::__extensions__::parse_seq_to_before_end_17860::_48c79835f9eb1011::_06 ()
#8  0x00000001006a50f7 in parse::parser::__extensions__::meth_17606::parse_fn_decl::_14f3785fe78967d::_06 ()
#9  0x00000001006b6f59 in parse::parser::__extensions__::meth_17987::parse_item_fn::_8a6be529cf7b2ca5::_06 ()
#10 0x00000001006ac839 in parse::parser::__extensions__::meth_17761::parse_item_or_view_item::_bfead947d6dd7d25::_06 ()
#11 0x00000001006c8b8f in parse::parser::__extensions__::meth_18364::parse_item::_96b54e33f65abe76::_06 ()
#12 0x000000010076179f in ext::tt::macro_rules::add_new_extension::generic_extension::anon::anon::expr_fn_23365 ()
#13 0x000000010072e793 in ext::expand::expand_item_mac::_a4f486c4465cfb1b::_06 ()
#14 0x00000001007b5ad3 in __morestack ()

There also a bunch of new warnings that I haven't cleaned up yet: https://gist.github.com/erickt/5048251.

@nikomatsakis thought there might be some scary bug in the parser caused by moving a vector in the parser instead of copying it, which is why I'm filing this pull request before it's ready. Thanks for any help!

@graydon
Copy link
Contributor

graydon commented Feb 27, 2013

I know this is unappealing to say, but have you tried bisecting the series to isolate that error?

@pcwalton
Copy link
Contributor

It may also help to try running the test suite (make check-stage1), which often produces smaller test cases.

@erickt
Copy link
Contributor Author

erickt commented Feb 27, 2013

@graydon: unfortunately there isn't much to bisect. After I merge in with HEAD, all but the commit that disables vecs_implicitly_copyable should be landable after passing all tests. I can't bisect because I need these 31 commits before I can turn off vecs_implicitly_copyable :(

@erickt
Copy link
Contributor Author

erickt commented Feb 27, 2013

Err, to be clear, all tests pass already pass for all but the last commit.

@brson
Copy link
Contributor

brson commented Feb 27, 2013

Since this is such a massive series, maybe we should go ahead and land all but the last commit.

@erickt
Copy link
Contributor Author

erickt commented Feb 27, 2013

@brson: Ok. I'll merge in HEAD and remove the last commit and file a bug.

@erickt
Copy link
Contributor Author

erickt commented Feb 27, 2013

This commit is mergable now, and all the tests passed on the try branch.

@erickt
Copy link
Contributor Author

erickt commented Mar 1, 2013

@bors: retry

@erickt
Copy link
Contributor Author

erickt commented Mar 2, 2013

@bors: retry p=50

@erickt
Copy link
Contributor Author

erickt commented Mar 2, 2013

r+

bors added a commit that referenced this pull request Mar 2, 2013
Good morning,

It's taken a long time, but I finally am almost done freeing libsyntax of `vecs_implicitly_copyable` in this pull request, but I'm running into some issues. I've confirmed that all but the last commit (which only disables `vecs_implicitly_copyable` pass the `check` tests. The last commit errors with this message, which makes no sense to me:

```
/Users/erickt/rust/rust/src/libcore/num/f32.rs:35:37: 35:43 error: expected `,` but found `=`
/Users/erickt/rust/rust/src/libcore/num/f32.rs:35         pub pure fn $name($( $arg : $arg_ty ),*) -> $rv {
                                                                                       ^~~~~~
```

and this stack trace:

```
#1  0x00000001000b059b in sys::begin_unwind_::_a923ca4ae164c::_06 ()
#2  0x00000001000b0542 in sys::begin_unwind::anon::anon::expr_fn_13876 ()
#3  0x00000001000048a1 in sys::begin_unwind::_8ec273289fc0adc0::_06 ()
#4  0x00000001005df999 in diagnostic::__extensions__::meth_7941::span_fatal::_efdf2d14612d79ec::_06 ()
#5  0x0000000100682d48 in parse::parser::__extensions__::meth_16938::fatal::_8aa3239426747a3::_06 ()
#6  0x00000001006850b8 in parse::common::__extensions__::meth_17005::expect::_d3604ec6c7698d5f::_06 ()
#7  0x00000001006b59f1 in parse::common::__extensions__::parse_seq_to_before_end_17860::_48c79835f9eb1011::_06 ()
#8  0x00000001006a50f7 in parse::parser::__extensions__::meth_17606::parse_fn_decl::_14f3785fe78967d::_06 ()
#9  0x00000001006b6f59 in parse::parser::__extensions__::meth_17987::parse_item_fn::_8a6be529cf7b2ca5::_06 ()
#10 0x00000001006ac839 in parse::parser::__extensions__::meth_17761::parse_item_or_view_item::_bfead947d6dd7d25::_06 ()
#11 0x00000001006c8b8f in parse::parser::__extensions__::meth_18364::parse_item::_96b54e33f65abe76::_06 ()
#12 0x000000010076179f in ext::tt::macro_rules::add_new_extension::generic_extension::anon::anon::expr_fn_23365 ()
#13 0x000000010072e793 in ext::expand::expand_item_mac::_a4f486c4465cfb1b::_06 ()
#14 0x00000001007b5ad3 in __morestack ()
```

There also a bunch of new warnings that I haven't cleaned up yet: https://gist.github.com/erickt/5048251.

@nikomatsakis thought there might be some scary bug in the parser caused by moving a vector in the parser instead of copying it, which is why I'm filing this pull request before it's ready. Thanks for any help!
@bors bors closed this Mar 2, 2013
@bors bors merged commit 5515fd5 into rust-lang:incoming Mar 2, 2013
bors added a commit that referenced this pull request Mar 3, 2013
My merges for #5143 missed a couple other copies. This patch corrects this, and gets stage0 to compile libsyntax with `#[deny(vecs_implicitly_copyable)]`. stage1 still fails though.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants