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

put reflection in format strings behind a feature gate or mark it with #[experimental] #12019

Closed
thestinger opened this issue Feb 4, 2014 · 13 comments · Fixed by #14364
Closed
Milestone

Comments

@thestinger
Copy link
Contributor

This is an ABI stability hazard as it makes the entire internal representation of a type part of the public ABI. The standard library modules are now marked experimental, but this warning doesn't trigger when they're used via the format macros.

@alexcrichton
Copy link
Member

Nominating.

@thestinger
Copy link
Contributor Author

An alternative would be adding a lint warning for this mentioning the ABI stability hazard. That way, it's very easy to use for debugging but relying on it to make the application/library work requires making the informed decision to silence the warning or ignore it.

@alexcrichton
Copy link
Member

A wacky idea would be to make this a lint that's allow-by-default at O0 and deny-by-default at O1+.

@sfackler
Copy link
Member

sfackler commented Feb 4, 2014

I would personally lean towards a lint as opposed to a feature gate.

@nikomatsakis
Copy link
Contributor

cc me

@pnkfelix
Copy link
Member

pnkfelix commented Feb 6, 2014

Assigning P-backcompat-lang, 1.0 blocker. (For making decisions about this and reflection more generally)

@pnkfelix pnkfelix added this to the 1.0 milestone Feb 6, 2014
@brson
Copy link
Contributor

brson commented Feb 6, 2014

We're not relying on this much anymore, can potentially remove reflection.

@lilyball
Copy link
Contributor

I am in favor of a lint. It's very useful to keep for debugging purposes, but I wouldn't go so far as to deny it outright for non-debug builds. A lint that warns by default seems like a reasonable compromise.

@thestinger
Copy link
Contributor Author

@kballard: It's a lot less useful now that it can't look inside vectors. Applications making good use of data structures like maps and sets are also unable to get much from it in many cases. It might make sense to just discard this kind of reflection.

@lilyball
Copy link
Contributor

@thestinger It's useful for custom user-defined data structures.

I wish there was a hybrid reflection, which would use Show for types that implement it and Poly for types that don't, but there's no way to make this work for collections without implementing the hybrid directly for them (because collections implement Show in terms of their elements, which must implement Show as well, but what I really want is to be able to print a vector of non-Show-able objects).

I don't suppose it would be possible to have a macro impl_debug_show!(MyType) that implements Show by returning the output of Poly, and having a lint be able to find invocations of that? This would require the lint being able to see through generic implementations of Show::fmt() and tell that they're calling a concrete implementation that's been flagged as a debug show, and I don't know if lints have that capability.

The idea here is that I could do something like

struct Foo {
    a: int,
    b: ~str
}
impl_debug_show!(Foo)

fn foos() -> ~[Foo] {
    ~[Foo { a: 3, b: ~"test" }, Foo { a: 4, b: ~"hello" }]
}

fn main() {
    let x = foos();
    println!("{}", x);
}

and get something like

[Foo { a: 3, b: ~"test" }, Foo { a: 4, b: ~"hello" }]

printed out, along with a lint warning during compilation pointing at the println!() call informing me that I'm relying on a debug Show.

@lilyball
Copy link
Contributor

I suppose we could add a hybrid show trait after all, implement it for all Rust-provided collections, and live with it using Poly for any user-defined collections that don't explicitly implement this, because at least it will still work with vectors and hash maps (which are probably the most important collections to be able to print).

@nikomatsakis
Copy link
Contributor

Why would impl_debug_show! be better than #[deriving(Show)]?

@lilyball
Copy link
Contributor

@nikomatsakis You mean a #[deriving(DebugShow)]? I suggested the macro because you could insert that somewhere else in your crate in order to turn on debug-printing of a struct, whereas deriving needs to be done at the struct definition, but this is all so hacky anyway.

alexcrichton added a commit to alexcrichton/rust that referenced this issue May 28, 2014
This commit moves reflection (as well as the {:?} format modifier) to a new
libdebug crate, all of which is marked experimental.

This is a breaking change because it now requires the debug crate to be
explicitly linked if the :? format qualifier is used. This means that any code
using this feature will have to add `extern crate debug;` to the top of the
crate. Any code relying on reflection will also need to do this.

Closes rust-lang#12019

[breaking-change]
bors added a commit that referenced this issue May 28, 2014
This commit moves reflection (as well as the {:?} format modifier) to a new
libdebug crate, all of which is marked experimental.

This is a breaking change because it now requires the debug crate to be
explicitly linked if the :? format qualifier is used. This means that any code
using this feature will have to add `extern crate debug;` to the top of the
crate. Any code relying on reflection will also need to do this.

Closes #12019

[breaking-change]
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
derive completions take existing derives into count

fixes rust-lang#12019

