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

Surprising missing stability attribute error #84935

Open
illicitonion opened this issue May 5, 2021 · 2 comments
Open

Surprising missing stability attribute error #84935

illicitonion opened this issue May 5, 2021 · 2 comments
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-stability Area: `#[stable]`, `#[unstable]` etc. C-bug Category: This is a bug.

Comments

@illicitonion
Copy link

On master (as of 716394d) I can currently successfully compile stage1 std by running ./x.py build --stage 1.

Before #81837 merged, applying the following patch would still allow me to successfully compile stage1 std:

diff --git a/compiler/rustc_builtin_macros/src/lib.rs b/compiler/rustc_builtin_macros/src/lib.rs
index d7926ed0e0b..95f398aee1e 100644
--- a/compiler/rustc_builtin_macros/src/lib.rs
+++ b/compiler/rustc_builtin_macros/src/lib.rs
@@ -101,6 +101,7 @@ pub fn register_builtin_macros(resolver: &mut dyn ResolverExpand) {
 
     register_derive! {
         Clone: clone::expand_deriving_clone,
+        DWH: clone::expand_deriving_clone,
         Copy: bounds::expand_deriving_copy,
         Debug: debug::expand_deriving_debug,
         Default: default::expand_deriving_default,
diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
index b2dac10c83f..47cf56cf7af 100644
--- a/compiler/rustc_span/src/symbol.rs
+++ b/compiler/rustc_span/src/symbol.rs
@@ -136,6 +136,7 @@
         Clone,
         Copy,
         Count,
+        DWH,
         Debug,
         DebugStruct,
         DebugTuple,
diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs
index 6032dc9a2d3..3cdf570e600 100644
--- a/library/core/src/num/mod.rs
+++ b/library/core/src/num/mod.rs
@@ -39,6 +39,19 @@
 mod nonzero;
 mod wrapping;
 
+#[unstable(feature = "dwh", issue = "none")]
+/// Docs.
+pub mod enums {
+    /// Docs.
+    #[cfg(not(bootstrap))]
+    #[rustc_builtin_macro]
+    #[unstable(feature = "dwh", issue = "none")]
+    #[allow_internal_unstable(core_intrinsics)]
+    pub macro DWH($item:item) {
+    /* compiler built-in */
+    }
+}
+
 #[stable(feature = "rust1", since = "1.0.0")]
 pub use wrapping::Wrapping;

After #81837, which added a non-pub const to that module, applying the above patch gives me the following error:

error: constant has missing stability attribute
   --> library/core/src/num/mod.rs:169:1
    |
170 | const ASCII_CASE_MASK: u8 = 0b0010_0000;
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

error: could not compile `core`

If I revert #81837 the error goes away.

This is surprising - I wouldn't expect the presence of an unrelated, but properly marked unstable, macro in a module to change whether a const needs to be marked stable. I'm not sure what's wrong here - maybe the const does need a stability attribute and a bug means that wasn't flagged, or maybe the lint checking for missing stability attributes is getting tripped up somehow, but something is wrong...

This reliably reproduces whether the added module is inline or a separate file. It does not reproduce if the macro isn't present in the module (e.g. it doesn't reproduce for an empty module).

@illicitonion illicitonion added the C-bug Category: This is a bug. label May 5, 2021
@jyn514
Copy link
Member

jyn514 commented May 5, 2021

maybe the lint checking for missing stability attributes is getting tripped up somehow, but something is wrong...

This seems more likely, stability attributes are only needed for public items.

@illicitonion
Copy link
Author

I did some digging - it looks like this is intentional. #63114 introduced an expectation that everything in a module which transitively contains a macro is reachable from that macro, and that reachability check is used to determine what needs stability attributes.

@matthewjasper I'm a little confused about why the macro core::clone::Clone doesn't care that core::num::ASCII_CASE_MASK doesn't have a stability attribute, but core::num::enums::SomeMacro would care - the original PR message says:

Mark private items reachable from the root of libcore as unstable - they are now reachable (in principle) in other crates via macros in libcore

which would suggest that core::clone::Clone should care.

It feels like there's definitely a bug here, and it feels like core::num:: ASCII_CASE_MASK probably should have a stability attribute based on the reasoning of that PR, but it feels like there's also a second bug that the checking for this only does an upward traversal of scope which can early-return - the way I read the above quote, the presence of any macro in core should require that every symbol in core should have a stability attribute?

@jyn514 jyn514 added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-stability Area: `#[stable]`, `#[unstable]` etc. labels Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-stability Area: `#[stable]`, `#[unstable]` etc. C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

2 participants