make `Parser::parse_foreign_item()` return a foreign item or error #54833

abonander · 2018-10-05T01:32:23Z

Fixes Parser::parse_foreign_item() to follow the convention of parse_trait_item() and parse_impl_item() in that it must parse an item or return an error, and then the caller is responsible for detecting the closing delimiter.

This prevents it from looping endlessly on an unexpected token in ext/expand.rs where it was also leaking memory by continually pushing to Parser::expected_tokens via Parser::check_keyword().

closes #54441

r? @petrochenkov
cc @dtolnay

abonander · 2018-10-05T01:38:52Z

@petrochenkov As a bugcheck I think it might be a good idea to add a check for duplicates in Parser::check_keyword, if not all the time then at least with debug assertions on.

rust-highfive · 2018-10-05T01:57:00Z

The job x86_64-gnu-llvm-5.0 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.

[00:05:49]    Compiling rustc_data_structures v0.0.0 (/checkout/src/librustc_data_structures)
[00:05:54]    Compiling arena v0.0.0 (/checkout/src/libarena)
[00:05:54]    Compiling syntax_pos v0.0.0 (/checkout/src/libsyntax_pos)
[00:06:01]    Compiling rustc_errors v0.0.0 (/checkout/src/librustc_errors)
[00:06:12] error[E0599]: no method named `conains` found for type `std::vec::Vec<parse::parser::TokenType>` in the current scope
[00:06:12]     |
[00:06:12]     |
[00:06:12] 867 |         debug_assert!(!self.expected_tokens.conains(&token),
[00:06:12]     |
[00:06:12]     = help: did you mean `contains`?
[00:06:12] 
[00:06:12] 
[00:06:12] error[E0277]: `syntax_pos::symbol::keywords::Keyword` doesn't implement `std::fmt::Debug`
[00:06:12]     |
[00:06:12]     |
[00:06:12] 869 |                       kw, self.expected_tokens);
[00:06:12]     |                       ^^ `syntax_pos::symbol::keywords::Keyword` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
[00:06:12]     = help: the trait `std::fmt::Debug` is not implemented for `syntax_pos::symbol::keywords::Keyword`
[00:06:12]     = note
travis_time:end:06d37088:start=1538704242217519360,finish=1538704616990765253,duration=374773245893

---
travis_time:end:01a2e69c:start=1538704617407530482,finish=1538704617412070228,duration=4539746
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:19dddbec
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0028f31a
travis_time:start:0028f31a
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:29c04c3a
$ dmesg | grep -i kill

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)

rust-highfive · 2018-10-05T02:33:08Z

The job x86_64-gnu-llvm-5.0 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.

[00:06:12]    Compiling rustc_data_structures v0.0.0 (/checkout/src/librustc_data_structures)
[00:06:17]    Compiling arena v0.0.0 (/checkout/src/libarena)
[00:06:21]    Compiling syntax_pos v0.0.0 (/checkout/src/libsyntax_pos)
[00:06:25]    Compiling rustc_errors v0.0.0 (/checkout/src/librustc_errors)
[00:06:37] error[E0277]: `parse::parser::TokenType` doesn't implement `std::fmt::Debug`
[00:06:37]     |
[00:06:37]     |
[00:06:37] 869 |                               kw.name(), self.expected_tokens));
[00:06:37]     |                                          ^^^^^^^^^^^^^^^^^^^^ `parse::parser::TokenType` cannot be formatted using `{:?}`
[00:06:37]     |
[00:06:37]     = help: the trait `std::fmt::Debug` is not implemented for `parse::parser::TokenType`
[00:06:37]     = note: add `#[derive(Debug)]` or manually implement `std::fmt::Debug`
[00:06:37]     = note: required because of the requirements on the impl of `std::fmt::Debug` for `std::vec::Vec<parse::parser::TokenType>`
[00:06:37]     = note: required by `std::fmt::Debug::fmt`
[00:06:40] error: aborting due to previous error
[00:06:40] 
[00:06:40] For more information about this error, try `rustc --explain E0277`.
[00:06:40] error: Could not compile `syntax`.
[00:06:40] error: Could not compile `syntax`.
[00:06:40] 
[00:06:40] To learn more, run the command again with --verbose.
[00:06:40] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" " jemalloc" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json"
[00:06:40] expected success, got: exit code: 101
[00:06:40] thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1112:9
[00:06:40] travis_fold:end:stage0-rustc

[00:06:40] travis_time:end:stage0-rustc:start=1538706624336085303,finish=1538706695486409537,duration=71150324234


[00:06:40] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:06:40] Build completed unsuccessfully in 0:02:12
[00:06:40] make: *** [all] Error 1
[00:06:40] Makefile:28: recipe for target 'all' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:021a1ace
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:24678e04:start=1538706696117094865,finish=1538706696120961547,duration=3866682
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:04d7de77
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:04986350
travis_time:start:04986350
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0b799690
$ dmesg | grep -i kill

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)