The immediate reason is that when we are doing derive completion, [`ctx.existing_derives`](https://github.com/rust-lang/rust-analyzer/blob/d1f6b4e2a0ab1a1343ab4a381c89b186a76fd001/crates/ide_completion/src/completions/attribute/derive.rs#L82) is empty, this is because we expand the macro when looking for the ancestors of the token to be completed. Take the following code as an example, we find the first `SyntaxNode` with kind `Attr` based on the ancestors of the token, but the parent of `Attr` is not a `Struct` as we [expect](https://github.com/rust-lang/rust-analyzer/blob/d1f6b4e2a0ab1a1343ab4a381c89b186a76fd001/crates/hir/src/semantics.rs#L518).
```rust
#[derive(PartialEq, Eq, Or$0)]
struct S;
```
The ancestors of the token to be completed above.
```
    NAME_REF@24..26
      IDENT@24..26 "Or"
    ,
    PATH_SEGMENT@24..26
      NAME_REF@24..26
        IDENT@24..26 "Or"
    ,
    PATH@24..26
      PATH_SEGMENT@24..26
        NAME_REF@24..26
          IDENT@24..26 "Or"
    ,
    META@24..26
      PATH@24..26
        PATH_SEGMENT@24..26
          NAME_REF@24..26
            IDENT@24..26 "Or"
    ,
    ATTR@21..28
      POUND@21..22 "#"
      WHITESPACE@22..23 " "
      L_BRACK@23..24 "["
      META@24..26
        PATH@24..26
          PATH_SEGMENT@24..26
            NAME_REF@24..26
              IDENT@24..26 "Or"
      R_BRACK@26..27 "]"
      WHITESPACE@27..28 " "
    ,
    TUPLE_EXPR@0..32
      ATTR@0..14
        POUND@0..1 "#"
        WHITESPACE@1..2 " "
        L_BRACK@2..3 "["
        META@3..12
          PATH@3..12
            PATH_SEGMENT@3..12
              NAME_REF@3..12
                IDENT@3..12 "PartialEq"
        R_BRACK@12..13 "]"
        WHITESPACE@13..14 " "
      ATTR@14..21
        POUND@14..15 "#"
        WHITESPACE@15..16 " "
        L_BRACK@16..17 "["
        META@17..19
          PATH@17..19
            PATH_SEGMENT@17..19
              NAME_REF@17..19
                IDENT@17..19 "Eq"
        R_BRACK@19..20 "]"
        WHITESPACE@20..21 " "
      ATTR@21..28
        POUND@21..22 "#"
        WHITESPACE@22..23 " "
        L_BRACK@23..24 "["
        META@24..26
          PATH@24..26
            PATH_SEGMENT@24..26
              NAME_REF@24..26
                IDENT@24..26 "Or"
        R_BRACK@26..27 "]"
        WHITESPACE@27..28 " "
      L_PAREN@28..29 "("
      WHITESPACE@29..30 " "
      R_PAREN@30..31 ")"
      WHITESPACE@31..32 " "
...
```

I make a small change to not do macro expansion when looking up the ancestors of the token.

What I don't understand is that `self.sema.token_ancestors_with_macros(self.token.clone())` doesn't seem to expand the macro if the derive completion triggered without any prefix, like `#[derive(PartialEq, Eq, $0)]`.

The ancestors of the token with  `#[derive(PartialEq, Eq, $0)]`.
```
    TOKEN_TREE@8..25
      L_PAREN@8..9 "("
      IDENT@9..18 "PartialEq"
      COMMA@18..19 ","
      WHITESPACE@19..20 " "
      IDENT@20..22 "Eq"
      COMMA@22..23 ","
      WHITESPACE@23..24 " "
      R_PAREN@24..25 ")"
    ,
    META@2..25
      PATH@2..8
        PATH_SEGMENT@2..8
          NAME_REF@2..8
            IDENT@2..8 "derive"
      TOKEN_TREE@8..25
        L_PAREN@8..9 "("
        IDENT@9..18 "PartialEq"
        COMMA@18..19 ","
        WHITESPACE@19..20 " "
        IDENT@20..22 "Eq"
        COMMA@22..23 ","
        WHITESPACE@23..24 " "
        R_PAREN@24..25 ")"
    ,
    ATTR@0..26
      POUND@0..1 "#"
      L_BRACK@1..2 "["
      META@2..25
        PATH@2..8
          PATH_SEGMENT@2..8
            NAME_REF@2..8
              IDENT@2..8 "derive"
        TOKEN_TREE@8..25
          L_PAREN@8..9 "("
          IDENT@9..18 "PartialEq"
          COMMA@18..19 ","
          WHITESPACE@19..20 " "
          IDENT@20..22 "Eq"
          COMMA@22..23 ","
          WHITESPACE@23..24 " "
          R_PAREN@24..25 ")"
      R_BRACK@25..26 "]"
    ,
    STRUCT@0..39
      ATTR@0..26
        POUND@0..1 "#"
        L_BRACK@1..2 "["
        META@2..25
          PATH@2..8
            PATH_SEGMENT@2..8
              NAME_REF@2..8
                IDENT@2..8 "derive"
          TOKEN_TREE@8..25
            L_PAREN@8..9 "("
            IDENT@9..18 "PartialEq"
            COMMA@18..19 ","
            WHITESPACE@19..20 " "
            IDENT@20..22 "Eq"
            COMMA@22..23 ","
            WHITESPACE@23..24 " "
            R_PAREN@24..25 ")"
        R_BRACK@25..26 "]"
      WHITESPACE@26..27 " "
      STRUCT_KW@27..33 "struct"
      WHITESPACE@33..34 " "
      NAME@34..38
        IDENT@34..38 "Test"
      SEMICOLON@38..39 ";"
...
```
flip1995 pushed a commit to flip1995/rust that referenced this issue Dec 28, 2023
Changelog for Rust 1.75 🎄

Roses are red,
Violets are blue,
Another year of development,
And a strong team, too

---

### The cat of this release:

<img width=500 src="https://cdn2.thecatapi.com/images/49r.gif?api_key=live_iA7QQQ2LdohvDfCghXThmLqVYCZ9kXIwGMcwyJyaYyOTRC8NZSYqykPoc2UypsMi" alt="The cats of this Clippy release" />

<sub>The cat for the next release can be nominated in the comments</sub>

---

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants