Removal of the lang feature gate tests whitelist #39059

est31 · 2017-01-14T09:11:48Z

PR #38914 has added a tidy check for gate tests in compile-fail for all unstable lang features, helping ensure that they are actually unstable. See also issue #22820.

To be recognized as gate test for a specific feature, tests either need to follow a naming scheme feature-gate-NAME-OF-THE-FEAT.rs or include a // gate-test-NAME-OF-THE-FEAT comment. For most of the currently present features, I could easily find matching tests in the test suite. Sadly, there is still a whitelist of currently 20 features for which it tolerates that it can't detect any gate tests.

The goal is to eliminate that whitelist, which this issue is tracking.

The features on the whitelist fall into the following categories:

Has already tests, they just need to be marked (found this out only on a second look):

more_struct_aliases → marked by PR [Gate Tests] - marking feature tests #39243
custom_attribute → marked by PR [Gate Tests] - marking feature tests #39243
default_type_parameter_fallback → marked by PR [Gate Tests] - marking feature tests #39243
use_extern_macros → marked by PR [Gate Tests] - marking feature tests #39243
quote → marked by PR [Gate Tests] - marking feature tests #39250
macro_reexport → marked by PR [Gate Tests] - marking feature tests #39250
inclusive_range_syntax → marked by PR Added test for inclusive_range_syntax in compile-fail test suite #40031

Used nowhere in the codebase. Remove?

got all removed by PR #39071

safe_suggestion
pushpop_unsafe

Needs a test (or I just haven't found a suiting test to mark, even on second look):

Don't know what to do with this one. Any ideas?

reflect → removed by PR Remove Reflect #39075

The "Needs a test" section is a good place for beginners to help out!

You basically need to:

if you haven't already, read CONTRIBUTING.md and COMPILER_TESTS.md
search in the src/test/run-pass suite for tests of that feature
copy one of those tests over to the compile-fail suite
rename it appropriately (feature-gate-FEAT-NAME.rs) and remove the #![feature(...)] line
remove the feature from the whitelist
test locally (./x.py test src/tools/tidy and ./x.py test src/test/compile-fail --test-args feature-gate-FEAT-NAME)
submit a PR!

It would also be very good if you searched the compile-fail testsuite for already existing gate tests and mark those instead. A good place to look is the PR that introduced that feature, they usually also add gate tests.

You can help out in the "Has already tests" section as well. Here, contributing is even easier!

You'll need to:

read CONTRIBUTING.md and COMPILER_TESTS.md (second one is not strictly required, but it never harms :D)
add a line with // gate-test-FEATURE_NAME to the files that were mentioned above
remove the feature from the whitelist
test locally (./x.py test src/tools/tidy, since you only added comments)
submit a PR!

If you need help, ask in this thread. Also, its best to prevent duplicate work, so it would be nice if you could announce that you work on some of the items. Thanks in advance!

The text was updated successfully, but these errors were encountered:

This removes the safe_suggestion feature from feature_gate.rs. It was added in commit 164f010 and then removed again in commit c11fe55 . As the removal was in the same PR rust-lang#38099 as the addition, we don't move it to the "removed" section. Removes an element from the whitelist of non gate tested unstable lang features (issue rust-lang#39059).

This marks the pushpop_unsafe feature as removed inside the feature_gate. It was added in commit 1829fa5 and then removed again in commit d399098 . Seems that the second commit forgot to mark it as removed in feature_gate.rs. This enables us to remove another element from the whitelist of non gate tested unstable lang features (issue rust-lang#39059).

Mark safe_suggestion and pushpop_unsafe as removed in feature_gate.rs This removes two features from feature_gate.rs: `safe_suggestion` and `pushpop_unsafe`. Both had been removed in other places already, but were forgotten to be removed from feature_gate.rs. * `safe_suggestion` was added in commit 164f010 and then removed again in commit c11fe55 both in the same PR #38099. * `pushpop_unsafe` was added in commit 1829fa5 and removed again in commit d399098 Removes two elements from the whitelist of non gate tested unstable lang features (issue #39059).

brson · 2017-01-18T00:38:13Z

Nice idea.

alanhoff · 2017-01-21T21:49:04Z

Hello folks, I'm just announcing that I'll try to do the Need a test section today

cseale · 2017-01-22T13:53:14Z

Guess I will do the already has tests section today!

est31 · 2017-01-22T14:46:00Z

@alanhoff @cseale wonderful! Ping me if you need any help or when you open your PR.

cseale · 2017-01-22T15:03:41Z

@est31 for example, in struct-path-self-feature-gate.rs it's just as straight forward as

// gate-test-more_struct_aliases
struct S;

trait Tr {
    type A;
}

fn f<T: Tr<A = S>>() {
    let _ = T::A {};
    //~^ ERROR `Self` and associated types in struct expressions and patterns are unstable
}

impl S {
    fn f() {
        let _ = Self {};
        //~^ ERROR `Self` and associated types in struct expressions and patterns are unstable
    }
}

fn main() {}

Where in the codebase is the build actually looking at these comments to detect if they should run or not? I'm used to annotations or functions declaring test groups in code (junit in java or jasmine in javascript). I am new to rust, is this a rust standard?

cseale · 2017-01-22T15:31:14Z

@est31 When I run the tidy check, everything run ok, but the following command produces an error:

$ ./x.py test src/test/compile-fail --test-args feature-gate-more_struct_aliases

Finished debug [unoptimized] target(s) in 0.0 secs
Synchronizing submodule url for 'src/compiler-rt'
Synchronizing submodule url for 'src/jemalloc'
Synchronizing submodule url for 'src/liblibc'
Synchronizing submodule url for 'src/llvm'
Synchronizing submodule url for 'src/rt/hoedown'
Synchronizing submodule url for 'src/rust-installer'
HEAD is now at a8fc4c1 Merge pull request #28 from xen0n/preprocessor-firefighting
HEAD is now at e058ca6 Change how the default zone is found
HEAD is now at e49e9bb Auto merge of #478 - jackpot51:patch-1, r=alexcrichton
HEAD is now at ceb177e Merge pull request #60 from japaric/gh38406
HEAD is now at a3736a0 Merge pull request #6 from intelfx/patch-1
HEAD is now at 4f99485 Merge pull request #54 from brson/docdir
Building stage0 std artifacts (x86_64-apple-darwin -> x86_64-apple-darwin)
warning: /Library/Developer/CommandLineTools/usr/bin/ranlib: file: /Users/user/projects/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/compiler_builtins-0511dd5445a3a209/out/libcompiler-rt.a(addtf3.o) has no symbols
warning: /Library/Developer/CommandLineTools/usr/bin/ranlib: file: /Users/user/projects/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/compiler_builtins-0511dd5445a3a209/out/libcompiler-rt.a(divtf3.o) has no symbols
warning: /Library/Developer/CommandLineTools/usr/bin/ranlib: file: /Users/user/projects/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/compiler_builtins-0511dd5445a3a209/out/libcompiler-rt.a(multf3.o) has no symbols
warning: /Library/Developer/CommandLineTools/usr/bin/ranlib: file: /Users/user/projects/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/compiler_builtins-0511dd5445a3a209/out/libcompiler-rt.a(powitf2.o) has no symbols
warning: /Library/Developer/CommandLineTools/usr/bin/ranlib: file: /Users/user/projects/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/compiler_builtins-0511dd5445a3a209/out/libcompiler-rt.a(subtf3.o) has no symbols
warning: /Library/Developer/CommandLineTools/usr/bin/ranlib: file: /Users/user/projects/rust/build/x86_64-apple-darwin/stage0-std/x86_64-apple-darwin/release/build/compiler_builtins-0511dd5445a3a209/out/libcompiler-rt.a(trampoline_setup.o) has no symbols
    Finished release [optimized] target(s) in 0.0 secs
Copying stage0 std from stage0 (x86_64-apple-darwin -> x86_64-apple-darwin / x86_64-apple-darwin)
Building stage0 test artifacts (x86_64-apple-darwin -> x86_64-apple-darwin)
    Finished release [optimized] target(s) in 0.0 secs
Copying stage0 test from stage0 (x86_64-apple-darwin -> x86_64-apple-darwin / x86_64-apple-darwin)
Building stage0 compiler artifacts (x86_64-apple-darwin -> x86_64-apple-darwin)
warning: ../rustllvm/RustWrapper.cpp:151:3: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default]
warning:   default:
warning:   ^
warning: ../rustllvm/RustWrapper.cpp:1235:3: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default]
warning:   default:
warning:   ^
warning: ../rustllvm/RustWrapper.cpp:1268:3: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default]
warning:   default:
warning:   ^
warning: ../rustllvm/RustWrapper.cpp:1282:3: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default]
warning:   default:
warning:   ^
warning: 4 warnings generated.
    Finished release [optimized] target(s) in 0.2 secs
Copying stage0 rustc from stage0 (x86_64-apple-darwin -> x86_64-apple-darwin / x86_64-apple-darwin)
Copying stage1 compiler (x86_64-apple-darwin)
thread 'main' panicked at 'failed to copy `/Users/user/projects/rust/build/x86_64-apple-darwin/stage0-sysroot/lib/rustlib/x86_64-apple-darwin/lib/libflate-89345601b107c0b9.dylib` to `/Users/user/projects/rust/build/x86_64-apple-darwin/stage1/lib/libflate-89345601b107c0b9.dylib`: Permission denied (os error 13)', src/bootstrap/util.rs:53
note: Run with `RUST_BACKTRACE=1` for a backtrace.

est31 · 2017-01-22T16:08:09Z

@cseale

Where in the codebase is the build actually looking at these comments to detect if they should run or not? I am new to rust, is this a rust standard?

No, this notation is actually internal to the rust compiler codebase. Its not used in normal rust codebases. The standard way of doing tests in rust is to use an attribute. The compiler however has special needs, and part of the information usually passed via attributes (like whether it should panic or not) is encoded in the path (run-pass vs compile-fail etc.) the test is inside. Also, the checking is done in an executable separate from the compiler, which doesn't have access to the parser. Doing it via comments makes this check simpler to implement. The specific check is implemented here: https://github.com/rust-lang/rust/blob/master/src/tools/tidy/src/features.rs#L118-L164

About your errors: don't know what the problem is. Did you do a clean clone? Running ./x.py test should not be very important for work in the "already has tests" section though, so if the tidy check is successful, you can open a PR :)

cseale · 2017-01-22T16:11:34Z

@est31 I think it's something to do with the sudo make install. I ran the build from source script when I first cloned rust, but I think it is causing issues when I try and run other build commands? How do I uninstall rust?

est31 · 2017-01-22T16:19:00Z

@cseale I'm not sure, maybe there is a script at /usr/local/lib/rustlib/uninstall.sh? In theory it should not interfere though. Maybe try to remove the build/stage* directories?

@est31

Removal of the lang feature gate tests whitelist rust-lang#39059 r? @est31

@est31

[Gate Tests] - marking feature tests Removal of the lang feature gate tests whitelist #39059 r? @est31

est31 · 2017-01-22T22:36:47Z

Okay, found two further gate tests, so I moved "quote" and "macro_reexport" to the section of features that need only marking. Note that their marking won't be effective until #39247 gets merged.

est31 · 2017-02-26T12:02:06Z

@danobi no, the cfg_target_thread_local feature is distinct from thread_local. They both share tracking issue #29594, but are about distinct things. thread_local is about the #[thread_local] attribute, while cfg_target_thread_local is about the target_thread_local cfg feature. In src/test/run-pass/auxiliary/thread-local-extern-static.rs you can see how both of the features are tested. It should be easy to make that only checks for target_thread_local. If you need any help, just ask!

Add compile fail test for unboxed_closures feature Hello, this is my first contribution to rust. Issue rust-lang#39059.

…arget-has-atomic, r=alexcrichton Add compile test for cfg_target_has_atomic Issue rust-lang#39059. I am concerned about whether the test is excessive.

topecongiro · 2017-03-01T02:51:18Z

Hello. I am working on adding compile fail test for cfg_target_has_atomic(PR #40150) and abi_ptx(will create PR after #40150 got merged).

…tx, r=petrochenkov Add compile fail test for abi_ptx Issue rust-lang#39059.

topecongiro · 2017-03-03T09:51:01Z

Is stmt_expr_attributes still unstable? src/rust/run-pass/cfg_stmt_expr.rs compiles without an error after removing #![feature(stmt_expr_attributes)].

est31 · 2017-03-03T10:07:01Z

@topecongiro I think its partly stable, partly not. See this comment: #15701 (comment)

Relevant for us is the status in src/libsyntax/feature_gate.rs and that says its still unstable. About that test file, I think the #![feature(...)] should be removed from it (and all other test files that still compile with #![feature(...)] included). There are other files that have stmt_expr_attributes, maybe one of them will fail to compile?

topecongiro · 2017-03-03T14:13:10Z

@est31 There are three files that have stmt_expr_attribues under src/test/run-pass, but all of them compile successfully without #![feature(...)].

Maybe a test file like the following is required to test stmt_expr_attributes on expression?

#![feature(stmt_expr_attributes)]
fn main() {
    let x = #[allow(dead_code)] 8;
}

Add compile fail test for SIMD This completes the missing SIMD test task for issue #39059.

est31 · 2017-03-04T02:08:05Z

Maybe a test file like the following is required to test stmt_expr_attributes on expression?

Does it fail when you remove the #![feature(...)]? Then it can be used as gate test. If it doesn't fail then its probably a bug of the code that checks the attributes.

topecongiro · 2017-03-04T02:45:55Z

It does fail after removing the #![feature(...)]. I will create a PR for this.

…=petrochenkov Add compile fail test for stmt_expr_attributes Adds a missing feature gate test for `stmt_expr_attributes`. Issue #39059.

gibfahn · 2017-03-05T19:39:15Z

I took the last two, unwind_attributes and cfg_target_thread_local, and removed the whitelist. I've never contributed to rust before but I think this is right: #40279

Test copied from src/test/codegen/extern-functions.rs. Refs: rust-lang#39059

Test copied from src/test/run-pass/thread-local-extern-static.rs. Refs: rust-lang#39059

est31 mentioned this issue Jan 14, 2017

Tracking issue for Reflect stabilization #27749

Closed

est31 mentioned this issue Jan 14, 2017

Mark safe_suggestion and pushpop_unsafe as removed in feature_gate.rs #39071

Merged

retep998 added the E-help-wanted Call for participation: Help is requested to fix this issue. label Jan 17, 2017

brson added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Jan 18, 2017

cseale mentioned this issue Jan 22, 2017

[Gate Tests] - marking feature tests #39243

Merged

cseale added a commit to cseale/rust that referenced this issue Jan 22, 2017

[Gate Tests] - marking feature tests

ddb6027

Removal of the lang feature gate tests whitelist rust-lang#39059 r? @est31

bors added a commit that referenced this issue Jan 22, 2017

Auto merge of #39243 - cseale:issue_30924, r=est31

a8fa310

[Gate Tests] - marking feature tests Removal of the lang feature gate tests whitelist #39059 r? @est31

est31 mentioned this issue Jan 22, 2017

Remove proc_macro from the tidy whitelist again #39247

Merged

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 28, 2017

Rollup merge of rust-lang#40047 - topecongiro:master, r=est31

c3ae2f8

Add compile fail test for unboxed_closures feature Hello, this is my first contribution to rust. Issue rust-lang#39059.

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 28, 2017

Rollup merge of rust-lang#40047 - topecongiro:master, r=est31

f190779

Add compile fail test for unboxed_closures feature Hello, this is my first contribution to rust. Issue rust-lang#39059.

eddyb added a commit to eddyb/rust that referenced this issue Feb 28, 2017

Rollup merge of rust-lang#40047 - topecongiro:master, r=est31

718882e

Add compile fail test for unboxed_closures feature Hello, this is my first contribution to rust. Issue rust-lang#39059.

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 28, 2017

Rollup merge of rust-lang#40047 - topecongiro:master, r=est31

922a262

Add compile fail test for unboxed_closures feature Hello, this is my first contribution to rust. Issue rust-lang#39059.

topecongiro mentioned this issue Feb 28, 2017

Add compile test for cfg_target_has_atomic #40150

Merged

topecongiro mentioned this issue Mar 1, 2017

Add compile fail test for abi_ptx #40168

Merged

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Mar 2, 2017

Rollup merge of rust-lang#40168 - topecongiro:compile-fail-test-abi-p…

418a776

…tx, r=petrochenkov Add compile fail test for abi_ptx Issue rust-lang#39059.

bors added a commit that referenced this issue Mar 3, 2017

Auto merge of #40101 - danobi:feat_gate_test_simd, r=alexcrichton

b1e3176

Add compile fail test for SIMD This completes the missing SIMD test task for issue #39059.

topecongiro mentioned this issue Mar 4, 2017

Add compile fail test for stmt_expr_attributes #40252

Merged

bors added a commit that referenced this issue Mar 5, 2017

Auto merge of #40252 - topecongiro:compile-fail-test-cfg-stmt-expr, r…

6b76c9e

…=petrochenkov Add compile fail test for stmt_expr_attributes Adds a missing feature gate test for `stmt_expr_attributes`. Issue #39059.

gibfahn mentioned this issue Mar 5, 2017

Add compile-fail tests for remaining items in whitelist and remove it #40279

Merged

gibfahn added a commit to gibfahn/rust that referenced this issue Mar 5, 2017

Add compile-fail test for unwind_attributes

ea3f148

Test copied from src/test/codegen/extern-functions.rs. Refs: rust-lang#39059

gibfahn added a commit to gibfahn/rust that referenced this issue Mar 5, 2017

Add compile-fail test for cfg_target_thread_local

cee4ef8

Test copied from src/test/run-pass/thread-local-extern-static.rs. Refs: rust-lang#39059

bors closed this as completed in 7c88d9e Mar 9, 2017

est31 mentioned this issue Mar 15, 2017

audit: ensure every gated feature has a compile-fail test #22820

Closed

40 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removal of the lang feature gate tests whitelist #39059

Removal of the lang feature gate tests whitelist #39059

est31 commented Jan 14, 2017 •

edited

Loading

brson commented Jan 18, 2017

alanhoff commented Jan 21, 2017

cseale commented Jan 22, 2017

est31 commented Jan 22, 2017

cseale commented Jan 22, 2017

cseale commented Jan 22, 2017 •

edited

Loading

est31 commented Jan 22, 2017

cseale commented Jan 22, 2017

est31 commented Jan 22, 2017

est31 commented Jan 22, 2017

est31 commented Feb 26, 2017

topecongiro commented Mar 1, 2017

topecongiro commented Mar 3, 2017

est31 commented Mar 3, 2017

topecongiro commented Mar 3, 2017

est31 commented Mar 4, 2017

topecongiro commented Mar 4, 2017

gibfahn commented Mar 5, 2017 •

edited

Loading

Removal of the lang feature gate tests whitelist #39059

Removal of the lang feature gate tests whitelist #39059

Comments

est31 commented Jan 14, 2017 • edited Loading

brson commented Jan 18, 2017

alanhoff commented Jan 21, 2017

cseale commented Jan 22, 2017

est31 commented Jan 22, 2017

cseale commented Jan 22, 2017

cseale commented Jan 22, 2017 • edited Loading

est31 commented Jan 22, 2017

cseale commented Jan 22, 2017

est31 commented Jan 22, 2017

est31 commented Jan 22, 2017

est31 commented Feb 26, 2017

topecongiro commented Mar 1, 2017

topecongiro commented Mar 3, 2017

est31 commented Mar 3, 2017

topecongiro commented Mar 3, 2017

est31 commented Mar 4, 2017

topecongiro commented Mar 4, 2017

gibfahn commented Mar 5, 2017 • edited Loading

est31 commented Jan 14, 2017 •

edited

Loading

cseale commented Jan 22, 2017 •

edited

Loading

gibfahn commented Mar 5, 2017 •

edited

Loading