rust-highfive · 2018-10-05T02:56:02Z

The job x86_64-gnu-llvm-5.0 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.

[00:19:54]    Compiling cc v1.0.25
[00:19:54]    Compiling core v0.0.0 (/checkout/src/libcore)
[00:19:54]    Compiling unwind v0.0.0 (/checkout/src/libunwind)
[00:19:54]    Compiling build_helper v0.1.0 (/checkout/src/build_helper)
[00:19:54] error: internal compiler error: pushing duplicate keyword to expected_tokens
[00:19:54]     |
[00:19:54]     |
[00:19:54] 137 | mod macros;
[00:19:54] 
[00:19:54] error: aborting due to previous error
[00:19:54] 
[00:19:54] error: Could not compile `core`.
---
travis_time:end:0a8ecd48:start=1538708157799517547,finish=1538708157803980057,duration=4462510
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:017c3980
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:2773c5f6
travis_time:start:2773c5f6
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers:

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)

abonander · 2018-10-05T03:26:21Z

Eh, nevermind.

rust-highfive · 2018-10-05T04:31:05Z

The job x86_64-gnu-llvm-5.0 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.

[00:50:13] ...............................................................................................i.... 2200/4550
[00:50:18] .................................................................................................... 2300/4550
[00:50:21] .................................................................................................... 2400/4550
[00:50:25] .................................................................................................... 2500/4550
[00:50:29] .......iiiiiiiii.................................................................................... 2600/4550
[00:50:35] .................................................................................................... 2800/4550
[00:50:39] .................................................................................................... 2900/4550
[00:50:42] ..........................i......................................................................... 3000/4550
[00:50:45] ......................................................................................i.i..ii....... 3100/4550
---
travis_time:start:test_parse-fail
Check compiletest suite=parse-fail mode=parse-fail (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:02:40] 
[01:02:40] running 273 tests
/bin/rustc" "--src-base" "/checkout/src/test/parse-fail" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/parse-fail" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "parse-fail" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-5.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "5.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:02:43] 
[01:02:43] 
[01:02:43] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:02:43] Build completed unsuccessfully in 0:17:13
[01:02:43] Build completed unsuccessfully in 0:17:13
[01:02:43] Makefile:58: recipe for target 'check' failed
[01:02:43] make: *** [check] Error 1
2839804 ./obj
2626236 ./obj/build
1993724 ./obj/build/x86_64-unknown-linux-gnu
1069132 ./src

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)

abonander · 2018-10-05T04:46:55Z

@petrochenkov Is this error pattern change okay (expecting closing brace later on, not in this error msg)?

[01:02:43] error: error pattern 'expected one of `(`, `fn`, `static`, `type`, or `}` here' not found!
[01:02:43] error: expected one of `(`, `fn`, `static`, or `type`, found `pub`
[01:02:43]   --> /checkout/src/test/parse-fail/duplicate-visibility.rs:15:9
[01:02:43]    |
[01:02:43] LL |     pub pub fn foo();
[01:02:43]    |         ^^^ expected one of `(`, `fn`, `static`, or `type` here
[01:02:43]

It's actually more correct, isn't it, since a pub } isn't syntactically valid?

petrochenkov · 2018-10-05T09:12:33Z

src/test/compile-fail/issue-54441.rs

+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.


Nits:

new tests are no longer added to the compile-fail directory (they go into ui now)

the license section is no longer necessary

petrochenkov · 2018-10-05T09:16:19Z

The error change looks ok, r=me with nits addressed.

closes rust-lang#54441

abonander · 2018-10-05T09:49:40Z

src/test/ui/macros/issue-54441.stderr

+  --> $DIR/issue-54441.rs:5:9
+   |
+LL | #![feature(macros_in_extern)]
+   | - expected one of `crate`, `fn`, `pub`, `static`, or `type` here


Yes, this is the "expected" line pointing to line 1 of the source. However, it looks to be an existing bug in Parser::expect_one_of() WRT macros: https://play.rust-lang.org/?gist=1865a38a912b5aaed3e9a23931ace87c&version=stable&mode=debug&edition=2015

I can fix it here or in a different PR.

I can fix it here or in a different PR.

Whatever is more convenient for you.

abonander · 2018-10-05T09:53:12Z

@petrochenkov Nits fixed, though look at that comment I left on issue-54441.stderr

petrochenkov · 2018-10-05T10:11:58Z

@bors r+

bors · 2018-10-05T10:11:59Z

📌 Commit 9da428d has been approved by petrochenkov

abonander · 2018-10-05T10:13:07Z

