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

Implement RFC 2396: #[target_feature] 1.1 #69274

Merged
merged 4 commits into from
May 2, 2020

Conversation

LeSeulArtichaut
Copy link
Contributor

Tracking issue: #69098

r? @nikomatsakis
cc @gnzlbg @joshtriplett

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 18, 2020
@rust-highfive

This comment has been minimized.

@LeSeulArtichaut

This comment has been minimized.

src/test/ui/rfcs/rfc-2396-target_feature-fn-ptr.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/coercion.rs Outdated Show resolved Hide resolved
src/test/ui/rfcs/rfc-2396-target_feature-safe-calls.rs Outdated Show resolved Hide resolved
src/test/ui/rfcs/rfc-2396-target_feature-safe-calls.rs Outdated Show resolved Hide resolved
src/test/ui/rfcs/rfc-2396-target_feature-safe-calls.rs Outdated Show resolved Hide resolved
src/librustc_mir/transform/check_unsafety.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/coercion.rs Outdated Show resolved Hide resolved
src/librustc/ty/error.rs Outdated Show resolved Hide resolved
src/librustc/ty/error.rs Outdated Show resolved Hide resolved
src/librustc_typeck/collect.rs Show resolved Hide resolved
@LeSeulArtichaut LeSeulArtichaut force-pushed the target-feature-11 branch 2 times, most recently from 46826ec to 76e4ad7 Compare February 20, 2020 21:43
@rust-highfive

This comment has been minimized.

@LeSeulArtichaut LeSeulArtichaut force-pushed the target-feature-11 branch 2 times, most recently from 2321921 to d4c853d Compare February 21, 2020 12:03
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-02-21T12:03:40.4627422Z ========================== Starting Command Output ===========================
2020-02-21T12:03:40.4630905Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/bcb38c67-087c-413f-91c0-9ee2644c3a2c.sh
2020-02-21T12:03:40.4631115Z 
2020-02-21T12:03:40.4634551Z ##[section]Finishing: Disable git automatic line ending conversion
2020-02-21T12:03:40.4653435Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/69274/merge to s
2020-02-21T12:03:40.4657207Z Task         : Get sources
2020-02-21T12:03:40.4657483Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-21T12:03:40.4657803Z Version      : 1.0.0
2020-02-21T12:03:40.4658297Z Author       : Microsoft
---
2020-02-21T12:03:41.4922609Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-02-21T12:03:41.4927031Z ##[command]git config gc.auto 0
2020-02-21T12:03:41.4930103Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-02-21T12:03:41.4932869Z ##[command]git config --get-all http.proxy
2020-02-21T12:03:41.4938957Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/69274/merge:refs/remotes/pull/69274/merge
---
2020-02-21T12:09:43.0071004Z     Finished release [optimized] target(s) in 1m 25s
2020-02-21T12:09:43.0149667Z tidy check
2020-02-21T12:09:44.2451364Z * 589 error codes
2020-02-21T12:09:44.2452664Z * highest error code: E0746
2020-02-21T12:09:44.5743396Z Expected a gate test for the feature 'target_feature_11'.
2020-02-21T12:09:44.5744634Z Hint: create a failing test file named 'feature-gate-target_feature_11.rs'
2020-02-21T12:09:44.5745436Z       in the 'ui' test suite, with its failures due to
2020-02-21T12:09:44.5745960Z       missing usage of `#![feature(target_feature_11)]`.
2020-02-21T12:09:44.5746897Z Hint: If you already have such a test and don't want to rename it,
2020-02-21T12:09:44.5747679Z       you can also add a // gate-test-target_feature_11 line to the test file.
2020-02-21T12:09:44.5748225Z tidy error: Found 1 features without a gate test.
2020-02-21T12:09:45.3801915Z some tidy checks failed
2020-02-21T12:09:45.3802146Z Found 487 error codes
2020-02-21T12:09:45.3802337Z Found 0 error codes with no tests
2020-02-21T12:09:45.3802695Z Done!
2020-02-21T12:09:45.3802695Z Done!
2020-02-21T12:09:45.3802786Z 
2020-02-21T12:09:45.3802867Z 
2020-02-21T12:09:45.3804187Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor"
2020-02-21T12:09:45.3804864Z 
2020-02-21T12:09:45.3804947Z 
2020-02-21T12:09:45.3805191Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2020-02-21T12:09:45.3805507Z Build completed unsuccessfully in 0:01:34
2020-02-21T12:09:45.3805507Z Build completed unsuccessfully in 0:01:34
2020-02-21T12:09:45.3847645Z == clock drift check ==
2020-02-21T12:09:45.3857462Z   local time: Fri Feb 21 12:09:45 UTC 2020
2020-02-21T12:09:45.6764879Z   network time: Fri, 21 Feb 2020 12:09:45 GMT
2020-02-21T12:09:45.6767972Z == end clock drift check ==
2020-02-21T12:09:47.0528957Z 
2020-02-21T12:09:47.0566489Z ##[error]Bash exited with code '1'.
2020-02-21T12:09:47.0583570Z ##[section]Finishing: Run build
2020-02-21T12:09:47.0628264Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/69274/merge to s
2020-02-21T12:09:47.0632637Z Task         : Get sources
2020-02-21T12:09:47.0632952Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-21T12:09:47.0633241Z Version      : 1.0.0
2020-02-21T12:09:47.0633458Z Author       : Microsoft
2020-02-21T12:09:47.0633458Z Author       : Microsoft
2020-02-21T12:09:47.0633953Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-02-21T12:09:47.0634328Z ==============================================================================
2020-02-21T12:09:47.3684352Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-02-21T12:09:47.3731065Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/69274/merge to s
2020-02-21T12:09:47.3819974Z Cleaning up task key
2020-02-21T12:09:47.3821380Z Start cleaning up orphan processes.
2020-02-21T12:09:47.3987819Z Terminate orphan process: pid (4317) (python)
2020-02-21T12:09:47.4134502Z ##[section]Finishing: Finalize Job

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@LeSeulArtichaut

