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

Deriving may want to ignore marker fields #14268

Closed
lilyball opened this issue May 18, 2014 · 5 comments
Closed

Deriving may want to ignore marker fields #14268

lilyball opened this issue May 18, 2014 · 5 comments

Comments

@lilyball
Copy link
Contributor

Marker fields are used to provide extra information about types, including opting out of Copy, Send, and Share bounds. However, the presence of a marker field can prevent the ability to #[derive] traits for that type. For example, core::kinds::marker::NoCopy only implements Clone and Eq (according to rustdoc; I am unsure if there are any libstd traits it implements because rustdoc doesn't show cross-crate traits). This means that any struct with a NoCopy field cannot e.g. derive Show, or Ord, or other useful traits.

I think the right solution here is to simply not consider the marker fields when deriving traits, with the only exception being Clone. Every other trait should just pretend that field doesn't exist.

@huonw
Copy link
Member

huonw commented May 18, 2014

I think the right solution here is to simply not consider the marker fields when deriving traits, with the only exception being Clone. Every other trait should just pretend that field doesn't exist.

There's no way to do this in general, #[deriving] cannot tell if a field is a marker or not.

Implementing more traits for the markers would work.

@lilyball
Copy link
Contributor Author

Implementing more traits for the markers would work.

Every single derivable trait would have to be implemented on the markers (except for Send/Share/Copy, of course). And that's not necessarily going to produce desired behavior in the container struct.

For example, a struct with a marker is unlikely to find Encodable/Decodable to be useful traits to derive, as the intended JSON (or whatever) representation is unlikely to want a nocopy field.

Similarly, including the field in Show is probably not going to be very useful.

There's no way to do this in general

Crud, looks like you're right. Deriving can see the path to the type, but I guess it can't actually get any info about it beyond that path.

An alternative would be an attribute on the field that says it should be skipped for derivings. Although again, it would want to be ignored for Clone, and I'm not sure how to handle Decodable either (unless in both cases we resort to a default value). Something like

#[deriving(Show)]
struct Foo {
    x: int,
    #[deriving(skip)]
    nocopy: ::std::kinds::marker::NoCopy
}

We could even use Default::default() to provide the default value (which does require implementing that one trait on the marker types).

@Kimundi
Copy link
Member

Kimundi commented May 18, 2014

An attribute to exclude a field sounds like a more useful solution, and also easily allows customization for other use cases.

@lilyball
Copy link
Contributor Author

@Kimundi I have a PR #14219 that allows for customizing of #[deriving(Default)] on a per-field basis. It uses an attribute like #[default="42"]. Perhaps this is the right time to talk about a more general approach to customizing Deriving in general. @alexcrichton seems to be against the idea, although I'm not sure why.

Perhaps we could re-use the deriving name, so that my above attribute is #[deriving(default="42")]? (assuming that syntax is compatible with the current attribute parser, I haven't checked to see what exactly we're accepting now, but of course the parser isn't set in stone). Then skipping fields could just be #[deriving(skip)] to mean "skip when possible". We'd have to define clone and decodable as perhaps using Default::default() as the value (which has a nice symmetry with #[deriving(Default)], e.g. derived traits that return a new instance of the struct will fill each field with Default::default() unless the derived trait instructs otherwise). Then we could also do e.g. #[deriving(Show=skip)]. Or maybe it would be better as #[deriving(skip(Show))].

@steveklabnik
Copy link
Member

I'm pulling a massive triage effort to get us ready for 1.0. As part of this, I'm moving stuff that's wishlist-like to the RFCs repo, as that's where major new things should get discussed/prioritized.

This issue has been moved to the RFCs repo: rust-lang/rfcs#726

bors added a commit to rust-lang-ci/rust that referenced this issue Jan 8, 2024
internal: Switch to `expected.assert_eq` for `ide` tests

This PR switches from `assert_debug_eq` to `assert_eq` and only compares parts of the result and not the whole. The aim is to only compare parts which are relevant to the test and also make it more readable.

Part of rust-lang#14268.

## Questions
- [x] Can I use `Vec`? If not, what is the alternative?
    I assume I cannot because of: https://github.com/rust-lang/rust-analyzer/blob/c3a00b5468576de4e39adc8fa5ceae35a0024e49/docs/dev/architecture.md?plain=1#L413
- [x] Should I group it by file, as proposed by Lukas?
    ```
    file_id 1:
        source_file_edits:
            - Indel { insert: "foo2", delete: 4..7 }

    file_id 2:
        file_system_edits:
            MoveFile AnchoredPathBuf { anchor: FileId(2), path: "foo2.rs", }
    ```
- [x] Is it okay to ignore `CreateFile` events? They do not have a FileId, which would be problematic, but they do not occur in the existing tests, so I marked them as `unreachable!()` so far.
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

No branches or pull requests

4 participants