Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

or_patterns: implement :pat edition-specific behavior #80100

Merged
merged 1 commit into from
Dec 20, 2020

Conversation

mark-i-m
Copy link
Member

cc #54883 @joshtriplett

This PR implements the edition-specific behavior of :pat wrt or-patterns, as determined by the crater runs and T-lang consensus in #54883 (comment).

I believe this can unblock stabilization of or_patterns.

r? @petrochenkov

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 17, 2020
@rust-log-analyzer

This comment has been minimized.

@mark-i-m
Copy link
Member Author

I keep getting:

Copying stage0 rustc from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Assembling stage1 compiler (x86_64-unknown-linux-gnu)
thread 'main' panicked at 'src.symlink_metadata() failed with No such file or directory (os error 2)', src/bootstrap/lib.rs:1216:24
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
failed to run: /home/mark/rust/build/bootstrap/debug/bootstrap test --bless
Build completed unsuccessfully in 0:17:56

I've tried everything...

@rust-log-analyzer

This comment has been minimized.

@petrochenkov
Copy link
Contributor

Some UI tests need an update.

@petrochenkov 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 Dec 17, 2020
@rust-log-analyzer

This comment has been minimized.

@mark-i-m
Copy link
Member Author

For some reason, I keep getting the same bootstrapping error above. I'm unable to run any tests locally :(

@petrochenkov
Copy link
Contributor

@mark-i-m
Copy link
Member Author

mark-i-m commented Dec 18, 2020

I think CI should be passing now. CI is passing now :)

@petrochenkov
Copy link
Contributor

r=me with commits squashed.

@mark-i-m
Copy link
Member Author

@petrochenkov squashed.

@mark-i-m
Copy link
Member Author

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 18, 2020
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 19, 2020

📌 Commit fd4ae3a has been approved by petrochenkov

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 19, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 19, 2020
or_patterns: implement :pat edition-specific behavior

cc rust-lang#54883 `@joshtriplett`

This PR implements the edition-specific behavior of `:pat` wrt or-patterns, as determined by the crater runs and T-lang consensus in rust-lang#54883 (comment).

I believe this can unblock stabilization of or_patterns.

r? `@petrochenkov`
@bors

This comment has been minimized.

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 19, 2020
@rust-log-analyzer
Copy link
Collaborator

The job dist-armv7-linux failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[TIMING] StdLink { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target_compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } -- 0.001
[TIMING] Std { target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None }, compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } } -- 76.664
Building LLVM for x86_64-unknown-linux-gnu
running: "cmake" "/checkout/src/llvm-project/llvm" "-G" "Ninja" "-DLLVM_ENABLE_ASSERTIONS=OFF" "-DLLVM_TARGETS_TO_BUILD=AArch64;ARM;Hexagon;MSP430;Mips;NVPTX;PowerPC;RISCV;Sparc;SystemZ;WebAssembly;X86" "-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=AVR" "-DLLVM_INCLUDE_EXAMPLES=OFF" "-DLLVM_INCLUDE_TESTS=OFF" "-DLLVM_INCLUDE_DOCS=OFF" "-DLLVM_INCLUDE_BENCHMARKS=OFF" "-DLLVM_ENABLE_TERMINFO=OFF" "-DLLVM_ENABLE_LIBEDIT=OFF" "-DLLVM_ENABLE_BINDINGS=OFF" "-DLLVM_ENABLE_Z3_SOLVER=OFF" "-DLLVM_PARALLEL_COMPILE_JOBS=16" "-DLLVM_TARGET_ARCH=x86_64" "-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-unknown-linux-gnu" "-DLLVM_ENABLE_ZLIB=ON" "-DCMAKE_EXE_LINKER_FLAGS=-Wl,-Bsymbolic -static-libstdc++" "-DLLVM_ENABLE_LIBXML2=OFF" "-DLLVM_VERSION_SUFFIX=-rust-1.50.0-nightly" "-DCMAKE_INSTALL_MESSAGE=LAZY" "-DCMAKE_C_COMPILER_LAUNCHER=sccache" "-DCMAKE_CXX_COMPILER_LAUNCHER=sccache" "-DCMAKE_C_COMPILER=cc" "-DCMAKE_CXX_COMPILER=c++" "-DCMAKE_ASM_COMPILER=cc" "-DCMAKE_C_FLAGS=-ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_CXX_FLAGS=-ffunction-sections -fdata-sections -fPIC -m64 -static-libstdc++" "-DCMAKE_INSTALL_PREFIX=/checkout/obj/build/x86_64-unknown-linux-gnu/llvm" "-DCMAKE_ASM_FLAGS= -ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_BUILD_TYPE=Release"
CMake Error: The source directory "/checkout/src/llvm-project/llvm" does not exist.
Specify --help for usage, or press the help button on the CMake GUI.
command did not execute successfully, got: exit code: 1

 finished in 0.011 seconds
 finished in 0.011 seconds
