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

Re-add most early unstable syntax gates as future-compat lints #535

Closed
2 of 3 tasks
CAD97 opened this issue Jul 27, 2022 · 4 comments
Closed
2 of 3 tasks

Re-add most early unstable syntax gates as future-compat lints #535

CAD97 opened this issue Jul 27, 2022 · 4 comments
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team

Comments

@CAD97
Copy link

CAD97 commented Jul 27, 2022

(This has both T-lang and T-compiler implications; if this would be better as a T-lang proposal it can be moved. However, it is my general understanding that the lack of these gates is considered a bug and not a stable language guarantee.)

Proposal

Many unstable syntaxes (e.g. box $expr, macro, try) are currently considered syntax-stable (playground) and can be present in syntax-checked-only contexts (i.e. passed to active/noninert attributes that don't reemit the syntax e.g. #[cfg(FALSE)]) despite being unstable.

Early gates were added for most unstable syntaxes in rust-lang/rust#65742, but were reverted in rust-lang/rust#66004 due to this (intentionally) regressing the ability to #[cfg]-gate unstable syntax (originally reported in rust-lang/rust$65860).

This proposal is to reintroduce these early gates, but as future-compatibility warning lints rather than hard errors, in order to warn against potential breakage from adjustments of unstable syntax (and discourage attribute proc macros from relying on the syntax availability) without just outright removing the ability to cfg-gate unstable syntax without indirection to a separate file which is cfg-gate-include!d.

Alternatively, a separate future-compatibility-like lint group/mechanism can be introduced for unstable syntax to refine future-compatibility's

warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

to something potentially more appropriate along the lines of

warning: this is unstable syntax provisionally accepted by the compiler; it may be changed or removed causing a hard error in a future release!

The emission of the early lint should be suppressed if the same span later emits an error for the use of the unstable feature.

Future Work

This is not part of this MCP, but included for illustration of a potential direction.

Discussed in rust-lang/rust#65860 is a potential compromise: only syntax-verify "one level deep" in the token tree. If this change is done, it should be carefully considered what this loses (e.g. rustc checked known syntax-correctness for external tooling such as rustfmt to be able to syntactically analyze block bodies without expanding attribute macros).

Possible directions to allow cfg-gating unstable syntax include shallow syntax verification for some or all language constructs with an active attribute or introduction of a std-blessed1 cfg_if style bang macro, as bang macros do allow arbitrary internal syntax.

Mentors or Reviewers

No mentor/reviewer has been suggested. I (@CAD97) am willing/able/available to do the work necessary to cherry-pick the previous early syntax gate implementation and modify it to emit a future-compatility lint instead of a hard error. (This has not been done yet, but I plan to do it shortly following a second, if given.)

The work to actually early-gate the features has previously been done and reviewed. The new work needing mentor/review is primarily just diagnostics (e.g. communicating the unstable-in-stable status and suppressing the warning if an error is also emitted).

References

Process

The main points of the Major Change Process are as follows:

  • File an issue describing the proposal.
  • A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Footnotes

  1. The advantage of being std-blessed is that tooling (such as rustfmt) can presume that the macro is the std one and process the internal syntax (potentially even without performing name resolution) such as is done for format_args!-like macros.

@CAD97 CAD97 added major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team labels Jul 27, 2022
@rustbot
Copy link
Collaborator

rustbot commented Jul 27, 2022

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

@rustbot rustbot added the to-announce Announce this issue on triage meeting label Jul 27, 2022
@petrochenkov
Copy link

@rustbot second

@rustbot rustbot added the final-comment-period The FCP has started, most (if not all) team members are in agreement label Jul 27, 2022
@CAD97
Copy link
Author

CAD97 commented Jul 30, 2022

Initial implementation is available: rust-lang/rust#99935

@apiraino
Copy link
Contributor

@rustbot label -final-comment-period +major-change-accepted

@rustbot rustbot added major-change-accepted A major change proposal that was accepted and removed final-comment-period The FCP has started, most (if not all) team members are in agreement labels Aug 10, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Aug 20, 2022
…ochenkov

Reenable disabled early syntax gates as future-incompatibility lints

- MCP: rust-lang/compiler-team#535

The approach taken by this PR is

- Introduce a new lint, `unstable_syntax_pre_expansion`, and reenable the early syntax gates to emit it
- Use the diagnostic stashing mechanism to stash warnings the early warnings
- When the hard error occurs post expansion, steal and cancel the early warning
- Don't display any stashed warnings if errors are present to avoid the same noise problem that hiding type ascription errors is avoiding

Commits are working commits, but in a coherent steps-to-implement manner. Can be squashed if desired.

The preexisting `soft_unstable` lint seems like it would've been a good fit, but it is deny-by-default (appropriate for `#[bench]`) and these gates should be introduced as warn-by-default.

It may be desirable to change the stash mechanism's behavior to not flush lint errors in the presence of other errors either (like is done for warnings here), but upgrading a stash-using lint from warn to error perhaps is enough of a request to see the lint that they shouldn't be hidden; additionally, fixing the last error to get new errors thrown at you always feels bad, so if we know the lint errors are present, we should show them.

Using a new flag/mechanism for a "weak diagnostic" which is suppressed by other errors may also be desirable over assuming any stashed warnings are "weak," but this is the first user of stashing warnings and seems an appropriate use of stashing (it follows the "know more later to refine the diagnostic" pattern; here we learn that it's in a compiled position) so we get to define what it means to stash a non-hard-error diagnostic.

cc `@petrochenkov` (seconded MCP)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 20, 2022
…ochenkov

Reenable disabled early syntax gates as future-incompatibility lints

- MCP: rust-lang/compiler-team#535

The approach taken by this PR is

- Introduce a new lint, `unstable_syntax_pre_expansion`, and reenable the early syntax gates to emit it
- Use the diagnostic stashing mechanism to stash warnings the early warnings
- When the hard error occurs post expansion, steal and cancel the early warning
- Don't display any stashed warnings if errors are present to avoid the same noise problem that hiding type ascription errors is avoiding

Commits are working commits, but in a coherent steps-to-implement manner. Can be squashed if desired.

The preexisting `soft_unstable` lint seems like it would've been a good fit, but it is deny-by-default (appropriate for `#[bench]`) and these gates should be introduced as warn-by-default.

It may be desirable to change the stash mechanism's behavior to not flush lint errors in the presence of other errors either (like is done for warnings here), but upgrading a stash-using lint from warn to error perhaps is enough of a request to see the lint that they shouldn't be hidden; additionally, fixing the last error to get new errors thrown at you always feels bad, so if we know the lint errors are present, we should show them.

Using a new flag/mechanism for a "weak diagnostic" which is suppressed by other errors may also be desirable over assuming any stashed warnings are "weak," but this is the first user of stashing warnings and seems an appropriate use of stashing (it follows the "know more later to refine the diagnostic" pattern; here we learn that it's in a compiled position) so we get to define what it means to stash a non-hard-error diagnostic.

cc ``@petrochenkov`` (seconded MCP)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 20, 2022
…ochenkov

Reenable disabled early syntax gates as future-incompatibility lints

- MCP: rust-lang/compiler-team#535

The approach taken by this PR is

- Introduce a new lint, `unstable_syntax_pre_expansion`, and reenable the early syntax gates to emit it
- Use the diagnostic stashing mechanism to stash warnings the early warnings
- When the hard error occurs post expansion, steal and cancel the early warning
- Don't display any stashed warnings if errors are present to avoid the same noise problem that hiding type ascription errors is avoiding

Commits are working commits, but in a coherent steps-to-implement manner. Can be squashed if desired.

The preexisting `soft_unstable` lint seems like it would've been a good fit, but it is deny-by-default (appropriate for `#[bench]`) and these gates should be introduced as warn-by-default.

It may be desirable to change the stash mechanism's behavior to not flush lint errors in the presence of other errors either (like is done for warnings here), but upgrading a stash-using lint from warn to error perhaps is enough of a request to see the lint that they shouldn't be hidden; additionally, fixing the last error to get new errors thrown at you always feels bad, so if we know the lint errors are present, we should show them.

Using a new flag/mechanism for a "weak diagnostic" which is suppressed by other errors may also be desirable over assuming any stashed warnings are "weak," but this is the first user of stashing warnings and seems an appropriate use of stashing (it follows the "know more later to refine the diagnostic" pattern; here we learn that it's in a compiled position) so we get to define what it means to stash a non-hard-error diagnostic.

cc ```@petrochenkov``` (seconded MCP)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 20, 2022
…ochenkov

Reenable disabled early syntax gates as future-incompatibility lints

- MCP: rust-lang/compiler-team#535

The approach taken by this PR is

- Introduce a new lint, `unstable_syntax_pre_expansion`, and reenable the early syntax gates to emit it
- Use the diagnostic stashing mechanism to stash warnings the early warnings
- When the hard error occurs post expansion, steal and cancel the early warning
- Don't display any stashed warnings if errors are present to avoid the same noise problem that hiding type ascription errors is avoiding

Commits are working commits, but in a coherent steps-to-implement manner. Can be squashed if desired.

The preexisting `soft_unstable` lint seems like it would've been a good fit, but it is deny-by-default (appropriate for `#[bench]`) and these gates should be introduced as warn-by-default.

It may be desirable to change the stash mechanism's behavior to not flush lint errors in the presence of other errors either (like is done for warnings here), but upgrading a stash-using lint from warn to error perhaps is enough of a request to see the lint that they shouldn't be hidden; additionally, fixing the last error to get new errors thrown at you always feels bad, so if we know the lint errors are present, we should show them.

Using a new flag/mechanism for a "weak diagnostic" which is suppressed by other errors may also be desirable over assuming any stashed warnings are "weak," but this is the first user of stashing warnings and seems an appropriate use of stashing (it follows the "know more later to refine the diagnostic" pattern; here we learn that it's in a compiled position) so we get to define what it means to stash a non-hard-error diagnostic.

cc ````@petrochenkov```` (seconded MCP)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 20, 2022
…ochenkov

Reenable disabled early syntax gates as future-incompatibility lints

- MCP: rust-lang/compiler-team#535

The approach taken by this PR is

- Introduce a new lint, `unstable_syntax_pre_expansion`, and reenable the early syntax gates to emit it
- Use the diagnostic stashing mechanism to stash warnings the early warnings
- When the hard error occurs post expansion, steal and cancel the early warning
- Don't display any stashed warnings if errors are present to avoid the same noise problem that hiding type ascription errors is avoiding

Commits are working commits, but in a coherent steps-to-implement manner. Can be squashed if desired.

The preexisting `soft_unstable` lint seems like it would've been a good fit, but it is deny-by-default (appropriate for `#[bench]`) and these gates should be introduced as warn-by-default.

It may be desirable to change the stash mechanism's behavior to not flush lint errors in the presence of other errors either (like is done for warnings here), but upgrading a stash-using lint from warn to error perhaps is enough of a request to see the lint that they shouldn't be hidden; additionally, fixing the last error to get new errors thrown at you always feels bad, so if we know the lint errors are present, we should show them.

Using a new flag/mechanism for a "weak diagnostic" which is suppressed by other errors may also be desirable over assuming any stashed warnings are "weak," but this is the first user of stashing warnings and seems an appropriate use of stashing (it follows the "know more later to refine the diagnostic" pattern; here we learn that it's in a compiled position) so we get to define what it means to stash a non-hard-error diagnostic.

cc `````@petrochenkov````` (seconded MCP)
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team
Projects
None yet
Development

No branches or pull requests

4 participants