Skip to content

make easier/possible to detect derive attributes after expansion #45216

Open
Open
make easier/possible to detect derive attributes after expansion#45216
Labels
A-macrosArea: All kinds of macros (custom derive, macro_rules!, proc macros, ..)C-feature-requestCategory: A feature request, i.e: not implemented / a PR.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Description

@zackmdavis
zackmdavis
opened on Oct 11, 2017

In the spirit of #44942, it would be nice if the missing_debug_implementations and missing_copy_implementations lints suggested adding a derive attribute (in the case where one does not already exist) or adding Debug (respectively Copy) to the list of traits in the derive attribute (in the case where it already exists). (This mostly for the sake of RLS and other tools, but the span highlighting is pretty, too.) But while the lints (obviously, necessarily) know how to look up whether the trait has been implemented, it's not clear how to make them detect an existing derive attribute (and its span): cx.tcx.get_attrs doesn't work because we apparently strip off the attribute during expansion (it looks like this happens twice, which is confusing: 1 2). But I can't modify the expansion code because I'm still not smart enough to understand it.

Activity

  1. TimNN
    added
    C-feature-requestCategory: A feature request, i.e: not implemented / a PR.
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    on Oct 17, 2017

  2. zackmdavis commented on Nov 13, 2017

    @zackmdavis
    zackmdavis
    on Nov 13, 2017
    ContributorAuthor

    Stripping derive has also created problems elsewhere.

  3. zackmdavis commented on Nov 13, 2017

    @zackmdavis
    zackmdavis
    on Nov 13, 2017
    ContributorAuthor

    cc @oli-obk @jseyfried ?

  4. jseyfried commented on Jan 5, 2018

    @jseyfried
    jseyfried
    on Jan 5, 2018
    Contributor

    I think it would be reasonable to avoid stripping derives.

    @zackmdavis

    it looks like this happens twice, which is confusing: 1 [2](https://github.com/rust-lang/rust/blob/cbf5d39cca/src/libsyntax/ext/derive.rs#L25-L27

    The second link is only stripping empty derives and erroneous derives (after emitting an error/warning) -- the second match arm is the successful case and retains the derive. Feel free to ping me on IRC to discuss how to proceed.

  5. Enselic commented on Oct 3, 2023

    @Enselic
    Enselic
    on Oct 3, 2023
    Member

    Triage: Currently the suggestion looks like this:

    #![warn(missing_debug_implementations)]

#[derive(Clone)]
pub struct MissingDebug;

fn main() {
}
    warning: type does not implement `Debug`; consider adding `#[derive(Debug)]` or a manual implementation
 --> src/main.rs:4:1
  |
4 | pub struct MissingDebug;
  | ^^^^^^^^^^^^^^^^^^^^^^^^
  |
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![warn(missing_debug_implementations)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    If I understand this issue correctly, it is about enabling the suggestion to be more specific by taking into account currently present derives. Maybe something like:

    warning: type does not implement Debug; consider adding Debug to #[derive(Clone, ...)] or a manual implementation

  6. Enselic
    added
    A-macrosArea: All kinds of macros (custom derive, macro_rules!, proc macros, ..)
    on Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-macrosArea: All kinds of macros (custom derive, macro_rules!, proc macros, ..)C-feature-requestCategory: A feature request, i.e: not implemented / a PR.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions