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.

Sign up for GitHub

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

Jump to bottom

ICEs should point to the clippy repo, not the rustc repo #2734

Closed
oli-obk opened this issue May 8, 2018 · 4 comments · Fixed by #4588
Closed

ICEs should point to the clippy repo, not the rustc repo #2734

oli-obk opened this issue May 8, 2018 · 4 comments · Fixed by #4588
Assignees
@phansch
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy

Comments

@oli-obk
Copy link
Contributor

oli-obk commented May 8, 2018

Frequently ICEs in clippy are reported in the rustc repo. It would probably be better to collect them only here. This might need changes to rustc_driver to allow custom drivers to inject a repo name to report ICEs to.
@phansch
Copy link
Member

phansch commented May 8, 2018 •
edited
Loading

Maybe we could use human-panic? I was trying to get it to work with clippy and it seems easy to add.

@oli-obk
Copy link
Contributor Author

oli-obk commented May 8, 2018

well... rustc could use it and then all drivers would benefit from it. I think all drivers should use a common infrastructure

@kennytm
Copy link
Member

kennytm commented May 12, 2018

Well this particular issue should be reported to the rustc repo instead 😛

@phansch phansch added the C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages label May 13, 2018
@phansch phansch added the good-first-issue These issues are a good way to get started with Clippy label Sep 24, 2019
@phansch
Copy link
Member

phansch commented Sep 24, 2019 •
edited
Loading

It's now possible to provide our own ICE hook. We currently use the rustc one via

rust-clippy/src/driver.rs

Line 250 in a5e568b

rustc_driver::install_ice_hook();

To do that we only have to provide our own hook that calls rustc's report_ice function, similar to the one in rustc:

https://github.com/rust-lang/rust/blob/66bf391c3aabfc77f5f7139fc9e6944f995d574e/src/librustc_driver/lib.rs#L1172-L1178

Then we'lll also have to initialize our hook, like rustc:

https://github.com/rust-lang/rust/blob/66bf391c3aabfc77f5f7139fc9e6944f995d574e/src/librustc_driver/lib.rs#L1253-L1255

All this would happen in src/driver.rs.

@mati865 mati865 mentioned this issue Sep 26, 2019
Closed
@phansch phansch self-assigned this Sep 26, 2019
phansch added a commit to phansch/rust-clippy that referenced this issue Sep 27, 2019
@phansch
Add custom ICE message that points to Clippy repo
4992033 
This utilizes rust-lang/rust#60584 by setting
our own `panic_hook` and pointing to our own issue tracker instead of
the rustc issue tracker.

This also adds a new internal lint to test the ICE message.

**Potential downsides**

* This essentially copies rustc's `report_ice` function as
  `report_clippy_ice`. I think that's how it's meant to be implemented, but
  maybe @jonas-schievink could have a look as well =)

  The downside of more-or-less copying this function is that we have to
  maintain it as well now.
  The original function can be found [here][original].
* `driver` now depends directly on `rustc` and `rustc_errors`

Closes rust-lang#2734

[original]: https://github.com/rust-lang/rust/blob/59367b074f1523353dddefa678ffe3cac9fd4e50/src/librustc_driver/lib.rs#L1185
@phansch phansch mentioned this issue Sep 27, 2019
Merged
phansch added a commit to phansch/rust-clippy that referenced this issue Oct 8, 2019
@phansch
Add custom ICE message that points to Clippy repo
3aff8b3 
This utilizes rust-lang/rust#60584 by setting
our own `panic_hook` and pointing to our own issue tracker instead of
the rustc issue tracker.

This also adds a new internal lint to test the ICE message.

**Potential downsides**

* This essentially copies rustc's `report_ice` function as
  `report_clippy_ice`. I think that's how it's meant to be implemented, but
  maybe @jonas-schievink could have a look as well =)

  The downside of more-or-less copying this function is that we have to
  maintain it as well now.
  The original function can be found [here][original].
* `driver` now depends directly on `rustc` and `rustc_errors`

Closes rust-lang#2734

[original]: https://github.com/rust-lang/rust/blob/59367b074f1523353dddefa678ffe3cac9fd4e50/src/librustc_driver/lib.rs#L1185
bors added a commit that referenced this issue Oct 9, 2019
@bors
Auto merge of #4588 - phansch:add_custom_ice_hook, r=Manishearth
e261df4 
Add custom ICE message that points to Clippy repo

changelog: Link to Clippy issue tracker in ICE messages

This utilizes rust-lang/rust#60584 by setting
our own `panic_hook` and pointing to our own issue tracker instead of
the rustc issue tracker.

This also adds a new internal lint to test the ICE message.

**Potential downsides**

* This essentially copies rustc's `report_ice` function as
  `report_clippy_ice`. I think that's how it's meant to be implemented, but
  maybe @jonas-schievink could have a look as well =)

  The downside of more-or-less copying this function is that we have to
  maintain it as well now.
  The original function can be found [here][original].
* `driver` now depends directly on `rustc` and `rustc_errors`

Closes #2734

[original]: https://github.com/rust-lang/rust/blob/59367b074f1523353dddefa678ffe3cac9fd4e50/src/librustc_driver/lib.rs#L1185
bors added a commit that referenced this issue Oct 9, 2019
@bors
Auto merge of #4588 - phansch:add_custom_ice_hook, r=Manishearth
64d77b2 
Add custom ICE message that points to Clippy repo

changelog: Link to Clippy issue tracker in ICE messages

This utilizes rust-lang/rust#60584 by setting
our own `panic_hook` and pointing to our own issue tracker instead of
the rustc issue tracker.

This also adds a new internal lint to test the ICE message.

**Potential downsides**

* This essentially copies rustc's `report_ice` function as
  `report_clippy_ice`. I think that's how it's meant to be implemented, but
  maybe @jonas-schievink could have a look as well =)

  The downside of more-or-less copying this function is that we have to
  maintain it as well now.
  The original function can be found [here][original].
* `driver` now depends directly on `rustc` and `rustc_errors`

Closes #2734

[original]: https://github.com/rust-lang/rust/blob/59367b074f1523353dddefa678ffe3cac9fd4e50/src/librustc_driver/lib.rs#L1185
phansch added a commit to phansch/rust-clippy that referenced this issue Nov 15, 2019
@phansch
Add custom ICE message that points to Clippy repo
c85f32a 
This utilizes rust-lang/rust#60584 by setting
our own `panic_hook` and pointing to our own issue tracker instead of
the rustc issue tracker.

This also adds a new internal lint to test the ICE message.

**Potential downsides**

* This essentially copies rustc's `report_ice` function as
  `report_clippy_ice`. I think that's how it's meant to be implemented, but
  maybe @jonas-schievink could have a look as well =)

  The downside of more-or-less copying this function is that we have to
  maintain it as well now.
  The original function can be found [here][original].
* `driver` now depends directly on `rustc` and `rustc_errors`

Closes rust-lang#2734

[original]: https://github.com/rust-lang/rust/blob/59367b074f1523353dddefa678ffe3cac9fd4e50/src/librustc_driver/lib.rs#L1185
phansch added a commit to phansch/rust-clippy that referenced this issue Nov 29, 2019
@phansch
Add custom ICE message that points to Clippy repo
1fb522e 
This utilizes rust-lang/rust#60584 by setting
our own `panic_hook` and pointing to our own issue tracker instead of
the rustc issue tracker.

This also adds a new internal lint to test the ICE message.

**Potential downsides**

* This essentially copies rustc's `report_ice` function as
  `report_clippy_ice`. I think that's how it's meant to be implemented, but
  maybe @jonas-schievink could have a look as well =)

  The downside of more-or-less copying this function is that we have to
  maintain it as well now.
  The original function can be found [here][original].
* `driver` now depends directly on `rustc` and `rustc_errors`

Closes rust-lang#2734

[original]: https://github.com/rust-lang/rust/blob/59367b074f1523353dddefa678ffe3cac9fd4e50/src/librustc_driver/lib.rs#L1185
bors added a commit that referenced this issue Nov 29, 2019
@bors
Auto merge of #4588 - phansch:add_custom_ice_hook, r=Manishearth
0e66a78 
Add custom ICE message that points to Clippy repo

changelog: Link to Clippy issue tracker in ICE messages

This utilizes rust-lang/rust#60584 by setting
our own `panic_hook` and pointing to our own issue tracker instead of
the rustc issue tracker.

This also adds a new internal lint to test the ICE message.

**Potential downsides**

* This essentially copies rustc's `report_ice` function as
  `report_clippy_ice`. I think that's how it's meant to be implemented, but
  maybe @jonas-schievink could have a look as well =)

  The downside of more-or-less copying this function is that we have to
  maintain it as well now.
  The original function can be found [here][original].
* `driver` now depends directly on `rustc` and `rustc_errors`

Closes #2734

[original]: https://github.com/rust-lang/rust/blob/59367b074f1523353dddefa678ffe3cac9fd4e50/src/librustc_driver/lib.rs#L1185
@bors bors closed this as completed in 676f14b Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@phansch phansch

Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add custom ICE message that points to Clippy repo phansch/rust-clippy
3 participants
@kennytm @oli-obk @phansch