This comment has been minimized.

src/librustc/ty/error.rs Outdated Show resolved Hide resolved
src/librustc/ty/error.rs Outdated Show resolved Hide resolved
src/test/ui/rfcs/rfc-2396-target_feature-11/fn-ptr.rs Outdated Show resolved Hide resolved
src/test/ui/rfcs/rfc-2396-target_feature-11/safe-calls.rs Outdated Show resolved Hide resolved
@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@LeSeulArtichaut LeSeulArtichaut force-pushed the target-feature-11 branch 2 times, most recently from d198a59 to 4bf48a5 Compare February 22, 2020 12:07
@rust-highfive

This comment has been minimized.

@LeSeulArtichaut LeSeulArtichaut force-pushed the target-feature-11 branch 2 times, most recently from 53d066f to 7882c32 Compare May 1, 2020 15:07
@hanna-kruppe
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 2, 2020

📌 Commit 8d9f73a has been approved by hanna-kruppe

@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 May 2, 2020
@petrochenkov
Copy link
Contributor

Do "target-feature" functions implement the Fn() family of traits?

If they do, then it looks like unsoundness because the unsafety check is performed before monomorphization.

If they don't, then it means that target-feature functions are still unsafe from the type system point of view, it's just hidden syntactically (and special-cased in the unsafety checker), so the special coercion restriction is not necessary.

@hanna-kruppe
Copy link
Contributor

I think you're right, this PR doesn't change that safe function items implement Fn* traits and this should lead to unsoundness. However, this is a concern about the contents of the RFC and belongs on the tracking issue.

I'd prefer to merge this PR regardless, the changes implemented here will probably be unaffected by whatever we end up doing to solve this soundness problem, and having 90% of an implementation in-tree is better than letting it bit-rot until we figure out the remaining 10%.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 2, 2020
…r=hanna-kruppe

Implement RFC 2396: `#[target_feature]` 1.1

Tracking issue: rust-lang#69098

r? @nikomatsakis
cc @gnzlbg @joshtriplett
@bors
Copy link
Contributor

bors commented May 2, 2020

⌛ Testing commit 8d9f73a with merge f05a524...

@bors
Copy link
Contributor

bors commented May 2, 2020

☀️ Test successful - checks-azure
Approved by: hanna-kruppe
Pushing f05a524 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 2, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2020
Rollup of 7 pull requests

Successful merges:

 - rust-lang#69274 (Implement RFC 2396: `#[target_feature]` 1.1)
 - rust-lang#71767 (doc: make Stack and StackElement a little pretty)
 - rust-lang#71772 (Mark query function as must_use.)
 - rust-lang#71777 (cleanup: `config::CrateType` -> `CrateType`)
 - rust-lang#71784 (Remove recommendation for unmaintained dirs crate)
 - rust-lang#71785 (Update comment regarding SO_REUSEADDR on Windows)
 - rust-lang#71787 (fix rustdoc warnings)

Failed merges:

r? @ghost
@bors bors merged commit f05a524 into rust-lang:master May 2, 2020
@LeSeulArtichaut LeSeulArtichaut deleted the target-feature-11 branch May 3, 2020 07:36
@LeSeulArtichaut LeSeulArtichaut added the F-target_feature_11 target feature 1.1 RFC label Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-target_feature_11 target feature 1.1 RFC 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.

10 participants