Implementation of incompatible features error #76293

Amjad50 · 2020-09-03T17:51:46Z

Proposal of a new error: Incompatible features

This error should happen if two features which are not compatible are used together.

For now the only incompatible features are const_generics and min_const_generics

fixes #76280

rust-highfive · 2020-09-03T17:51:50Z

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

jonas-schievink · 2020-09-03T17:54:44Z

r? @lcnr

lcnr

This is a good idea 👍

I think it would be better if this is a hard error instead, considering that there really shouldn't be a reason to opt into this. Afaik we also would have to be more reluctant if we were to ever remove this lint in the future, as users can reference it, for example by using #![allow(incomplete_features)].

compiler/rustc_lint/src/builtin.rs

lcnr · 2020-09-03T18:34:31Z

compiler/rustc_lint/src/builtin.rs

+                        cx.struct_span_lint(INCOMPATIBLE_FEATURES, spans.clone(), |lint| {
+                            lint.build(&format!(
+                                "features `{}` and `{}` are incompatible, using them \
+                                at the same time can result in an undefined behavior",


Suggested change

at the same time can result in an undefined behavior",

at the same time can result in undefined behavior",

Undefined behavior has a very specific meaning: the compiler is allowed to do anything to your code, up to and including segfaults, security vulnerabilities, and just giving an error. Is that's what's intended here? Or is this just saying 'rustc could be buggy'? I think we should distinguish between compiler bugs and behavior that is intended to be undefined.

cc @RalfJung

Its not that 'rustc is buggy', to fix the issue we might have checked everytime min_const_generics is used that const_generics is not present, but I thought of making this general in case in the future we encountered something similar.

I intended to produce an error so that users cannot use them together, It thought rustc_lint was the correct place.

Ok, in that case let's just say 'not allowed'. UB has other connotations which aren't applicable here.

I think it's better to produce an error instead of the current warning. Do you know how to do that?

UB would be accurate here if there are known soundness bugs -- like there are for incomplete features, though I think there I made the text say something like "unsafe to use" or so. I think it is important to drive home the point that even if compilation succeeds (no ICE or so), things could still go horribly wrong.

Oh, but this seems different. We just want people to disable min_const_generic when enabling const_generic. And this is a hard error, not a warning. So yeah, it probably doesn't make sense to speak about UB here.

yah, working on making it hard error

Should I still do it in rustc_lint? or which part of rustc is better for this?

I think rustc_ast_passes/feature_gate.rs might be fitting, not too sure about that though

compiler/rustc_lint/src/builtin.rs

compiler/rustc_feature/src/active.rs

Amjad50 · 2020-09-03T22:22:44Z

I also agree making it a hard error is better it will also have a better meaning for 'undefined behavior', but I don't know which rustc part is responsible for this, after some search I found rustc_lint so I thought it might work.

I'm ready to change it into someplace else.

Amjad50 · 2020-09-04T10:35:00Z

moved it to rustc_ast_passes/feature_gate.rs, but can be moved elsewhere if not appropriate. Now it produce hard error with message not allowed

lcnr

A few optional nits about style, otherwise r=me

compiler/rustc_ast_passes/src/feature_gate.rs

If two features are defined as incompatible, using them together would result in an error

lcnr · 2020-09-04T14:26:21Z

@bors r+ rollup

Thank you ❤️

bors · 2020-09-04T14:26:22Z

📌 Commit 8f2d906 has been approved by lcnr

…, r=lcnr Implementation of incompatible features error Proposal of a new error: Incompatible features This error should happen if two features which are not compatible are used together. For now the only incompatible features are `const_generics` and `min_const_generics` fixes rust-lang#76280

@ghost

Rollup of 18 pull requests Successful merges: - rust-lang#76273 (Move some Vec UI tests into alloc unit tests) - rust-lang#76274 (Allow try blocks as the argument to return expressions) - rust-lang#76287 (Remove an unnecessary allowed lint) - rust-lang#76293 (Implementation of incompatible features error) - rust-lang#76299 (Make `Ipv4Addr` and `Ipv6Addr` const tests unit tests under `library`) - rust-lang#76302 (Address review comments on `Peekable::next_if`) - rust-lang#76303 (Link to `#capacity-and-reallocation` when using with_capacity) - rust-lang#76305 (Move various ui const tests to `library`) - rust-lang#76309 (Indent a note to make folding work nicer) - rust-lang#76312 (time.rs: Make spelling of "Darwin" consistent) - rust-lang#76318 (Use ops::ControlFlow in rustc_data_structures::graph::iterate) - rust-lang#76324 (Move Vec slice UI tests in library) - rust-lang#76338 (add some intra-doc links to `Iterator`) - rust-lang#76340 (Remove unused duplicated `trivial_dropck_outlives`) - rust-lang#76344 (Improve docs for `std::env::args()`) - rust-lang#76346 (Docs: nlink example typo) - rust-lang#76358 (Minor grammar fix in doc comment for soft-deprecated methods) - rust-lang#76364 (Disable atomics on avr target.) Failed merges: - rust-lang#76304 (Make delegation methods of `std::net::IpAddr` unstably const) r? @ghost

rust-highfive assigned nikomatsakis Sep 3, 2020

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

rust-highfive assigned lcnr and unassigned nikomatsakis Sep 3, 2020

lcnr reviewed Sep 3, 2020

View reviewed changes

compiler/rustc_lint/src/builtin.rs Outdated Show resolved Hide resolved

lcnr reviewed Sep 3, 2020

View reviewed changes

jyn514 added the requires-nightly This issue requires a nightly compiler in some way. label Sep 3, 2020

jyn514 reviewed Sep 3, 2020

View reviewed changes

compiler/rustc_lint/src/builtin.rs Outdated Show resolved Hide resolved

compiler/rustc_feature/src/active.rs Outdated Show resolved Hide resolved

Amjad50 force-pushed the incompatible_features_error branch from fe85606 to 50a31b4 Compare September 4, 2020 10:31

lcnr approved these changes Sep 4, 2020

View reviewed changes

compiler/rustc_ast_passes/src/feature_gate.rs Outdated Show resolved Hide resolved

compiler/rustc_ast_passes/src/feature_gate.rs Outdated Show resolved Hide resolved

Amjad50 force-pushed the incompatible_features_error branch from 50a31b4 to cd8fbfb Compare September 4, 2020 14:01

Implementation of incompatible features error

8f2d906

If two features are defined as incompatible, using them together would result in an error

Amjad50 force-pushed the incompatible_features_error branch from cd8fbfb to 8f2d906 Compare September 4, 2020 14:17

Amjad50 requested a review from jyn514 September 4, 2020 14:19

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 Sep 4, 2020

Dylan-DPC-zz mentioned this pull request Sep 5, 2020

Rollup of 11 pull requests #76377

Closed

This was referenced Sep 6, 2020

Rollup of 18 pull requests #76414

Closed

Rollup of 18 pull requests #76422

Merged

bors merged commit 3d834bc into rust-lang:master Sep 7, 2020

rustbot added this to the 1.48.0 milestone Sep 7, 2020

lcnr added the F-const_generics `#![feature(const_generics)]` label Nov 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of incompatible features error #76293

Implementation of incompatible features error #76293

Amjad50 commented Sep 3, 2020

rust-highfive commented Sep 3, 2020

jonas-schievink commented Sep 3, 2020

lcnr left a comment •

edited

Loading

lcnr Sep 3, 2020

jyn514 Sep 3, 2020

Amjad50 Sep 3, 2020

jyn514 Sep 3, 2020

Amjad50 Sep 4, 2020

RalfJung Sep 4, 2020 •

edited

Loading

RalfJung Sep 4, 2020

Amjad50 Sep 4, 2020

Amjad50 Sep 4, 2020

lcnr Sep 4, 2020

Amjad50 commented Sep 3, 2020

Amjad50 commented Sep 4, 2020

lcnr left a comment

lcnr commented Sep 4, 2020

bors commented Sep 4, 2020

	at the same time can result in an undefined behavior",
	at the same time can result in undefined behavior",

Implementation of incompatible features error #76293

Implementation of incompatible features error #76293

Conversation

Amjad50 commented Sep 3, 2020

rust-highfive commented Sep 3, 2020

jonas-schievink commented Sep 3, 2020

lcnr left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung Sep 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amjad50 commented Sep 3, 2020

Amjad50 commented Sep 4, 2020

lcnr left a comment

Choose a reason for hiding this comment

lcnr commented Sep 4, 2020

bors commented Sep 4, 2020

lcnr left a comment •

edited

Loading

RalfJung Sep 4, 2020 •

edited

Loading