Auto merge of #151232 - fmease:rustdoc-dont-eval-assoc-consts, r=yotamofek

rustdoc: Stop unconditionally evaluating the initializer of associated consts

See the descriptions of the added tests for details.

Fixes #131625.
Fixes [after beta-1.93 backport] #149635.
Fixes #150312.

Supersedes #150629 IINM.

CC @cuviper (#149635 (comment))
r? @GuillaumeGomez or @yotamofek (#150629)

Author: bors

src/librustdoc/html/render/mod.rs

@@ -1050,14 +1050,11 @@ fn assoc_const(
             ty = print_type(ty, cx),
         )?;
         if let AssocConstValue::TraitDefault(konst) | AssocConstValue::Impl(konst) = value {
-            // FIXME: `.value()` uses `clean::utils::format_integer_with_underscore_sep` under the
-            //        hood which adds noisy underscores and a type suffix to number literals.
-            //        This hurts readability in this context especially when more complex expressions
-            //        are involved and it doesn't add much of value.
-            //        Find a way to print constants here without all that jazz.
-            let repr = konst.value(tcx).unwrap_or_else(|| konst.expr(tcx));
+            let repr = konst.expr(tcx);
             if match value {
                 AssocConstValue::TraitDefault(_) => true, // always show
+                // FIXME: Comparing against the special string "_" denoting overly complex const exprs
+                //        is rather hacky; `ConstKind::expr` should have a richer return type.
                 AssocConstValue::Impl(_) => repr != "_", // show if there is a meaningful value to show
                 AssocConstValue::None => unreachable!(),
             } {

tests/rustdoc-html/anchors/anchors.no_const_anchor2.html

@@ -1 +1 @@
-<section id="associatedconstant.X" class="associatedconstant"><a class="src rightside" href="../src/foo/anchors.rs.html#42">Source</a><h4 class="code-header">pub const <a href="#associatedconstant.X" class="constant">X</a>: <a class="primitive" href="{{channel}}/std/primitive.i32.html">i32</a> = 0i32</h4></section>
+<section id="associatedconstant.X" class="associatedconstant"><a class="src rightside" href="../src/foo/anchors.rs.html#42">Source</a><h4 class="code-header">pub const <a href="#associatedconstant.X" class="constant">X</a>: <a class="primitive" href="{{channel}}/std/primitive.i32.html">i32</a> = 0</h4></section>

tests/rustdoc-html/attributes.rs

@@ -59,7 +59,7 @@ pub enum Enum {
 pub trait Trait {
     //@ has 'foo/trait.Trait.html'
     //@ has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]/*[@class="code-attribute"]' '#[unsafe(link_section = "bar")]'
-    //@ has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]' 'const BAR: u32 = 0u32'
+    //@ has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]' 'const BAR: u32 = 0'
     #[unsafe(link_section = "bar")]
     const BAR: u32 = 0;
 

tests/rustdoc-html/constant/assoc-const-has-projection-ty.rs

@@ -0,0 +1,31 @@
+// Ensure that we properly print the value `1` as `1` in the initializer of associated constants
+// that have user type "projection".
+//
+// We once used to evaluate the initializer in rustdoc and use rustc's MIR pretty-printer to
+// render the resulting MIR const value. This pretty printer matches on the type to interpret
+// the data and falls back to a cryptic `"{transmute(0x$data): $ty}"` for types it can't handle.
+// Crucially, when constructing the MIR const we passed the unnormalized type of the initializer,
+// i.e., the projection `<Struct as Trait>::Ty` instead of the normalized `u32` which the
+// pretty printer obviously can't handle.
+//
+// Now we no longer evaluate it and use a custom printer for the const expr.
+//
+// issue: <https://github.com/rust-lang/rust/issues/150312>
+
+#![crate_name = "it"]
+
+pub trait Trait {
+    type Ty;
+
+    const CT: Self::Ty;
+}
+
+pub struct Struct;
+
+impl Trait for Struct {
+    type Ty = u32;
+
+    //@ has it/struct.Struct.html
+    //@ has - '//*[@id="associatedconstant.CT"]' 'const CT: Self::Ty = 1'
+    const CT: Self::Ty = 1;
+}

tests/rustdoc-html/constant/assoc-consts.rs

@@ -1,6 +1,6 @@
 pub trait Foo {
     //@ has assoc_consts/trait.Foo.html '//pre[@class="rust item-decl"]' \
-    //      'const FOO: usize = 13usize;'
+    //      'const FOO: usize = _;'
     //@ has - '//*[@id="associatedconstant.FOO"]' 'const FOO: usize'
     const FOO: usize = 12 + 1;
     //@ has - '//*[@id="associatedconstant.FOO_NO_DEFAULT"]' 'const FOO_NO_DEFAULT: bool'

tests/rustdoc-html/deref/deref-to-primitive.rs

@@ -3,7 +3,7 @@
 //@ has 'foo/struct.Foo.html'
 //@ has - '//*[@id="deref-methods-i32"]' 'Methods from Deref<Target = i32>'
 //@ has - '//*[@id="deref-methods-i32-1"]//*[@id="associatedconstant.BITS"]/h4' \
-//        'pub const BITS: u32 = 32u32'
+//        'pub const BITS: u32 = u32::BITS'
 pub struct Foo(i32);
 
 impl std::ops::Deref for Foo {

tests/rustdoc-html/display-hidden-items.rs

@@ -20,7 +20,7 @@ pub trait TraitHidden {}
 //@ has 'foo/index.html' '//dt/a[@class="trait"]' 'Trait'
 pub trait Trait {
     //@ has 'foo/trait.Trait.html'
-    //@ has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]' '#[doc(hidden)] const BAR: u32 = 0u32'
+    //@ has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]' '#[doc(hidden)] const BAR: u32 = 0'
     #[doc(hidden)]
     const BAR: u32 = 0;
 
@@ -44,7 +44,7 @@ impl Struct {
 }
 
 impl Trait for Struct {
-    //@ has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]' '#[doc(hidden)] const BAR: u32 = 0u32'
+    //@ has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]' '#[doc(hidden)] const BAR: u32 = 0'
     //@ has - '//*[@id="method.foo"]/*[@class="code-header"]' '#[doc(hidden)] fn foo()'
 }
 //@ has - '//*[@id="impl-TraitHidden-for-Struct"]/*[@class="code-header"]' 'impl TraitHidden for Struct'

tests/rustdoc-html/impl/impl-associated-items-order.rs

@@ -16,7 +16,7 @@ impl Bar {
     // 'pub fn foo()'
     pub fn foo() {}
     //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[1]/h4' \
-    // 'pub const X: u8 = 12u8'
+    // 'pub const X: u8 = 12'
     pub const X: u8 = 12;
     //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[2]/h4' \
     // 'pub type Y = u8'
@@ -34,7 +34,7 @@ impl Foo for Bar {
     // 'type Z = u8'
     type Z = u8;
     //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[1]/h4' \
-    // 'const W: u32 = 12u32'
+    // 'const W: u32 = 12'
     const W: u32 = 12;
     //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[3]/h4' \
     // 'fn yeay()'

tests/rustdoc-ui/diverging-assoc-consts.rs

@@ -0,0 +1,33 @@
+// Ensure that we don't unconditionally evaluate the initializer of associated constants.
+//
+// We once used to evaluate them so we could display more kinds of expressions
+// (like `1 + 1` as `2`) given the fact that we generally only want to render
+// literals (otherwise we would risk dumping extremely large exprs or leaking
+// private struct fields).
+//
+// However, that deviated from rustc's behavior, made rustdoc accept less code
+// and was understandably surprising to users. So let's not.
+//
+// In the future we *might* provide users a mechanism to control this behavior.
+// E.g., via a new `#[doc(...)]` attribute.
+//
+// See also:
+// issue: <https://github.com/rust-lang/rust/issues/131625>
+// issue: <https://github.com/rust-lang/rust/issues/149635>
+
+//@ check-pass
+
+pub struct Type;
+
+impl Type {
+    pub const K0: () = panic!();
+    pub const K1: std::convert::Infallible = loop {};
+}
+
+pub trait Trait {
+    const K2: i32 = panic!();
+}
+
+impl Trait for Type {
+    const K2: i32 = loop {};
+}