I'll open an issue so this can be merged.

@petrochenkov

make `Parser::parse_foreign_item()` return a foreign item or error Fixes `Parser::parse_foreign_item()` to follow the convention of `parse_trait_item()` and `parse_impl_item()` in that it *must* parse an item or return an error, and then the caller is responsible for detecting the closing delimiter. This prevents it from looping endlessly on an unexpected token in `ext/expand.rs` where it was also leaking memory by continually pushing to `Parser::expected_tokens` via `Parser::check_keyword()`. closes rust-lang#54441 r? @petrochenkov cc @dtolnay

@ghost

Rollup of 11 pull requests Successful merges: - #54078 (Expand the documentation for the `std::sync` module) - #54717 (Cleanup rustc/ty part 1) - #54781 (Add examples to `TyKind::FnDef` and `TyKind::FnPtr` docs) - #54787 (Only warn about unused `mut` in user-written code) - #54804 (add suggestion for inverted function parameters) - #54812 (Regression test for #32382.) - #54833 (make `Parser::parse_foreign_item()` return a foreign item or error) - #54834 (rustdoc: overflow:auto doesn't work nicely on small screens) - #54838 (Fix typo in src/libsyntax/parse/parser.rs) - #54851 (Fix a regression in 1.30 by reverting #53564) - #54853 (Remove unneccessary error from test, revealing NLL error.) Failed merges: r? @ghost

rust-highfive assigned petrochenkov Oct 5, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 5, 2018

abonander force-pushed the issue-54441 branch 2 times, most recently from 1be986b to b3426aa Compare October 5, 2018 01:48

abonander force-pushed the issue-54441 branch from b3426aa to e6e9237 Compare October 5, 2018 02:22

abonander force-pushed the issue-54441 branch from e6e9237 to fbeb1ca Compare October 5, 2018 02:34

abonander force-pushed the issue-54441 branch from fbeb1ca to ddcbe08 Compare October 5, 2018 03:26

abonander force-pushed the issue-54441 branch 2 times, most recently from ca6f20c to 48ed783 Compare October 5, 2018 05:51

petrochenkov reviewed Oct 5, 2018

View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 5, 2018

make Parser::parse_foreign_item() return a foreign item or error

9da428d

closes rust-lang#54441

abonander force-pushed the issue-54441 branch from 48ed783 to 9da428d Compare October 5, 2018 09:48

abonander commented Oct 5, 2018

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 5, 2018

abonander mentioned this pull request Oct 5, 2018

Incorrect diagnostic span on syntax error in macro invoc inside trait, impl, extern blocks #54841

Closed

pietroalbini mentioned this pull request Oct 5, 2018

Rollup of 11 pull requests #54859

Merged

bors merged commit 9da428d into rust-lang:master Oct 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make `Parser::parse_foreign_item()` return a foreign item or error #54833

make `Parser::parse_foreign_item()` return a foreign item or error #54833

abonander commented Oct 5, 2018 •

edited

Loading

abonander commented Oct 5, 2018

rust-highfive commented Oct 5, 2018

rust-highfive commented Oct 5, 2018

rust-highfive commented Oct 5, 2018

abonander commented Oct 5, 2018

rust-highfive commented Oct 5, 2018

abonander commented Oct 5, 2018 •

edited

Loading

petrochenkov Oct 5, 2018

petrochenkov commented Oct 5, 2018

abonander Oct 5, 2018

petrochenkov Oct 5, 2018

abonander commented Oct 5, 2018

petrochenkov commented Oct 5, 2018

bors commented Oct 5, 2018

abonander commented Oct 5, 2018

make Parser::parse_foreign_item() return a foreign item or error #54833

make Parser::parse_foreign_item() return a foreign item or error #54833

Conversation

abonander commented Oct 5, 2018 • edited Loading

abonander commented Oct 5, 2018

rust-highfive commented Oct 5, 2018

rust-highfive commented Oct 5, 2018

rust-highfive commented Oct 5, 2018

abonander commented Oct 5, 2018

rust-highfive commented Oct 5, 2018

abonander commented Oct 5, 2018 • edited Loading

petrochenkov Oct 5, 2018

Choose a reason for hiding this comment

petrochenkov commented Oct 5, 2018

abonander Oct 5, 2018

Choose a reason for hiding this comment

petrochenkov Oct 5, 2018

Choose a reason for hiding this comment

abonander commented Oct 5, 2018

petrochenkov commented Oct 5, 2018

bors commented Oct 5, 2018

abonander commented Oct 5, 2018

make `Parser::parse_foreign_item()` return a foreign item or error #54833

make `Parser::parse_foreign_item()` return a foreign item or error #54833

abonander commented Oct 5, 2018 •

edited

Loading

abonander commented Oct 5, 2018 •

edited

Loading