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

Complete move to "diagnostic items" #66075

Closed
2 of 3 tasks
RalfJung opened this issue Nov 4, 2019 · 1 comment · Fixed by #76910
Closed
2 of 3 tasks

Complete move to "diagnostic items" #66075

RalfJung opened this issue Nov 4, 2019 · 1 comment · Fixed by #76910
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Nov 4, 2019

With #60966, we can avoid lang items and manual path matching for lints, and instead mark them in the code. But I think there are some places that could still be ported to the new system:

  • The InvalidValue lint uses match_def_path for transmute -- somehow, using a diagnostic item did not work (is_diagnostic_item returns false).
  • Aren't Rc and Arc lang items just for diagnostics? Here's where they get used:
    try_alt_rcvr(&mut err, rcvr_t, lang_items::Arc);
    try_alt_rcvr(&mut err, rcvr_t, lang_items::Rc);

    And, indirectly, at
    if ty.is_rc() {
    format!("an `Rc`")
    } else if ty.is_arc() {
    format!("an `Arc`")

    So, I am not sure if those should be lang items or diagnostic items now? No language semantics is attached to them, the uses above are just in error messages from what I can see.
  • There's also no language semantics attached to MaybeUninit; it is a lang item because it gets used in layout construction for generators. Doesn't really fit "diagnostics item" either, though. (EDIT: decided this makes more sense to remain a lang item)

@oli-obk @Centril any thoughts?

@Centril Centril added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 5, 2019
@Centril
Copy link
Contributor

Centril commented Nov 5, 2019

So, I am not sure if those should be lang items or diagnostic items now? No language semantics is attached to them, the uses above are just in error messages from what I can see.

If they are only used in diagnostics then they shouldn't be lang items.

  • There's also no language semantics attached to MaybeUninit; it is a lang item because it gets used in layout construction for generators. Doesn't really fit "diagnostics item" either, though.

A reasonable argument could be made for "layout construction for generators" => language semantics attached; but it's a sort of code-reuse that's going on in layout construction... something with similar semantics as compared to MaybeUninit<T> could ostensibly be used instead. cc @tmandry

JohnTitor added a commit to JohnTitor/rust that referenced this issue Nov 10, 2019
invalid_value lint: use diagnostic items

This adjusts the invalid_value lint to use diagnostic items.

@Centril @oli-obk For some reason, this fails to recognize `transmute` -- somehow the diagnostic item is not found. Any idea why?

r? @Centril

Cc rust-lang#66075
RalfJung added a commit to RalfJung/rust that referenced this issue Sep 20, 2020
transmute: use diagnostic item

closes rust-lang#66075, we now have no remaining uses of `match_def_path`  in the compiler while some uses still remain in `clippy`.

cc @RalfJung
@bors bors closed this as completed in 7ff17c1 Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants