Skip to content

Conversation

JhonnyBillM
Copy link
Contributor

Renamed Diagnostics/Subdiagnostics traits and macros in rust-lang/rust#101558

Cc @rust-lang/wg-diagnostics

@JohnTitor JohnTitor added the S-blocked Status: this PR is blocked waiting for something label Sep 11, 2022
@JohnTitor JohnTitor requested a review from davidtwco September 11, 2022 21:19
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me once unblocked

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2022
…agnostic-handler-refactor, r=davidtwco

Move and rename `SessionDiagnostic` & `SessionSubdiagnostic` traits and macros

After PR rust-lang#101434, we want to:
- [x] Move `SessionDiagnostic` to `rustc_errors`.
- [x] Add `emit_` methods that accept `impl SessionDiagnostic` to `Handler`.
- [x] _(optional)_ Rename trait `SessionDiagnostic` to `DiagnosticHandler`.
- [x] _(optional)_ Rename macro `SessionDiagnostic` to `DiagnosticHandler`.
- [x] Update Rustc Dev Guide and Docs to reflect these changes. rust-lang/rustc-dev-guide#1460

Now I am having build issues getting the compiler to build when trying to rename the macro.

<details>
  <summary>See diagnostics errors and context when building.</summary>

```
error: diagnostics should only be created in `SessionDiagnostic`/`AddSubdiagnostic` impls
  --> compiler/rustc_attr/src/session_diagnostics.rs:13:10
   |
13 |   #[derive(DiagnosticHandler)]
   |            ^^^^^^^^^^^^^^^^^ in this derive macro expansion
   |
  ::: /Users/jhonny/.cargo/registry/src/github.com-1ecc6299db9ec823/synstructure-0.12.6/src/macros.rs:94:9
   |
94 | /         pub fn $derives(
95 | |             i: $crate::macros::TokenStream
96 | |         ) -> $crate::macros::TokenStream {
   | |________________________________________- in this expansion of `#[derive(DiagnosticHandler)]`
   |
note: the lint level is defined here
  --> compiler/rustc_attr/src/lib.rs:10:9
   |
10 | #![deny(rustc::diagnostic_outside_of_impl)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

```

And also this one:

```
error: diagnostics should only be created in `SessionDiagnostic`/`AddSubdiagnostic` impls
   --> compiler/rustc_attr/src/session_diagnostics.rs:213:32
    |
213 |         let mut diag = handler.struct_span_err_with_code(
    |                                ^^^^^^^^^^^^^^^^^^^^^^^^^
```

> **Note**
> Can't find where this message is coming from, because you can see in [this experimental branch](https://github.com/JhonnyBillM/rust/tree/experimental/trying-to-rename-session-diagnostic-macro)  that I updated all errors and diags to say:
> error: diagnostics should only be created in **`DiagnosticHandler`**/`AddSubdiagnostic` impls
> and not:
> error: diagnostics should only be created in **`SessionDiagnostic`**/`AddSubdiagnostic` impls

</details>

I tried building the compiler in different ways (playing with the stages etc), but nothing worked.

## Question

**Do we need to build or do something different when renaming a macro and identifiers?**

For context, see experimental commit JhonnyBillM@f2193a9 where the macro and symbols are renamed, but it doesn't compile.
@JhonnyBillM
Copy link
Contributor Author

Can be unblocked now that rust-lang/rust#101558 has been merged

@davidtwco davidtwco merged commit 1c79085 into rust-lang:master Sep 22, 2022
@JhonnyBillM JhonnyBillM deleted the update-session-diagnostic-to-diagnostic-refactor-docs branch September 22, 2022 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: this PR is blocked waiting for something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants