From 1acd3789bb70c39f23c8fb13f0db50402ebb146a Mon Sep 17 00:00:00 2001 From: kennytm Date: Wed, 21 Feb 2018 22:25:12 +0800 Subject: [PATCH 01/21] Provides direct link to the PR when toolstate is changed. Fix rust-lang-nursery/rust-toolstate#1. --- src/tools/publish_toolstate.py | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index b90947e5a434a..e2dbdb301e286 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -17,6 +17,7 @@ import copy import datetime import collections +import textwrap # List of people to ping when the status of a tool changed. MAINTAINERS = { @@ -38,7 +39,12 @@ def read_current_status(current_commit, path): return {} -def update_latest(current_commit, relevant_pr_number, current_datetime): +def update_latest( + current_commit, + relevant_pr_number, + relevant_pr_url, + current_datetime +): '''Updates `_data/latest.json` to match build result of the given commit. ''' with open('_data/latest.json', 'rb+') as f: @@ -50,8 +56,13 @@ def update_latest(current_commit, relevant_pr_number, current_datetime): } slug = 'rust-lang/rust' - message = '📣 Toolstate changed by {}!\n\nTested on commit {}@{}.\n\n' \ - .format(relevant_pr_number, slug, current_commit) + message = textwrap.dedent('''\ + 📣 Toolstate changed by {}! + + Tested on commit {}@{}. + Direct link to PR: <{}> + + ''').format(relevant_pr_number, slug, current_commit, relevant_pr_url) anything_changed = False for status in latest: tool = status['tool'] @@ -90,13 +101,21 @@ def update_latest(current_commit, relevant_pr_number, current_datetime): cur_commit_msg = sys.argv[2] save_message_to_path = sys.argv[3] - relevant_pr_match = re.search('#[0-9]+', cur_commit_msg) + relevant_pr_match = re.search('#([0-9]+)', cur_commit_msg) if relevant_pr_match: - relevant_pr_number = 'rust-lang/rust' + relevant_pr_match.group(0) + number = relevant_pr_match.group(1) + relevant_pr_number = 'rust-lang/rust#' + number + relevant_pr_url = 'https://github.com/rust-lang/rust/pull/' + number else: relevant_pr_number = '' - - message = update_latest(cur_commit, relevant_pr_number, cur_datetime) + relevant_pr_url = '' + + message = update_latest( + cur_commit, + relevant_pr_number, + relevant_pr_url, + cur_datetime + ) if message: print(message) with open(save_message_to_path, 'w') as f: From e18105079e4d0b925897fbb786cebb9539662e13 Mon Sep 17 00:00:00 2001 From: kennytm Date: Wed, 21 Feb 2018 22:58:06 +0800 Subject: [PATCH 02/21] Submit a comment to the PR in additional to pushing a commit. Fix rust-lang-nursery/rust-toolstate#2. --- .travis.yml | 2 +- src/tools/publish_toolstate.py | 30 +++++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 280da05699506..0d8641e45ed15 100644 --- a/.travis.yml +++ b/.travis.yml @@ -188,7 +188,7 @@ matrix: script: MESSAGE_FILE=$(mktemp -t msg.XXXXXX); . src/ci/docker/x86_64-gnu-tools/repo.sh; - commit_toolstate_change "$MESSAGE_FILE" "$TRAVIS_BUILD_DIR/src/tools/publish_toolstate.py" "$(git rev-parse HEAD)" "$(git log --format=%s -n1 HEAD)" "$MESSAGE_FILE" + commit_toolstate_change "$MESSAGE_FILE" "$TRAVIS_BUILD_DIR/src/tools/publish_toolstate.py" "$(git rev-parse HEAD)" "$(git log --format=%s -n1 HEAD)" "$MESSAGE_FILE" "$TOOLSTATE_REPO_ACCESS_TOKEN"; env: global: diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index e2dbdb301e286..40abe81c449ef 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -18,6 +18,10 @@ import datetime import collections import textwrap +try: + import urllib2 +except ImportError: + import urllib.request as urllib2 # List of people to ping when the status of a tool changed. MAINTAINERS = { @@ -100,6 +104,7 @@ def update_latest( cur_datetime = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%SZ') cur_commit_msg = sys.argv[2] save_message_to_path = sys.argv[3] + github_token = sys.argv[4] relevant_pr_match = re.search('#([0-9]+)', cur_commit_msg) if relevant_pr_match: @@ -107,6 +112,7 @@ def update_latest( relevant_pr_number = 'rust-lang/rust#' + number relevant_pr_url = 'https://github.com/rust-lang/rust/pull/' + number else: + number = '-1' relevant_pr_number = '' relevant_pr_url = '' @@ -116,9 +122,23 @@ def update_latest( relevant_pr_url, cur_datetime ) - if message: - print(message) - with open(save_message_to_path, 'w') as f: - f.write(message) - else: + if not message: print('') + sys.exit(0) + + print(message) + with open(save_message_to_path, 'w') as f: + f.write(message) + + # Write the toolstate comment on the PR as well. + gh_url = 'https://api.github.com/repos/rust-lang/rust/issues/{}/comments' \ + .format(number) + response = urllib2.urlopen(urllib2.Request( + gh_url, + json.dumps({'body': message}), + { + 'Authorization': 'token ' + github_token, + 'Content-Type': 'application/json', + } + )) + response.read() From f6e4751ebeaac327db7fe444ed7991f70e8fd929 Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 22 Feb 2018 00:37:14 +0800 Subject: [PATCH 03/21] Disallow toolstate regression at the last week of the 6-week cycle. --- src/ci/docker/x86_64-gnu-tools/Dockerfile | 1 + .../x86_64-gnu-tools/checkregression.py | 40 +++++++++++++++++++ src/ci/docker/x86_64-gnu-tools/checktools.sh | 8 ++++ 3 files changed, 49 insertions(+) create mode 100755 src/ci/docker/x86_64-gnu-tools/checkregression.py diff --git a/src/ci/docker/x86_64-gnu-tools/Dockerfile b/src/ci/docker/x86_64-gnu-tools/Dockerfile index 8975d419d2055..bab9145cbcb9c 100644 --- a/src/ci/docker/x86_64-gnu-tools/Dockerfile +++ b/src/ci/docker/x86_64-gnu-tools/Dockerfile @@ -17,6 +17,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ COPY scripts/sccache.sh /scripts/ RUN sh /scripts/sccache.sh +COPY x86_64-gnu-tools/checkregression.py /tmp/ COPY x86_64-gnu-tools/checktools.sh /tmp/ COPY x86_64-gnu-tools/repo.sh /tmp/ diff --git a/src/ci/docker/x86_64-gnu-tools/checkregression.py b/src/ci/docker/x86_64-gnu-tools/checkregression.py new file mode 100755 index 0000000000000..df791d12645fd --- /dev/null +++ b/src/ci/docker/x86_64-gnu-tools/checkregression.py @@ -0,0 +1,40 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +# Copyright 2018 The Rust Project Developers. See the COPYRIGHT +# file at the top-level directory of this distribution and at +# http://rust-lang.org/COPYRIGHT. +# +# Licensed under the Apache License, Version 2.0 or the MIT license +# , at your +# option. This file may not be copied, modified, or distributed +# except according to those terms. + +import sys +import json + +if __name__ == '__main__': + os_name = sys.argv[1] + toolstate_file = sys.argv[2] + current_state = sys.argv[3] + + with open(toolstate_file, 'r') as f: + toolstate = json.load(f) + with open(current_state, 'r') as f: + current = json.load(f) + + regressed = False + for cur in current: + tool = cur['tool'] + state = cur[os_name] + new_state = toolstate.get(tool, '') + if new_state < state: + print( + 'Error: The state of "{}" has regressed from "{}" to "{}"' + .format(tool, state, new_state) + ) + regressed = True + + if regressed: + sys.exit(1) diff --git a/src/ci/docker/x86_64-gnu-tools/checktools.sh b/src/ci/docker/x86_64-gnu-tools/checktools.sh index 61bb5a84d21ac..a7d0c6a1e6a7f 100755 --- a/src/ci/docker/x86_64-gnu-tools/checktools.sh +++ b/src/ci/docker/x86_64-gnu-tools/checktools.sh @@ -17,6 +17,9 @@ TOOLSTATE_FILE="$(realpath $2)" OS="$3" COMMIT="$(git rev-parse HEAD)" CHANGED_FILES="$(git diff --name-status HEAD HEAD^)" +SIX_WEEK_CYCLE="$(( ($(date +%s) / 604800 - 3) % 6 ))" +# ^ 1970 Jan 1st is a Thursday, and our release dates are also on Thursdays, +# thus we could divide by 604800 (7 days in seconds) directly. touch "$TOOLSTATE_FILE" @@ -59,6 +62,11 @@ if [ "$RUST_RELEASE_CHANNEL" = nightly -a -n "${TOOLSTATE_REPO_ACCESS_TOKEN+is_s sed -i "1 a\\ $COMMIT\t$(cat "$TOOLSTATE_FILE") " "history/$OS.tsv" + # if we are at the last week in the 6-week release cycle, reject any kind of regression. + if [ $SIX_WEEK_CYCLE -eq 5 ]; then + python2.7 "$(dirname $0)/checkregression.py" \ + "$OS" "$TOOLSTATE_FILE" "rust-toolstate/_data/latest.json" + fi rm -f "$MESSAGE_FILE" exit 0 fi From 51238c77e676f399a5b47d922e0da6af4837aff5 Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 22 Feb 2018 02:05:32 +0800 Subject: [PATCH 04/21] CI: Fixed the incorrect folder region when building codegen dylib. --- src/bootstrap/compile.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 2dcc0e0e7cd9f..84dd8f099f3df 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -630,6 +630,8 @@ impl Step for CodegenBackend { .arg(build.src.join("src/librustc_trans/Cargo.toml")); rustc_cargo_env(build, &mut cargo); + let _folder = build.fold_output(|| format!("stage{}-rustc_trans", compiler.stage)); + match &*self.backend { "llvm" | "emscripten" => { // Build LLVM for our target. This will implicitly build the @@ -643,7 +645,6 @@ impl Step for CodegenBackend { features.push_str(" emscripten"); } - let _folder = build.fold_output(|| format!("stage{}-rustc_trans", compiler.stage)); println!("Building stage{} codegen artifacts ({} -> {}, {})", compiler.stage, &compiler.host, target, self.backend); From 0d300d4b9d83b5243217aea98551c1f1676567d4 Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 22 Feb 2018 03:13:34 +0800 Subject: [PATCH 05/21] Split test::Docs into one Step for each book. The *.md at the root directory in src/doc are no longer tested, but this should be fine since all files there are deprecated. --- src/bootstrap/builder.rs | 6 ++-- src/bootstrap/test.rs | 65 ++++++++++++++++++++++++++++++++-------- 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 66a1c97246200..49c7fc4770563 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -231,7 +231,7 @@ pub struct ShouldRun<'a> { paths: BTreeSet, // If this is a default rule, this is an additional constraint placed on - // it's run. Generally something like compiler docs being enabled. + // its run. Generally something like compiler docs being enabled. is_really_default: bool, } @@ -326,7 +326,9 @@ impl<'a> Builder<'a> { test::RunPassPretty, test::RunFailPretty, test::RunPassValgrindPretty, test::RunPassFullDepsPretty, test::RunFailFullDepsPretty, test::RunMake, test::Crate, test::CrateLibrustc, test::Rustdoc, test::Linkcheck, test::Cargotest, - test::Cargo, test::Rls, test::Docs, test::ErrorIndex, test::Distcheck, + test::Cargo, test::Rls, test::ErrorIndex, test::Distcheck, + test::Nomicon, test::Reference, test::RustdocBook, test::RustByExample, + test::TheBook, test::UnstableBook, test::Rustfmt, test::Miri, test::Clippy, test::RustdocJS, test::RustdocTheme), Kind::Bench => describe!(test::Crate, test::CrateLibrustc), Kind::Doc => describe!(doc::UnstableBook, doc::UnstableBookGen, doc::TheBook, diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index bd8c36a296c09..03b7c18c55162 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -994,23 +994,19 @@ impl Step for Compiletest { } #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] -pub struct Docs { +struct DocTest { compiler: Compiler, + path: &'static str, + name: &'static str, + is_ext_doc: bool, } -impl Step for Docs { +impl Step for DocTest { type Output = (); - const DEFAULT: bool = true; const ONLY_HOSTS: bool = true; fn should_run(run: ShouldRun) -> ShouldRun { - run.path("src/doc") - } - - fn make_run(run: RunConfig) { - run.builder.ensure(Docs { - compiler: run.builder.compiler(run.builder.top_stage, run.host), - }); + run.never() } /// Run `rustdoc --test` for all documentation in `src/doc`. @@ -1026,9 +1022,9 @@ impl Step for Docs { // Do a breadth-first traversal of the `src/doc` directory and just run // tests for all files that end in `*.md` - let mut stack = vec![build.src.join("src/doc")]; + let mut stack = vec![build.src.join(self.path)]; let _time = util::timeit(); - let _folder = build.fold_output(|| "test_docs"); + let _folder = build.fold_output(|| format!("test_{}", self.name)); while let Some(p) = stack.pop() { if p.is_dir() { @@ -1051,6 +1047,51 @@ impl Step for Docs { } } +macro_rules! test_book { + ($($name:ident, $path:expr, $book_name:expr, default=$default:expr;)+) => { + $( + #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] + pub struct $name { + compiler: Compiler, + } + + impl Step for $name { + type Output = (); + const DEFAULT: bool = $default; + const ONLY_HOSTS: bool = true; + + fn should_run(run: ShouldRun) -> ShouldRun { + run.path($path) + } + + fn make_run(run: RunConfig) { + run.builder.ensure($name { + compiler: run.builder.compiler(run.builder.top_stage, run.host), + }); + } + + fn run(self, builder: &Builder) { + builder.ensure(DocTest { + compiler: self.compiler, + path: $path, + name: $book_name, + is_ext_doc: !$default, + }); + } + } + )+ + } +} + +test_book!( + Nomicon, "src/doc/nomicon", "nomicon", default=false; + Reference, "src/doc/reference", "reference", default=false; + RustdocBook, "src/doc/rustdoc", "rustdoc", default=true; + RustByExample, "src/doc/rust-by-example", "rust-by-example", default=false; + TheBook, "src/doc/book", "book", default=false; + UnstableBook, "src/doc/unstable-book", "unstable-book", default=true; +); + #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct ErrorIndex { compiler: Compiler, From a9f940e320bccc86dc7ab8b6179e918d3c05454d Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 22 Feb 2018 03:25:23 +0800 Subject: [PATCH 06/21] Run the external doc tests in tools job. --- src/bootstrap/test.rs | 22 ++++++++--- src/ci/docker/x86_64-gnu-tools/checktools.sh | 39 ++++++++++++++------ src/tools/publish_toolstate.py | 6 ++- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 03b7c18c55162..b27ddfdbc5e58 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -78,15 +78,17 @@ fn try_run(build: &Build, cmd: &mut Command) -> bool { true } -fn try_run_quiet(build: &Build, cmd: &mut Command) { +fn try_run_quiet(build: &Build, cmd: &mut Command) -> bool { if !build.fail_fast { if !build.try_run_quiet(cmd) { let mut failures = build.delayed_failures.borrow_mut(); failures.push(format!("{:?}", cmd)); + return false; } } else { build.run_quiet(cmd); } + true } #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] @@ -1042,7 +1044,15 @@ impl Step for DocTest { continue; } - markdown_test(builder, compiler, &p); + let test_result = markdown_test(builder, compiler, &p); + if self.is_ext_doc { + let toolstate = if test_result { + ToolState::TestPass + } else { + ToolState::TestFail + }; + build.save_toolstate(self.name, toolstate); + } } } } @@ -1142,13 +1152,13 @@ impl Step for ErrorIndex { } } -fn markdown_test(builder: &Builder, compiler: Compiler, markdown: &Path) { +fn markdown_test(builder: &Builder, compiler: Compiler, markdown: &Path) -> bool { let build = builder.build; let mut file = t!(File::open(markdown)); let mut contents = String::new(); t!(file.read_to_string(&mut contents)); if !contents.contains("```") { - return; + return true; } println!("doc tests for: {}", markdown.display()); @@ -1162,9 +1172,9 @@ fn markdown_test(builder: &Builder, compiler: Compiler, markdown: &Path) { cmd.arg("--test-args").arg(test_args); if build.config.quiet_tests { - try_run_quiet(build, &mut cmd); + try_run_quiet(build, &mut cmd) } else { - try_run(build, &mut cmd); + try_run(build, &mut cmd) } } diff --git a/src/ci/docker/x86_64-gnu-tools/checktools.sh b/src/ci/docker/x86_64-gnu-tools/checktools.sh index a7d0c6a1e6a7f..da89aa9423b2d 100755 --- a/src/ci/docker/x86_64-gnu-tools/checktools.sh +++ b/src/ci/docker/x86_64-gnu-tools/checktools.sh @@ -25,6 +25,10 @@ touch "$TOOLSTATE_FILE" set +e python2.7 "$X_PY" test --no-fail-fast \ + src/doc/book \ + src/doc/nomicon \ + src/doc/reference \ + src/doc/rust-by-example \ src/tools/rls \ src/tools/rustfmt \ src/tools/miri \ @@ -32,27 +36,38 @@ python2.7 "$X_PY" test --no-fail-fast \ set -e cat "$TOOLSTATE_FILE" +echo -# If this PR is intended to update one of these tools, do not let the build pass -# when they do not test-pass. -for TOOL in rls rustfmt clippy; do - echo "Verifying status of $TOOL..." - if echo "$CHANGED_FILES" | grep -q "^M[[:blank:]]src/tools/$TOOL$"; then - echo "This PR updated 'src/tools/$TOOL', verifying if status is 'test-pass'..." - if grep -vq '"'"$TOOL"'[^"]*":"test-pass"' "$TOOLSTATE_FILE"; then +verify_status() { + echo "Verifying status of $1..." + if echo "$CHANGED_FILES" | grep -q "^M[[:blank:]]$2$"; then + echo "This PR updated '$2', verifying if status is 'test-pass'..." + if grep -vq '"'"$1"'":"test-pass"' "$TOOLSTATE_FILE"; then echo - echo "⚠️ We detected that this PR updated '$TOOL', but its tests failed." + echo "⚠️ We detected that this PR updated '$1', but its tests failed." echo - echo "If you do intend to update '$TOOL', please check the error messages above and" + echo "If you do intend to update '$1', please check the error messages above and" echo "commit another update." echo - echo "If you do NOT intend to update '$TOOL', please ensure you did not accidentally" - echo "change the submodule at 'src/tools/$TOOL'. You may ask your reviewer for the" + echo "If you do NOT intend to update '$1', please ensure you did not accidentally" + echo "change the submodule at '$2'. You may ask your reviewer for the" echo "proper steps." exit 3 fi fi -done +} + +# If this PR is intended to update one of these tools, do not let the build pass +# when they do not test-pass. + +verify_status book src/doc/book +verify_status nomicon src/doc/nomicon +verify_status reference src/doc/reference +verify_status rust-by-example src/doc/rust-by-example +verify_status rls src/tool/rls +verify_status rustfmt src/tool/rustfmt +verify_status clippy-driver src/tool/clippy +#verify_status miri src/tool/miri if [ "$RUST_RELEASE_CHANNEL" = nightly -a -n "${TOOLSTATE_REPO_ACCESS_TOKEN+is_set}" ]; then . "$(dirname $0)/repo.sh" diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 40abe81c449ef..8e23519f57ebc 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -29,6 +29,10 @@ 'clippy-driver': '@Manishearth @llogiq @mcarton @oli-obk', 'rls': '@nrc', 'rustfmt': '@nrc', + 'book': '@carols10cents @steveklabnik', + 'nomicon': '@frewsxcv @Gankro', + 'reference': '@steveklabnik @Havvy @matthewjasper @alercah', + 'rust-by-example': '@steveklabnik @marioidival @projektir', } @@ -83,7 +87,7 @@ def update_latest( elif new < old: changed = True message += '💔 {} on {}: {} → {} (cc {}).\n' \ - .format(tool, os, old, new, MAINTAINERS[tool]) + .format(tool, os, old, new, MAINTAINERS.get(tool)) if changed: status['commit'] = current_commit From d27fac618d0e9e054fbc65130788eaacda259a9f Mon Sep 17 00:00:00 2001 From: Ryan Cumming Date: Sun, 25 Feb 2018 11:41:08 +1100 Subject: [PATCH 07/21] Fix find_width_of_character_at_span bounds check Commit 0bd96671f0 added bounds checking of our current target byte position to prevent infinite loops. Unfortunately it was comparing the file-relative `target` versus the global relative `file_start_pos` and `file_end_pos`. The result is failing to detect multibyte characters unless their file-relative offset fit within their global offset. This causes other parts of the compiler to generate spans pointing to the middle of a multibyte character which will ultimately panic in `bytepos_to_file_charpos`. Fix by comparing the `target` to the total file size when moving forward and doing checked subtraction when moving backwards. This should preserve the intent of the bounds check while removing the offset confusion. Fixes #48508 --- src/libsyntax/codemap.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index df5845f6c217d..5d36f39b2e43c 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -705,17 +705,20 @@ impl CodeMap { }; debug!("find_width_of_character_at_span: snippet=`{:?}`", snippet); - let file_start_pos = local_begin.fm.start_pos.to_usize(); - let file_end_pos = local_begin.fm.end_pos.to_usize(); - debug!("find_width_of_character_at_span: file_start_pos=`{:?}` file_end_pos=`{:?}`", - file_start_pos, file_end_pos); - let mut target = if forwards { end_index + 1 } else { end_index - 1 }; debug!("find_width_of_character_at_span: initial target=`{:?}`", target); - while !snippet.is_char_boundary(target - start_index) - && target >= file_start_pos && target <= file_end_pos { - target = if forwards { target + 1 } else { target - 1 }; + while !snippet.is_char_boundary(target - start_index) && target < source_len { + target = if forwards { + target + 1 + } else { + match target.checked_sub(1) { + Some(target) => target, + None => { + break; + } + } + }; debug!("find_width_of_character_at_span: target=`{:?}`", target); } debug!("find_width_of_character_at_span: final target=`{:?}`", target); From c237d4f859c47ddc3f03b485915d2cadbd741a1c Mon Sep 17 00:00:00 2001 From: Ryan Cumming Date: Mon, 26 Feb 2018 19:25:10 +1100 Subject: [PATCH 08/21] Add test for #48508 This is named for the issue as it's testing the specific details of that bug. It's a bit tricky as the ICE requires multiple files and debug info enabled to trigger. --- src/test/run-pass/issue-48508-aux.rs | 16 ++++++++++++++++ src/test/run-pass/issue-48508.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 src/test/run-pass/issue-48508-aux.rs create mode 100644 src/test/run-pass/issue-48508.rs diff --git a/src/test/run-pass/issue-48508-aux.rs b/src/test/run-pass/issue-48508-aux.rs new file mode 100644 index 0000000000000..a00361a2c9d33 --- /dev/null +++ b/src/test/run-pass/issue-48508-aux.rs @@ -0,0 +1,16 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// ignore-test Not a test. Used by issue-48508.rs + +pub fn other() -> f64 { + let µ = 1.0; + µ +} diff --git a/src/test/run-pass/issue-48508.rs b/src/test/run-pass/issue-48508.rs new file mode 100644 index 0000000000000..303ea2aa6012f --- /dev/null +++ b/src/test/run-pass/issue-48508.rs @@ -0,0 +1,27 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Regression test for issue #48508: +// +// Confusion between global and local file offsets caused incorrect handling of multibyte character +// spans when compiling multiple files. One visible effect was an ICE generating debug information +// when a multibyte character is at the end of a scope. The problematic code is actually in +// issue-48508-aux.rs + +// compile-flags:-g + +#![feature(non_ascii_idents)] + +#[path = "issue-48508-aux.rs"] +mod other_file; + +fn main() { + other_file::other(); +} From c99f4c4c5b9f968b82037cf643b6662b140d9b1f Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Tue, 27 Feb 2018 17:00:01 +0100 Subject: [PATCH 09/21] Stabilize LocalKey::try_with --- src/libstd/io/stdio.rs | 5 ++++- src/libstd/thread/local.rs | 28 ++++++++++++++------------- src/libstd/thread/mod.rs | 5 ++++- src/test/run-pass/tls-init-on-init.rs | 1 + 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index 831688bb73d1c..f01ca18851d77 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -17,7 +17,9 @@ use io::{self, Initializer, BufReader, LineWriter}; use sync::{Arc, Mutex, MutexGuard}; use sys::stdio; use sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard}; -use thread::{LocalKey, LocalKeyState}; +use thread::LocalKey; +#[allow(deprecated)] +use thread::LocalKeyState; /// Stdout used by print! and println! macros thread_local! { @@ -668,6 +670,7 @@ pub fn set_print(sink: Option>) -> Option> { /// thread, it will just fall back to the global stream. /// /// However, if the actual I/O causes an error, this function does panic. +#[allow(deprecated)] fn print_to(args: fmt::Arguments, local_s: &'static LocalKey>>>, global_s: fn() -> T, diff --git a/src/libstd/thread/local.rs b/src/libstd/thread/local.rs index fcbca38a98f0b..b6dbcf8914cbf 100644 --- a/src/libstd/thread/local.rs +++ b/src/libstd/thread/local.rs @@ -199,6 +199,7 @@ macro_rules! __thread_local_inner { #[unstable(feature = "thread_local_state", reason = "state querying was recently added", issue = "27716")] +#[rustc_deprecated(since = "1.26.0", reason = "use `LocalKey::try_with` instead")] #[derive(Debug, Eq, PartialEq, Copy, Clone)] pub enum LocalKeyState { /// All keys are in this state whenever a thread starts. Keys will @@ -234,25 +235,19 @@ pub enum LocalKeyState { } /// An error returned by [`LocalKey::try_with`](struct.LocalKey.html#method.try_with). -#[unstable(feature = "thread_local_state", - reason = "state querying was recently added", - issue = "27716")] +#[stable(feature = "thread_local_try_with", since = "1.26.0")] pub struct AccessError { _private: (), } -#[unstable(feature = "thread_local_state", - reason = "state querying was recently added", - issue = "27716")] +#[stable(feature = "thread_local_try_with", since = "1.26.0")] impl fmt::Debug for AccessError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_struct("AccessError").finish() } } -#[unstable(feature = "thread_local_state", - reason = "state querying was recently added", - issue = "27716")] +#[stable(feature = "thread_local_try_with", since = "1.26.0")] impl fmt::Display for AccessError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt("already destroyed", f) @@ -341,6 +336,8 @@ impl LocalKey { #[unstable(feature = "thread_local_state", reason = "state querying was recently added", issue = "27716")] + #[rustc_deprecated(since = "1.26.0", reason = "use `LocalKey::try_with` instead")] + #[allow(deprecated)] pub fn state(&'static self) -> LocalKeyState { unsafe { match (self.inner)() { @@ -365,11 +362,11 @@ impl LocalKey { /// /// This function will still `panic!()` if the key is uninitialized and the /// key's initializer panics. - #[unstable(feature = "thread_local_state", - reason = "state querying was recently added", - issue = "27716")] + #[stable(feature = "thread_local_try_with", since = "1.26.0")] pub fn try_with(&'static self, f: F) -> Result - where F: FnOnce(&T) -> R { + where + F: FnOnce(&T) -> R, + { unsafe { let slot = (self.inner)().ok_or(AccessError { _private: (), @@ -530,6 +527,7 @@ pub mod os { mod tests { use sync::mpsc::{channel, Sender}; use cell::{Cell, UnsafeCell}; + #[allow(deprecated)] use super::LocalKeyState; use thread; @@ -565,6 +563,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn states() { struct Foo; impl Drop for Foo { @@ -602,6 +601,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn circular() { struct S1; struct S2; @@ -642,6 +642,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn self_referential() { struct S1; thread_local!(static K1: UnsafeCell> = UnsafeCell::new(None)); @@ -663,6 +664,7 @@ mod tests { // test on macOS. #[test] #[cfg_attr(target_os = "macos", ignore)] + #[allow(deprecated)] fn dtors_in_dtors_in_dtors() { struct S1(Sender<()>); thread_local!(static K1: UnsafeCell> = UnsafeCell::new(None)); diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index ff121e2d7ee4e..01898679bdcfc 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -191,7 +191,10 @@ use time::Duration; #[macro_use] mod local; #[stable(feature = "rust1", since = "1.0.0")] -pub use self::local::{LocalKey, LocalKeyState, AccessError}; +pub use self::local::{LocalKey, AccessError}; +#[stable(feature = "rust1", since = "1.0.0")] +#[allow(deprecated)] +pub use self::local::LocalKeyState; // The types used by the thread_local! macro to access TLS keys. Note that there // are two types, the "OS" type and the "fast" type. The OS thread local key diff --git a/src/test/run-pass/tls-init-on-init.rs b/src/test/run-pass/tls-init-on-init.rs index b44c535d3a487..b5b9fb561ae7e 100644 --- a/src/test/run-pass/tls-init-on-init.rs +++ b/src/test/run-pass/tls-init-on-init.rs @@ -11,6 +11,7 @@ // ignore-emscripten no threads support #![feature(thread_local_state)] +#![allow(deprecated)] use std::thread::{self, LocalKeyState}; use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT}; From 989134e71bd01eaa9baf1e8e4654da3258fef014 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Wed, 28 Feb 2018 21:56:37 +0900 Subject: [PATCH 10/21] Add regression test for #48551 --- src/test/run-pass/issue-48551.rs | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 src/test/run-pass/issue-48551.rs diff --git a/src/test/run-pass/issue-48551.rs b/src/test/run-pass/issue-48551.rs new file mode 100644 index 0000000000000..61146cd087b15 --- /dev/null +++ b/src/test/run-pass/issue-48551.rs @@ -0,0 +1,40 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::ops::{Mul, MulAssign}; + +pub trait ClosedMul: Sized + Mul + MulAssign {} +impl ClosedMul for T +where + T: Mul + MulAssign, +{ +} + +pub trait InnerSpace: ClosedMul<::Real> { + type Real; +} + +pub trait FiniteDimVectorSpace: ClosedMul<::Field> { + type Field; +} + +pub trait FiniteDimInnerSpace + : InnerSpace + FiniteDimVectorSpace::Real> { +} + +pub trait EuclideanSpace: ClosedMul<::Real> { + type Coordinates: FiniteDimInnerSpace + + Mul + + MulAssign; + + type Real; +} + +fn main() {} From 90b28135ccee40577bd0915d70fcd5613aa52544 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 26 Feb 2018 23:46:33 +0000 Subject: [PATCH 11/21] Remove the v7 feature from AArch64 It isn't a valid LLVM feature for this architecture. --- src/librustc_trans/llvm_util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_trans/llvm_util.rs b/src/librustc_trans/llvm_util.rs index 00ac9d802457c..d80ef49d49ae7 100644 --- a/src/librustc_trans/llvm_util.rs +++ b/src/librustc_trans/llvm_util.rs @@ -81,7 +81,7 @@ unsafe fn configure_llvm(sess: &Session) { const ARM_WHITELIST: &'static [&'static str] = &["neon", "v7", "vfp2", "vfp3", "vfp4"]; -const AARCH64_WHITELIST: &'static [&'static str] = &["neon", "v7"]; +const AARCH64_WHITELIST: &'static [&'static str] = &["neon"]; const X86_WHITELIST: &'static [&'static str] = &["aes", "avx", "avx2", "avx512bw", "avx512cd", "avx512dq", "avx512er", From f756ad3d9478a5b82b0dfb304bf31b46a031ed08 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 27 Feb 2018 02:05:58 +0000 Subject: [PATCH 12/21] Add AArch64 features --- src/librustc_trans/attributes.rs | 2 +- src/librustc_trans/llvm_util.rs | 24 ++++++++++++++++-------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/librustc_trans/attributes.rs b/src/librustc_trans/attributes.rs index 8309c91ab2573..57cd47063dcef 100644 --- a/src/librustc_trans/attributes.rs +++ b/src/librustc_trans/attributes.rs @@ -212,7 +212,7 @@ fn from_target_feature( let value = value.as_str(); for feature in value.split(',') { if whitelist.contains(feature) { - let llvm_feature = llvm_util::to_llvm_feature(feature); + let llvm_feature = llvm_util::to_llvm_feature(&tcx.sess, feature); target_features.push(format!("+{}", llvm_feature)); continue } diff --git a/src/librustc_trans/llvm_util.rs b/src/librustc_trans/llvm_util.rs index d80ef49d49ae7..45445a48e233e 100644 --- a/src/librustc_trans/llvm_util.rs +++ b/src/librustc_trans/llvm_util.rs @@ -81,7 +81,9 @@ unsafe fn configure_llvm(sess: &Session) { const ARM_WHITELIST: &'static [&'static str] = &["neon", "v7", "vfp2", "vfp3", "vfp4"]; -const AARCH64_WHITELIST: &'static [&'static str] = &["neon"]; +const AARCH64_WHITELIST: &'static [&'static str] = &["fp", "neon", "sve", "crc", "crypto", + "ras", "lse", "rdm", "fp16", "rcpc", + "dotprod", "v8.1a", "v8.2a", "v8.3a"]; const X86_WHITELIST: &'static [&'static str] = &["aes", "avx", "avx2", "avx512bw", "avx512cd", "avx512dq", "avx512er", @@ -104,12 +106,18 @@ const POWERPC_WHITELIST: &'static [&'static str] = &["altivec", const MIPS_WHITELIST: &'static [&'static str] = &["msa"]; -pub fn to_llvm_feature(s: &str) -> &str { - match s { - "pclmulqdq" => "pclmul", - "rdrand" => "rdrnd", - "bmi1" => "bmi", - s => s, +pub fn to_llvm_feature<'a>(sess: &Session, s: &'a str) -> &'a str { + let arch = if sess.target.target.arch == "x86_64" { + "x86" + } else { + &*sess.target.target.arch + }; + match (arch, s) { + ("x86", "pclmulqdq") => "pclmul", + ("x86", "rdrand") => "rdrnd", + ("x86", "bmi1") => "bmi", + ("aarch64", "fp16") => "fullfp16", + (_, s) => s, } } @@ -118,7 +126,7 @@ pub fn target_features(sess: &Session) -> Vec { target_feature_whitelist(sess) .iter() .filter(|feature| { - let llvm_feature = to_llvm_feature(feature); + let llvm_feature = to_llvm_feature(sess, feature); let cstr = CString::new(llvm_feature).unwrap(); unsafe { llvm::LLVMRustHasFeature(target_machine, cstr.as_ptr()) } }) From 27fae2b24af48041ceea6e04c7f217d3db372164 Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Wed, 28 Feb 2018 18:59:12 +0100 Subject: [PATCH 13/21] Remove thread_local_state --- src/libstd/io/stdio.rs | 39 +++++++------ src/libstd/thread/local.rs | 111 +++---------------------------------- src/libstd/thread/mod.rs | 3 - 3 files changed, 26 insertions(+), 127 deletions(-) diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index f01ca18851d77..b8fb83ad46597 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -18,8 +18,6 @@ use sync::{Arc, Mutex, MutexGuard}; use sys::stdio; use sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard}; use thread::LocalKey; -#[allow(deprecated)] -use thread::LocalKeyState; /// Stdout used by print! and println! macros thread_local! { @@ -670,25 +668,26 @@ pub fn set_print(sink: Option>) -> Option> { /// thread, it will just fall back to the global stream. /// /// However, if the actual I/O causes an error, this function does panic. -#[allow(deprecated)] -fn print_to(args: fmt::Arguments, - local_s: &'static LocalKey>>>, - global_s: fn() -> T, - label: &str) where T: Write { - let result = match local_s.state() { - LocalKeyState::Uninitialized | - LocalKeyState::Destroyed => global_s().write_fmt(args), - LocalKeyState::Valid => { - local_s.with(|s| { - if let Ok(mut borrowed) = s.try_borrow_mut() { - if let Some(w) = borrowed.as_mut() { - return w.write_fmt(args); - } - } - global_s().write_fmt(args) - }) +fn print_to( + args: fmt::Arguments, + local_s: &'static LocalKey>>>, + global_s: fn() -> T, + label: &str, +) +where + T: Write, +{ + let result = local_s.try_with(|s| { + if let Ok(mut borrowed) = s.try_borrow_mut() { + if let Some(w) = borrowed.as_mut() { + return w.write_fmt(args); + } } - }; + global_s().write_fmt(args) + }).unwrap_or_else(|_| { + global_s().write_fmt(args) + }); + if let Err(e) = result { panic!("failed printing to {}: {}", label, e); } diff --git a/src/libstd/thread/local.rs b/src/libstd/thread/local.rs index b6dbcf8914cbf..25fedcb277265 100644 --- a/src/libstd/thread/local.rs +++ b/src/libstd/thread/local.rs @@ -195,45 +195,6 @@ macro_rules! __thread_local_inner { } } -/// Indicator of the state of a thread local storage key. -#[unstable(feature = "thread_local_state", - reason = "state querying was recently added", - issue = "27716")] -#[rustc_deprecated(since = "1.26.0", reason = "use `LocalKey::try_with` instead")] -#[derive(Debug, Eq, PartialEq, Copy, Clone)] -pub enum LocalKeyState { - /// All keys are in this state whenever a thread starts. Keys will - /// transition to the `Valid` state once the first call to [`with`] happens - /// and the initialization expression succeeds. - /// - /// Keys in the `Uninitialized` state will yield a reference to the closure - /// passed to [`with`] so long as the initialization routine does not panic. - /// - /// [`with`]: ../../std/thread/struct.LocalKey.html#method.with - Uninitialized, - - /// Once a key has been accessed successfully, it will enter the `Valid` - /// state. Keys in the `Valid` state will remain so until the thread exits, - /// at which point the destructor will be run and the key will enter the - /// `Destroyed` state. - /// - /// Keys in the `Valid` state will be guaranteed to yield a reference to the - /// closure passed to [`with`]. - /// - /// [`with`]: ../../std/thread/struct.LocalKey.html#method.with - Valid, - - /// When a thread exits, the destructors for keys will be run (if - /// necessary). While a destructor is running, and possibly after a - /// destructor has run, a key is in the `Destroyed` state. - /// - /// Keys in the `Destroyed` states will trigger a panic when accessed via - /// [`with`]. - /// - /// [`with`]: ../../std/thread/struct.LocalKey.html#method.with - Destroyed, -} - /// An error returned by [`LocalKey::try_with`](struct.LocalKey.html#method.try_with). #[stable(feature = "thread_local_try_with", since = "1.26.0")] pub struct AccessError { @@ -307,51 +268,6 @@ impl LocalKey { (*ptr).as_ref().unwrap() } - /// Query the current state of this key. - /// - /// A key is initially in the `Uninitialized` state whenever a thread - /// starts. It will remain in this state up until the first call to [`with`] - /// within a thread has run the initialization expression successfully. - /// - /// Once the initialization expression succeeds, the key transitions to the - /// `Valid` state which will guarantee that future calls to [`with`] will - /// succeed within the thread. Some keys might skip the `Uninitialized` - /// state altogether and start in the `Valid` state as an optimization - /// (e.g. keys initialized with a constant expression), but no guarantees - /// are made. - /// - /// When a thread exits, each key will be destroyed in turn, and as keys are - /// destroyed they will enter the `Destroyed` state just before the - /// destructor starts to run. Keys may remain in the `Destroyed` state after - /// destruction has completed. Keys without destructors (e.g. with types - /// that are [`Copy`]), may never enter the `Destroyed` state. - /// - /// Keys in the `Uninitialized` state can be accessed so long as the - /// initialization does not panic. Keys in the `Valid` state are guaranteed - /// to be able to be accessed. Keys in the `Destroyed` state will panic on - /// any call to [`with`]. - /// - /// [`with`]: ../../std/thread/struct.LocalKey.html#method.with - /// [`Copy`]: ../../std/marker/trait.Copy.html - #[unstable(feature = "thread_local_state", - reason = "state querying was recently added", - issue = "27716")] - #[rustc_deprecated(since = "1.26.0", reason = "use `LocalKey::try_with` instead")] - #[allow(deprecated)] - pub fn state(&'static self) -> LocalKeyState { - unsafe { - match (self.inner)() { - Some(cell) => { - match *cell.get() { - Some(..) => LocalKeyState::Valid, - None => LocalKeyState::Uninitialized, - } - } - None => LocalKeyState::Destroyed, - } - } - } - /// Acquires a reference to the value in this TLS key. /// /// This will lazily initialize the value if this thread has not referenced @@ -527,8 +443,6 @@ pub mod os { mod tests { use sync::mpsc::{channel, Sender}; use cell::{Cell, UnsafeCell}; - #[allow(deprecated)] - use super::LocalKeyState; use thread; struct Foo(Sender<()>); @@ -563,26 +477,21 @@ mod tests { } #[test] - #[allow(deprecated)] fn states() { struct Foo; impl Drop for Foo { fn drop(&mut self) { - assert!(FOO.state() == LocalKeyState::Destroyed); + assert!(FOO.try_with(|_| ()).is_err()); } } fn foo() -> Foo { - assert!(FOO.state() == LocalKeyState::Uninitialized); + assert!(FOO.try_with(|_| ()).is_err()); Foo } thread_local!(static FOO: Foo = foo()); thread::spawn(|| { - assert!(FOO.state() == LocalKeyState::Uninitialized); - FOO.with(|_| { - assert!(FOO.state() == LocalKeyState::Valid); - }); - assert!(FOO.state() == LocalKeyState::Valid); + assert!(FOO.try_with(|_| ()).is_ok()); }).join().ok().unwrap(); } @@ -601,7 +510,6 @@ mod tests { } #[test] - #[allow(deprecated)] fn circular() { struct S1; struct S2; @@ -612,8 +520,7 @@ mod tests { impl Drop for S1 { fn drop(&mut self) { unsafe { - HITS += 1; - if K2.state() == LocalKeyState::Destroyed { + if K2.try_with(|_| ()).is_err() { assert_eq!(HITS, 3); } else { if HITS == 1 { @@ -629,7 +536,7 @@ mod tests { fn drop(&mut self) { unsafe { HITS += 1; - assert!(K1.state() != LocalKeyState::Destroyed); + assert!(K1.try_with(|_| ()).is_ok()); assert_eq!(HITS, 2); K1.with(|s| *s.get() = Some(S1)); } @@ -642,14 +549,13 @@ mod tests { } #[test] - #[allow(deprecated)] fn self_referential() { struct S1; thread_local!(static K1: UnsafeCell> = UnsafeCell::new(None)); impl Drop for S1 { fn drop(&mut self) { - assert!(K1.state() == LocalKeyState::Destroyed); + assert!(K1.try_with(|_| ()).is_err()); } } @@ -664,7 +570,6 @@ mod tests { // test on macOS. #[test] #[cfg_attr(target_os = "macos", ignore)] - #[allow(deprecated)] fn dtors_in_dtors_in_dtors() { struct S1(Sender<()>); thread_local!(static K1: UnsafeCell> = UnsafeCell::new(None)); @@ -674,9 +579,7 @@ mod tests { fn drop(&mut self) { let S1(ref tx) = *self; unsafe { - if K2.state() != LocalKeyState::Destroyed { - K2.with(|s| *s.get() = Some(Foo(tx.clone()))); - } + let _ = K2.try_with(|s| *s.get() = Some(Foo(tx.clone()))); } } } diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 01898679bdcfc..71aee673cfe3e 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -192,9 +192,6 @@ use time::Duration; #[stable(feature = "rust1", since = "1.0.0")] pub use self::local::{LocalKey, AccessError}; -#[stable(feature = "rust1", since = "1.0.0")] -#[allow(deprecated)] -pub use self::local::LocalKeyState; // The types used by the thread_local! macro to access TLS keys. Note that there // are two types, the "OS" type and the "fast" type. The OS thread local key From 11eb83ae749f037997ad6bf0841daa69d625f066 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 28 Feb 2018 13:04:12 -0500 Subject: [PATCH 14/21] Update issue-48551.rs --- src/test/run-pass/issue-48551.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/run-pass/issue-48551.rs b/src/test/run-pass/issue-48551.rs index 61146cd087b15..93bddbc533549 100644 --- a/src/test/run-pass/issue-48551.rs +++ b/src/test/run-pass/issue-48551.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// Regression test for #48551. Covers a case where duplicate candidates +// arose during associated type projection. + use std::ops::{Mul, MulAssign}; pub trait ClosedMul: Sized + Mul + MulAssign {} From 2ec79f936a564c2538c28bf7bdf2ef26a6ab6006 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Thu, 22 Feb 2018 17:50:06 -0600 Subject: [PATCH 15/21] Remove E0245; improve E0404 explanation --- src/librustc_resolve/diagnostics.rs | 24 ++++++++++++++++++++++-- src/librustc_resolve/lib.rs | 16 +++++++++------- src/librustc_typeck/astconv.rs | 9 ++++----- src/librustc_typeck/diagnostics.rs | 2 +- src/test/ui/error-codes/E0404.rs | 5 +++-- src/test/ui/error-codes/E0404.stderr | 6 ++++++ 6 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index c1e6b4f5a17d9..a0fc5533f8e54 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -542,7 +542,7 @@ fn foo(s: T, u: T) {} // error: the name `T` is already used for a type // parameter in this type parameter list ``` -Please verify that none of the type parameterss are misspelled, and rename any +Please verify that none of the type parameters are misspelled, and rename any clashing parameters. Example: ``` @@ -551,7 +551,8 @@ fn foo(s: T, u: Y) {} // ok! "##, E0404: r##" -You tried to implement something which was not a trait on an object. +You tried to use something which is not a trait in a trait position, such as +a bound or `impl`. Erroneous code example: @@ -562,6 +563,14 @@ struct Bar; impl Foo for Bar {} // error: `Foo` is not a trait ``` +Another erroneous code example: + +```compile_fail,E0404 +struct Foo; + +fn bar(t: T) {} // error: `Foo` is not a trait +``` + Please verify that you didn't misspell the trait's name or otherwise use the wrong identifier. Example: @@ -575,6 +584,17 @@ impl Foo for Bar { // ok! // functions implementation } ``` + +or + +``` +trait Foo { + // some functions +} + +fn bar(t: T) {} // ok! +``` + "##, E0405: r##" diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index e4e9ee58330cc..42bcc62f291c8 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2163,6 +2163,7 @@ impl<'a> Resolver<'a> { result } + /// This is called to resolve a trait reference from an `impl` (i.e. `impl Trait for Foo`) fn with_optional_trait_ref(&mut self, opt_trait_ref: Option<&TraitRef>, f: F) -> T where F: FnOnce(&mut Resolver, Option) -> T { @@ -2172,13 +2173,14 @@ impl<'a> Resolver<'a> { let path: Vec<_> = trait_ref.path.segments.iter() .map(|seg| respan(seg.span, seg.identifier)) .collect(); - let def = self.smart_resolve_path_fragment(trait_ref.ref_id, - None, - &path, - trait_ref.path.span, - trait_ref.path.segments.last().unwrap().span, - PathSource::Trait(AliasPossibility::No)) - .base_def(); + let def = self.smart_resolve_path_fragment( + trait_ref.ref_id, + None, + &path, + trait_ref.path.span, + trait_ref.path.segments.last().unwrap().span, + PathSource::Trait(AliasPossibility::No) + ).base_def(); if def != Def::Err { new_id = Some(def.def_id()); let span = trait_ref.path.span; diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 650e530519887..782638af13f73 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -313,7 +313,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { /// Instantiates the path for the given trait reference, assuming that it's /// bound to a valid trait type. Returns the def_id for the defining trait. - /// Fails if the type is a type other than a trait type. + /// The type _cannot_ be a type other than a trait type. /// /// If the `projections` argument is `None`, then assoc type bindings like `Foo` /// are disallowed. Otherwise, they are pushed onto the vector given. @@ -331,6 +331,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { trait_ref.path.segments.last().unwrap()) } + /// Get the DefId of the given trait ref. It _must_ actually be a trait. fn trait_def_id(&self, trait_ref: &hir::TraitRef) -> DefId { let path = &trait_ref.path; match path.def { @@ -339,13 +340,11 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { Def::Err => { self.tcx().sess.fatal("cannot continue compilation due to previous error"); } - _ => { - span_fatal!(self.tcx().sess, path.span, E0245, "`{}` is not a trait", - self.tcx().hir.node_to_pretty_string(trait_ref.ref_id)); - } + _ => unreachable!(), } } + /// The given `trait_ref` must actually be trait. pub(super) fn instantiate_poly_trait_ref_inner(&self, trait_ref: &hir::TraitRef, self_ty: Ty<'tcx>, diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 1c0e084832ebc..6c195a991c24e 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -4802,7 +4802,7 @@ register_diagnostics! { // E0240, // E0241, // E0242, - E0245, // not a trait +// E0245, // not a trait // E0246, // invalid recursive type // E0247, // E0248, // value used as a type, now reported earlier during resolution as E0412 diff --git a/src/test/ui/error-codes/E0404.rs b/src/test/ui/error-codes/E0404.rs index 1c088a71d7d4b..1c6ff5ae8413d 100644 --- a/src/test/ui/error-codes/E0404.rs +++ b/src/test/ui/error-codes/E0404.rs @@ -13,5 +13,6 @@ struct Bar; impl Foo for Bar {} //~ ERROR E0404 -fn main() { -} +fn main() {} + +fn baz(_: T) {} //~ ERROR E0404 diff --git a/src/test/ui/error-codes/E0404.stderr b/src/test/ui/error-codes/E0404.stderr index d49c0f3ea5e19..ac1d2a00cf448 100644 --- a/src/test/ui/error-codes/E0404.stderr +++ b/src/test/ui/error-codes/E0404.stderr @@ -4,6 +4,12 @@ error[E0404]: expected trait, found struct `Foo` LL | impl Foo for Bar {} //~ ERROR E0404 | ^^^ not a trait +error[E0404]: expected trait, found struct `Foo` + --> $DIR/E0404.rs:18:11 + | +LL | fn baz(_: T) {} //~ ERROR E0404 + | ^^^ not a trait + error: cannot continue compilation due to previous error If you want more information on this error, try using "rustc --explain E0404" From 082dd6d7af37ac76d99147ae7242080d4f5c74aa Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Wed, 28 Feb 2018 20:50:26 +0100 Subject: [PATCH 16/21] Fix a few run-pass tests --- src/test/run-pass/tls-init-on-init.rs | 11 ++++------- src/test/run-pass/tls-try-with.rs | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/test/run-pass/tls-init-on-init.rs b/src/test/run-pass/tls-init-on-init.rs index b5b9fb561ae7e..48a0d4a99ecc9 100644 --- a/src/test/run-pass/tls-init-on-init.rs +++ b/src/test/run-pass/tls-init-on-init.rs @@ -10,10 +10,9 @@ // ignore-emscripten no threads support -#![feature(thread_local_state)] -#![allow(deprecated)] +#![feature(thread_local_try_with)] -use std::thread::{self, LocalKeyState}; +use std::thread; use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT}; struct Foo { cnt: usize } @@ -38,10 +37,8 @@ impl Drop for Foo { FOO.with(|foo| assert_eq!(foo.cnt, 0)); } else { assert_eq!(self.cnt, 0); - match FOO.state() { - LocalKeyState::Valid => panic!("should not be in valid state"), - LocalKeyState::Uninitialized | - LocalKeyState::Destroyed => {} + if FOO.try_with(|_| ()).is_ok() { + panic!("should not be in valid state"); } } } diff --git a/src/test/run-pass/tls-try-with.rs b/src/test/run-pass/tls-try-with.rs index c072ec0679d73..552f4c5e829e1 100644 --- a/src/test/run-pass/tls-try-with.rs +++ b/src/test/run-pass/tls-try-with.rs @@ -10,7 +10,7 @@ // ignore-emscripten no threads support -#![feature(thread_local_state)] +#![feature(thread_local_try_with)] use std::thread; From 804666f4ad5a66e6a1bae3b2e326efa030135a05 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 26 Feb 2018 18:59:47 -0800 Subject: [PATCH 17/21] rustc: Tweak funclet cleanups of ffi functions This commit is targeted at addressing #48251 by specifically fixing a case where a longjmp over Rust frames on MSVC runs cleanups, accidentally running the "abort the program" cleanup as well. Added in #46833 `extern` ABI functions in Rust will abort the process if Rust panics, and currently this is modeled as a normal cleanup like all other destructors. Unfortunately it turns out that `longjmp` on MSVC is implemented with SEH, the same mechanism used to implement panics in Rust. This means that `longjmp` over Rust frames will run Rust cleanups (even though we don't necessarily want it to). Notably this means that if you `longjmp` over a Rust stack frame then that probably means you'll abort the program because one of the cleanups will abort the process. After some discussion on IRC it turns out that `longjmp` doesn't run cleanups for *caught* exceptions, it only runs cleanups for cleanup pads. Using this information this commit tweaks the codegen for an `extern` function to a catch-all clause for exceptions instead of a cleanup block. This catch-all is equivalent to the C++ code: try { foo(); } catch (...) { bar(); } and in fact our codegen here is designed to match exactly what clang emits for that C++ code! With this tweak a longjmp over Rust code will no longer abort the process. A longjmp will continue to "accidentally" run Rust cleanups (destructors) on MSVC. Other non-MSVC platforms will not rust destructors with a longjmp, so we'll probably still recommend "don't have destructors on the stack", but in any case this is a more surgical fix than #48567 and should help us stick to standard personality functions a bit longer. --- src/librustc_trans/mir/mod.rs | 60 +++++++++++++++++-- .../run-make/longjmp-across-rust/Makefile | 5 ++ src/test/run-make/longjmp-across-rust/foo.c | 28 +++++++++ src/test/run-make/longjmp-across-rust/main.rs | 40 +++++++++++++ 4 files changed, 127 insertions(+), 6 deletions(-) create mode 100644 src/test/run-make/longjmp-across-rust/Makefile create mode 100644 src/test/run-make/longjmp-across-rust/foo.c create mode 100644 src/test/run-make/longjmp-across-rust/main.rs diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs index da01592d9118a..99e3a59e2c4f4 100644 --- a/src/librustc_trans/mir/mod.rs +++ b/src/librustc_trans/mir/mod.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use common::{C_i32, C_null}; use libc::c_uint; use llvm::{self, ValueRef, BasicBlockRef}; use llvm::debuginfo::DIScope; @@ -23,6 +24,7 @@ use common::{CodegenCx, Funclet}; use debuginfo::{self, declare_local, VariableAccess, VariableKind, FunctionDebugContext}; use monomorphize::Instance; use abi::{ArgAttribute, FnType, PassMode}; +use type_::Type; use syntax_pos::{DUMMY_SP, NO_EXPANSION, BytePos, Span}; use syntax::symbol::keywords; @@ -222,7 +224,7 @@ pub fn trans_mir<'a, 'tcx: 'a>( // Compute debuginfo scopes from MIR scopes. let scopes = debuginfo::create_mir_scopes(cx, mir, &debug_context); - let (landing_pads, funclets) = create_funclets(&bx, &cleanup_kinds, &block_bxs); + let (landing_pads, funclets) = create_funclets(mir, &bx, &cleanup_kinds, &block_bxs); let mut fx = FunctionCx { mir, @@ -333,6 +335,7 @@ pub fn trans_mir<'a, 'tcx: 'a>( } fn create_funclets<'a, 'tcx>( + mir: &'a Mir<'tcx>, bx: &Builder<'a, 'tcx>, cleanup_kinds: &IndexVec, block_bxs: &IndexVec) @@ -341,14 +344,59 @@ fn create_funclets<'a, 'tcx>( { block_bxs.iter_enumerated().zip(cleanup_kinds).map(|((bb, &llbb), cleanup_kind)| { match *cleanup_kind { - CleanupKind::Funclet if base::wants_msvc_seh(bx.sess()) => { + CleanupKind::Funclet if base::wants_msvc_seh(bx.sess()) => {} + _ => return (None, None) + } + + let cleanup; + let ret_llbb; + match mir[bb].terminator.as_ref().map(|t| &t.kind) { + // This is a basic block that we're aborting the program for, + // notably in an `extern` function. These basic blocks are inserted + // so that we assert that `extern` functions do indeed not panic, + // and if they do we abort the process. + // + // On MSVC these are tricky though (where we're doing funclets). If + // we were to do a cleanuppad (like below) the normal functions like + // `longjmp` would trigger the abort logic, terminating the + // program. Instead we insert the equivalent of `catch(...)` for C++ + // which magically doesn't trigger when `longjmp` files over this + // frame. + // + // Lots more discussion can be found on #48251 but this codegen is + // modeled after clang's for: + // + // try { + // foo(); + // } catch (...) { + // bar(); + // } + Some(&mir::TerminatorKind::Abort) => { + let cs_bx = bx.build_sibling_block(&format!("cs_funclet{:?}", bb)); + let cp_bx = bx.build_sibling_block(&format!("cp_funclet{:?}", bb)); + ret_llbb = cs_bx.llbb(); + + let cs = cs_bx.catch_switch(None, None, 1); + cs_bx.add_handler(cs, cp_bx.llbb()); + + // The "null" here is actually a RTTI type descriptor for the + // C++ personality function, but `catch (...)` has no type so + // it's null. The 64 here is actually a bitfield which + // represents that this is a catch-all block. + let null = C_null(Type::i8p(bx.cx)); + let sixty_four = C_i32(bx.cx, 64); + cleanup = cp_bx.catch_pad(cs, &[null, sixty_four, null]); + cp_bx.br(llbb); + } + _ => { let cleanup_bx = bx.build_sibling_block(&format!("funclet_{:?}", bb)); - let cleanup = cleanup_bx.cleanup_pad(None, &[]); + ret_llbb = cleanup_bx.llbb(); + cleanup = cleanup_bx.cleanup_pad(None, &[]); cleanup_bx.br(llbb); - (Some(cleanup_bx.llbb()), Some(Funclet::new(cleanup))) } - _ => (None, None) - } + }; + + (Some(ret_llbb), Some(Funclet::new(cleanup))) }).unzip() } diff --git a/src/test/run-make/longjmp-across-rust/Makefile b/src/test/run-make/longjmp-across-rust/Makefile new file mode 100644 index 0000000000000..9d71ed8fcf3ab --- /dev/null +++ b/src/test/run-make/longjmp-across-rust/Makefile @@ -0,0 +1,5 @@ +-include ../tools.mk + +all: $(call NATIVE_STATICLIB,foo) + $(RUSTC) main.rs + $(call RUN,main) diff --git a/src/test/run-make/longjmp-across-rust/foo.c b/src/test/run-make/longjmp-across-rust/foo.c new file mode 100644 index 0000000000000..eb9939576741b --- /dev/null +++ b/src/test/run-make/longjmp-across-rust/foo.c @@ -0,0 +1,28 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#include +#include + +static jmp_buf ENV; + +extern void test_middle(); + +void test_start(void(*f)()) { + if (setjmp(ENV) != 0) + return; + f(); + assert(0); +} + +void test_end() { + longjmp(ENV, 1); + assert(0); +} diff --git a/src/test/run-make/longjmp-across-rust/main.rs b/src/test/run-make/longjmp-across-rust/main.rs new file mode 100644 index 0000000000000..c420473a560eb --- /dev/null +++ b/src/test/run-make/longjmp-across-rust/main.rs @@ -0,0 +1,40 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[link(name = "foo", kind = "static")] +extern { + fn test_start(f: extern fn()); + fn test_end(); +} + +fn main() { + unsafe { + test_start(test_middle); + } +} + +struct A; + +impl Drop for A { + fn drop(&mut self) { + } +} + +extern fn test_middle() { + let _a = A; + foo(); +} + +fn foo() { + let _a = A; + unsafe { + test_end(); + } +} From c9aff92e6dc3ea43228d3d4e24ee7f5485943569 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 24 Feb 2018 15:27:06 +0300 Subject: [PATCH 18/21] Support parentheses in patterns under feature gate Improve recovery for trailing comma after `..` --- src/librustc/hir/lowering.rs | 154 +++++++++--------- src/librustc_lint/builtin.rs | 1 + src/libsyntax/ast.rs | 4 +- src/libsyntax/feature_gate.rs | 7 + src/libsyntax/fold.rs | 1 + src/libsyntax/parse/parser.rs | 66 +++++--- src/libsyntax/print/pprust.rs | 5 + src/libsyntax/visit.rs | 3 +- src/test/parse-fail/pat-tuple-2.rs | 2 +- .../pat-tuple-7.rs} | 4 +- .../ui/feature-gate-pattern_parentheses.rs | 15 ++ .../feature-gate-pattern_parentheses.stderr | 11 ++ 12 files changed, 166 insertions(+), 107 deletions(-) rename src/test/{parse-fail/pat-tuple-6.rs => run-pass/pat-tuple-7.rs} (83%) create mode 100644 src/test/ui/feature-gate-pattern_parentheses.rs create mode 100644 src/test/ui/feature-gate-pattern_parentheses.stderr diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 89ed47ea194fb..99bf8486e9f64 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2465,86 +2465,88 @@ impl<'a> LoweringContext<'a> { } fn lower_pat(&mut self, p: &Pat) -> P { - let LoweredNodeId { node_id, hir_id } = self.lower_node_id(p.id); + let node = match p.node { + PatKind::Wild => hir::PatKind::Wild, + PatKind::Ident(ref binding_mode, pth1, ref sub) => { + match self.resolver.get_resolution(p.id).map(|d| d.base_def()) { + // `None` can occur in body-less function signatures + def @ None | def @ Some(Def::Local(_)) => { + let canonical_id = match def { + Some(Def::Local(id)) => id, + _ => p.id + }; + hir::PatKind::Binding(self.lower_binding_mode(binding_mode), + canonical_id, + respan(pth1.span, pth1.node.name), + sub.as_ref().map(|x| self.lower_pat(x))) + } + Some(def) => { + hir::PatKind::Path(hir::QPath::Resolved(None, P(hir::Path { + span: pth1.span, + def, + segments: hir_vec![ + hir::PathSegment::from_name(pth1.node.name) + ], + }))) + } + } + } + PatKind::Lit(ref e) => hir::PatKind::Lit(P(self.lower_expr(e))), + PatKind::TupleStruct(ref path, ref pats, ddpos) => { + let qpath = self.lower_qpath(p.id, &None, path, ParamMode::Optional, + ImplTraitContext::Disallowed); + hir::PatKind::TupleStruct(qpath, + pats.iter().map(|x| self.lower_pat(x)).collect(), + ddpos) + } + PatKind::Path(ref qself, ref path) => { + hir::PatKind::Path(self.lower_qpath(p.id, qself, path, ParamMode::Optional, + ImplTraitContext::Disallowed)) + } + PatKind::Struct(ref path, ref fields, etc) => { + let qpath = self.lower_qpath(p.id, &None, path, ParamMode::Optional, + ImplTraitContext::Disallowed); + + let fs = fields.iter() + .map(|f| { + Spanned { + span: f.span, + node: hir::FieldPat { + name: self.lower_ident(f.node.ident), + pat: self.lower_pat(&f.node.pat), + is_shorthand: f.node.is_shorthand, + }, + } + }) + .collect(); + hir::PatKind::Struct(qpath, fs, etc) + } + PatKind::Tuple(ref elts, ddpos) => { + hir::PatKind::Tuple(elts.iter().map(|x| self.lower_pat(x)).collect(), ddpos) + } + PatKind::Box(ref inner) => hir::PatKind::Box(self.lower_pat(inner)), + PatKind::Ref(ref inner, mutbl) => { + hir::PatKind::Ref(self.lower_pat(inner), self.lower_mutability(mutbl)) + } + PatKind::Range(ref e1, ref e2, ref end) => { + hir::PatKind::Range(P(self.lower_expr(e1)), + P(self.lower_expr(e2)), + self.lower_range_end(end)) + } + PatKind::Slice(ref before, ref slice, ref after) => { + hir::PatKind::Slice(before.iter().map(|x| self.lower_pat(x)).collect(), + slice.as_ref().map(|x| self.lower_pat(x)), + after.iter().map(|x| self.lower_pat(x)).collect()) + } + PatKind::Paren(ref inner) => return self.lower_pat(inner), + PatKind::Mac(_) => panic!("Shouldn't exist here"), + }; + let LoweredNodeId { node_id, hir_id } = self.lower_node_id(p.id); P(hir::Pat { id: node_id, hir_id, - node: match p.node { - PatKind::Wild => hir::PatKind::Wild, - PatKind::Ident(ref binding_mode, pth1, ref sub) => { - match self.resolver.get_resolution(p.id).map(|d| d.base_def()) { - // `None` can occur in body-less function signatures - def @ None | def @ Some(Def::Local(_)) => { - let canonical_id = match def { - Some(Def::Local(id)) => id, - _ => p.id - }; - hir::PatKind::Binding(self.lower_binding_mode(binding_mode), - canonical_id, - respan(pth1.span, pth1.node.name), - sub.as_ref().map(|x| self.lower_pat(x))) - } - Some(def) => { - hir::PatKind::Path(hir::QPath::Resolved(None, P(hir::Path { - span: pth1.span, - def, - segments: hir_vec![ - hir::PathSegment::from_name(pth1.node.name) - ], - }))) - } - } - } - PatKind::Lit(ref e) => hir::PatKind::Lit(P(self.lower_expr(e))), - PatKind::TupleStruct(ref path, ref pats, ddpos) => { - let qpath = self.lower_qpath(p.id, &None, path, ParamMode::Optional, - ImplTraitContext::Disallowed); - hir::PatKind::TupleStruct(qpath, - pats.iter().map(|x| self.lower_pat(x)).collect(), - ddpos) - } - PatKind::Path(ref qself, ref path) => { - hir::PatKind::Path(self.lower_qpath(p.id, qself, path, ParamMode::Optional, - ImplTraitContext::Disallowed)) - } - PatKind::Struct(ref path, ref fields, etc) => { - let qpath = self.lower_qpath(p.id, &None, path, ParamMode::Optional, - ImplTraitContext::Disallowed); - - let fs = fields.iter() - .map(|f| { - Spanned { - span: f.span, - node: hir::FieldPat { - name: self.lower_ident(f.node.ident), - pat: self.lower_pat(&f.node.pat), - is_shorthand: f.node.is_shorthand, - }, - } - }) - .collect(); - hir::PatKind::Struct(qpath, fs, etc) - } - PatKind::Tuple(ref elts, ddpos) => { - hir::PatKind::Tuple(elts.iter().map(|x| self.lower_pat(x)).collect(), ddpos) - } - PatKind::Box(ref inner) => hir::PatKind::Box(self.lower_pat(inner)), - PatKind::Ref(ref inner, mutbl) => { - hir::PatKind::Ref(self.lower_pat(inner), self.lower_mutability(mutbl)) - } - PatKind::Range(ref e1, ref e2, ref end) => { - hir::PatKind::Range(P(self.lower_expr(e1)), - P(self.lower_expr(e2)), - self.lower_range_end(end)) - } - PatKind::Slice(ref before, ref slice, ref after) => { - hir::PatKind::Slice(before.iter().map(|x| self.lower_pat(x)).collect(), - slice.as_ref().map(|x| self.lower_pat(x)), - after.iter().map(|x| self.lower_pat(x)).collect()) - } - PatKind::Mac(_) => panic!("Shouldn't exist here"), - }, + node, span: p.span, }) } diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index de55710bdf3d0..66f34a72edf2d 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -737,6 +737,7 @@ impl EarlyLintPass for IllegalFloatLiteralPattern { PatKind::TupleStruct(..) | PatKind::Ref(..) | PatKind::Box(..) | + PatKind::Paren(..) | PatKind::Slice(..) => (), // Extract the expressions and check them diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 6609b77b132c6..40000bc378ed0 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -562,7 +562,7 @@ impl Pat { PatKind::TupleStruct(_, ref s, _) | PatKind::Tuple(ref s, _) => { s.iter().all(|p| p.walk(it)) } - PatKind::Box(ref s) | PatKind::Ref(ref s, _) => { + PatKind::Box(ref s) | PatKind::Ref(ref s, _) | PatKind::Paren(ref s) => { s.walk(it) } PatKind::Slice(ref before, ref slice, ref after) => { @@ -656,6 +656,8 @@ pub enum PatKind { /// `[a, b, ..i, y, z]` is represented as: /// `PatKind::Slice(box [a, b], Some(i), box [y, z])` Slice(Vec>, Option>, Vec>), + /// Parentheses in patters used for grouping, i.e. `(PAT)`. + Paren(P), /// A macro pattern; pre-expansion Mac(Mac), } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 1ebf52e9fe8c0..c9ac34aa91784 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -449,6 +449,9 @@ declare_features! ( // Multiple patterns with `|` in `if let` and `while let` (active, if_while_or_patterns, "1.26.0", Some(48215)), + + // Parentheses in patterns + (active, pattern_parentheses, "1.26.0", None), ); declare_features! ( @@ -1663,6 +1666,10 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { gate_feature_post!(&self, dotdoteq_in_patterns, pattern.span, "`..=` syntax in patterns is experimental"); } + PatKind::Paren(..) => { + gate_feature_post!(&self, pattern_parentheses, pattern.span, + "parentheses in patterns are unstable"); + } _ => {} } visit::walk_pat(self, pattern) diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index e8eb75f5e6018..1963ab45f1a32 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -1148,6 +1148,7 @@ pub fn noop_fold_pat(p: P, folder: &mut T) -> P { slice.map(|x| folder.fold_pat(x)), after.move_map(|x| folder.fold_pat(x))) } + PatKind::Paren(inner) => PatKind::Paren(folder.fold_pat(inner)), PatKind::Mac(mac) => PatKind::Mac(folder.fold_mac(mac)) }, span: folder.new_span(span) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 09dd00fa5fa3a..881e3e412d4eb 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3484,33 +3484,47 @@ impl<'a> Parser<'a> { }; } - fn parse_pat_tuple_elements(&mut self, unary_needs_comma: bool) - -> PResult<'a, (Vec>, Option)> { - let mut fields = vec![]; - let mut ddpos = None; + // Parses a parenthesized list of patterns like + // `()`, `(p)`, `(p,)`, `(p, q)`, or `(p, .., q)`. Returns: + // - a vector of the patterns that were parsed + // - an option indicating the index of the `..` element + // - a boolean indicating whether a trailing comma was present. + // Trailing commas are significant because (p) and (p,) are different patterns. + fn parse_parenthesized_pat_list(&mut self) -> PResult<'a, (Vec>, Option, bool)> { + self.expect(&token::OpenDelim(token::Paren))?; - while !self.check(&token::CloseDelim(token::Paren)) { - if ddpos.is_none() && self.eat(&token::DotDot) { - ddpos = Some(fields.len()); - if self.eat(&token::Comma) { - // `..` needs to be followed by `)` or `, pat`, `..,)` is disallowed. - fields.push(self.parse_pat()?); + let mut fields = Vec::new(); + let mut ddpos = None; + let mut trailing_comma = false; + loop { + if self.eat(&token::DotDot) { + if ddpos.is_none() { + ddpos = Some(fields.len()); + } else { + // Emit a friendly error, ignore `..` and continue parsing + self.span_err(self.prev_span, + "`..` can only be used once per tuple or tuple struct pattern"); } - } else if ddpos.is_some() && self.eat(&token::DotDot) { - // Emit a friendly error, ignore `..` and continue parsing - self.span_err(self.prev_span, "`..` can only be used once per \ - tuple or tuple struct pattern"); - } else { + } else if !self.check(&token::CloseDelim(token::Paren)) { fields.push(self.parse_pat()?); + } else { + break } - if !self.check(&token::CloseDelim(token::Paren)) || - (unary_needs_comma && fields.len() == 1 && ddpos.is_none()) { - self.expect(&token::Comma)?; + trailing_comma = self.eat(&token::Comma); + if !trailing_comma { + break } } - Ok((fields, ddpos)) + if ddpos == Some(fields.len()) && trailing_comma { + // `..` needs to be followed by `)` or `, pat`, `..,)` is disallowed. + self.span_err(self.prev_span, "trailing comma is not permitted after `..`"); + } + + self.expect(&token::CloseDelim(token::Paren))?; + + Ok((fields, ddpos, trailing_comma)) } fn parse_pat_vec_elements( @@ -3714,10 +3728,12 @@ impl<'a> Parser<'a> { } token::OpenDelim(token::Paren) => { // Parse (pat,pat,pat,...) as tuple pattern - self.bump(); - let (fields, ddpos) = self.parse_pat_tuple_elements(true)?; - self.expect(&token::CloseDelim(token::Paren))?; - pat = PatKind::Tuple(fields, ddpos); + let (fields, ddpos, trailing_comma) = self.parse_parenthesized_pat_list()?; + pat = if fields.len() == 1 && ddpos.is_none() && !trailing_comma { + PatKind::Paren(fields.into_iter().nth(0).unwrap()) + } else { + PatKind::Tuple(fields, ddpos) + }; } token::OpenDelim(token::Bracket) => { // Parse [pat,pat,...] as slice pattern @@ -3807,9 +3823,7 @@ impl<'a> Parser<'a> { return Err(self.fatal("unexpected `(` after qualified path")); } // Parse tuple struct or enum pattern - self.bump(); - let (fields, ddpos) = self.parse_pat_tuple_elements(false)?; - self.expect(&token::CloseDelim(token::Paren))?; + let (fields, ddpos, _) = self.parse_parenthesized_pat_list()?; pat = PatKind::TupleStruct(path, fields, ddpos) } _ => pat = PatKind::Path(qself, path), diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 9cad9f46e98cf..77afafbb4e003 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -2659,6 +2659,11 @@ impl<'a> State<'a> { |s, p| s.print_pat(p))?; self.s.word("]")?; } + PatKind::Paren(ref inner) => { + self.popen()?; + self.print_pat(inner)?; + self.pclose()?; + } PatKind::Mac(ref m) => self.print_mac(m, token::Paren)?, } self.ann.post(self, NodePat(pat)) diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index 640f90ecb4a43..5a24c61cb5aaf 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -425,7 +425,8 @@ pub fn walk_pat<'a, V: Visitor<'a>>(visitor: &mut V, pattern: &'a Pat) { walk_list!(visitor, visit_pat, tuple_elements); } PatKind::Box(ref subpattern) | - PatKind::Ref(ref subpattern, _) => { + PatKind::Ref(ref subpattern, _) | + PatKind::Paren(ref subpattern) => { visitor.visit_pat(subpattern) } PatKind::Ident(_, ref pth1, ref optional_subpattern) => { diff --git a/src/test/parse-fail/pat-tuple-2.rs b/src/test/parse-fail/pat-tuple-2.rs index ad52fa5787007..07b391d327984 100644 --- a/src/test/parse-fail/pat-tuple-2.rs +++ b/src/test/parse-fail/pat-tuple-2.rs @@ -12,6 +12,6 @@ fn main() { match 0 { - (pat, ..,) => {} //~ ERROR expected pattern, found `)` + (pat, ..,) => {} //~ ERROR trailing comma is not permitted after `..` } } diff --git a/src/test/parse-fail/pat-tuple-6.rs b/src/test/run-pass/pat-tuple-7.rs similarity index 83% rename from src/test/parse-fail/pat-tuple-6.rs rename to src/test/run-pass/pat-tuple-7.rs index 3252d92fe1b50..6d51df63e158a 100644 --- a/src/test/parse-fail/pat-tuple-6.rs +++ b/src/test/run-pass/pat-tuple-7.rs @@ -8,10 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// compile-flags: -Z parse-only +#![feature(pattern_parentheses)] fn main() { match 0 { - (pat) => {} //~ ERROR expected one of `,` or `@`, found `)` + (pat) => assert_eq!(pat, 0) } } diff --git a/src/test/ui/feature-gate-pattern_parentheses.rs b/src/test/ui/feature-gate-pattern_parentheses.rs new file mode 100644 index 0000000000000..29768018f0e44 --- /dev/null +++ b/src/test/ui/feature-gate-pattern_parentheses.rs @@ -0,0 +1,15 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + match 0 { + (pat) => {} //~ ERROR parentheses in patterns are unstable + } +} diff --git a/src/test/ui/feature-gate-pattern_parentheses.stderr b/src/test/ui/feature-gate-pattern_parentheses.stderr new file mode 100644 index 0000000000000..4fc1441a0fadd --- /dev/null +++ b/src/test/ui/feature-gate-pattern_parentheses.stderr @@ -0,0 +1,11 @@ +error[E0658]: parentheses in patterns are unstable + --> $DIR/feature-gate-pattern_parentheses.rs:13:9 + | +LL | (pat) => {} //~ ERROR parentheses in patterns are unstable + | ^^^^^ + | + = help: add #![feature(pattern_parentheses)] to the crate attributes to enable + +error: aborting due to previous error + +If you want more information on this error, try using "rustc --explain E0658" From cb56b2d1522e83c5bb0613abcf78b686e994df9e Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Thu, 1 Mar 2018 00:07:27 +0100 Subject: [PATCH 19/21] Fix a bug introduced in previous commit --- src/libstd/io/stdio.rs | 4 ++-- src/libstd/thread/local.rs | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index b8fb83ad46597..1f73054e3beed 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -663,8 +663,8 @@ pub fn set_print(sink: Option>) -> Option> { /// /// This function is used to print error messages, so it takes extra /// care to avoid causing a panic when `local_stream` is unusable. -/// For instance, if the TLS key for the local stream is uninitialized -/// or already destroyed, or if the local stream is locked by another +/// For instance, if the TLS key for the local stream is +/// already destroyed, or if the local stream is locked by another /// thread, it will just fall back to the global stream. /// /// However, if the actual I/O causes an error, this function does panic. diff --git a/src/libstd/thread/local.rs b/src/libstd/thread/local.rs index 25fedcb277265..99479bc56eff3 100644 --- a/src/libstd/thread/local.rs +++ b/src/libstd/thread/local.rs @@ -272,7 +272,7 @@ impl LocalKey { /// /// This will lazily initialize the value if this thread has not referenced /// this key yet. If the key has been destroyed (which may happen if this is called - /// in a destructor), this function will return a ThreadLocalError. + /// in a destructor), this function will return a `ThreadLocalError`. /// /// # Panics /// @@ -484,11 +484,7 @@ mod tests { assert!(FOO.try_with(|_| ()).is_err()); } } - fn foo() -> Foo { - assert!(FOO.try_with(|_| ()).is_err()); - Foo - } - thread_local!(static FOO: Foo = foo()); + thread_local!(static FOO: Foo = Foo); thread::spawn(|| { assert!(FOO.try_with(|_| ()).is_ok()); @@ -520,6 +516,7 @@ mod tests { impl Drop for S1 { fn drop(&mut self) { unsafe { + HITS += 1; if K2.try_with(|_| ()).is_err() { assert_eq!(HITS, 3); } else { From 6edbe374372f8afed59cfa2fd4ea6a3ca3beca59 Mon Sep 17 00:00:00 2001 From: Tobias Stolzmann Date: Thu, 1 Mar 2018 03:04:26 +0100 Subject: [PATCH 20/21] Fix link to rustc guide in README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index fd35606ec0dbb..e78bbb82711ab 100644 --- a/README.md +++ b/README.md @@ -233,7 +233,7 @@ find out how various parts of the compiler work. [IRC]: https://en.wikipedia.org/wiki/Internet_Relay_Chat [#rust]: irc://irc.mozilla.org/rust [#rust-beginners]: irc://irc.mozilla.org/rust-beginners -[rustc-guide]: https://rust-lang-nursery.github.io/rustc-guide/about-this-guide.html +[rustc guide]: https://rust-lang-nursery.github.io/rustc-guide/about-this-guide.html ## License [license]: #license From 363d6040fdac7baa8eb18a804f02832fec5ad35f Mon Sep 17 00:00:00 2001 From: Ryan Cumming Date: Thu, 1 Mar 2018 17:51:14 +1100 Subject: [PATCH 21/21] Add ignore-pretty for issue-48506.rs The out-of-line module #37195 --- src/test/run-pass/issue-48508.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/run-pass/issue-48508.rs b/src/test/run-pass/issue-48508.rs index 303ea2aa6012f..1b10d873f11c4 100644 --- a/src/test/run-pass/issue-48508.rs +++ b/src/test/run-pass/issue-48508.rs @@ -16,6 +16,7 @@ // issue-48508-aux.rs // compile-flags:-g +// ignore-pretty issue #37195 #![feature(non_ascii_idents)]