build script failed, must exit now', /cargo/registry/src/github.com-1ecc6299db9ec823/cmake-0.1.44/src/lib.rs:885:5
failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host armv7-unknown-linux-gnueabihf --target armv7-unknown-linux-gnueabihf
Build completed unsuccessfully in 0:01:17

@bors
Copy link
Contributor

bors commented Dec 19, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 19, 2020
@mark-i-m
Copy link
Member Author

I don't understand the CI failure. It looks like llvm is missing? Seems unrelated?

@petrochenkov
Copy link
Contributor

The failure looks unrelated to this PR.
@bors retry

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 19, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 20, 2020
or_patterns: implement :pat edition-specific behavior

cc rust-lang#54883 `@joshtriplett`

This PR implements the edition-specific behavior of `:pat` wrt or-patterns, as determined by the crater runs and T-lang consensus in rust-lang#54883 (comment).

I believe this can unblock stabilization of or_patterns.

r? `@petrochenkov`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 20, 2020
or_patterns: implement :pat edition-specific behavior

cc rust-lang#54883 ``@joshtriplett``

This PR implements the edition-specific behavior of `:pat` wrt or-patterns, as determined by the crater runs and T-lang consensus in rust-lang#54883 (comment).

I believe this can unblock stabilization of or_patterns.

r? ``@petrochenkov``
@bors
Copy link
Contributor

bors commented Dec 20, 2020

⌛ Testing commit 1a7d00a with merge 29e3212...

@bors
Copy link
Contributor

bors commented Dec 20, 2020

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 29e3212 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 20, 2020
@bors bors merged commit 29e3212 into rust-lang:master Dec 20, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 20, 2020
@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Dec 24, 2020

This seems to have been a slight regression in instruction counts (0.5%) on the deep vector benchmark. I think it would probably be good to modify the code to lazily query the edition if we encounter a pat pattern (or, even better, one that would differ in behavior) - edition queries are relatively expensive.

@mark-i-m
Copy link
Member Author

@Mark-Simulacrum Hmm... that's good to know. Yeah, I can do that. I think we will want to do something different altogether, though -- adding a new NonterminalKind for 2015/18/21 editions and their different :pats (if I understood the lang team consensus correctly). I think that should naturally handle the regression.

I can hack something together to address the regression now if you want, but I would prefer to just do the above (it might take a bit longer though; not sure I can do it before the beta branches)...

@Mark-Simulacrum
Copy link
Member

I am not worried about this hitting beta - regression is minor. I agree that the long term planning in this area probably means we don't need a temporary fix for now.

@mark-i-m mark-i-m deleted the pattORns-2 branch December 28, 2020 19:56
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 31, 2020
Implement edition-based macro :pat feature

This PR does two things:
1. Fixes the perf regression from rust-lang#80100 (comment)
2. Implements `:pat2018` and `:pat2021` matchers, as described by `@joshtriplett`  in rust-lang#54883 (comment) behind the feature gate `edition_macro_pat`.

r? `@petrochenkov`

cc `@Mark-Simulacrum`
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 31, 2020
Implement edition-based macro :pat feature

This PR does two things:
1. Fixes the perf regression from rust-lang#80100 (comment)
2. Implements `:pat2018` and `:pat2021` matchers, as described by `@joshtriplett`  in rust-lang#54883 (comment) behind the feature gate `edition_macro_pat`.

r? `@petrochenkov`

cc `@Mark-Simulacrum`
@mark-i-m
Copy link
Member Author

mark-i-m commented Jan 1, 2021

FYI, if someone is following up perf regressions, #80459 should resolve this regression and recently merged.

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2021
…matsakis

Stabilize or_patterns (RFC 2535, 2530, 2175)

closes rust-lang#54883

This PR stabilizes the or_patterns feature in Rust 1.53.

This is blocked on the following (in order):
- [x] The crater run in rust-lang#78935 (comment)
- [x] The resolution of the unresolved questions and a second crater run (rust-lang#78935 (comment))
    - It looks like we will need to pursue some sort of edition-based transition for `:pat`.
- [x] Nomination and discussion by T-lang
- [x] Implement new behavior for `:pat` based on consensus (rust-lang#80100).
- [ ] An FCP on stabilization

EDIT: Stabilization report is in rust-lang#79278 (comment)
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Mar 25, 2021
Stabilize or_patterns (RFC 2535, 2530, 2175)

closes #54883

This PR stabilizes the or_patterns feature in Rust 1.53.

This is blocked on the following (in order):
- [x] The crater run in rust-lang/rust#78935 (comment)
- [x] The resolution of the unresolved questions and a second crater run (rust-lang/rust#78935 (comment))
    - It looks like we will need to pursue some sort of edition-based transition for `:pat`.
- [x] Nomination and discussion by T-lang
- [x] Implement new behavior for `:pat` based on consensus (rust-lang/rust#80100).
- [ ] An FCP on stabilization

EDIT: Stabilization report is in rust-lang/rust#79278 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants