Stream: t-compiler

Topic: #54896 panic handler


davidtwco (Oct 11 2018 at 14:19, on Zulip):

Opening a topic for this from weekly meeting on 2018-10-11.

nagisa (Oct 11 2018 at 14:22, on Zulip):

So the most relevant piece of code is this: https://github.com/rust-lang/rust/blob/master/src/librustc_typeck/check/mod.rs#L1169

oli (Oct 11 2018 at 14:23, on Zulip):

I have not been able to reproduce the linker error, but https://play.rust-lang.org/?gist=5b2e557e04e24a8b8c69ff19aea2c45e&version=nightly&mode=debug&edition=2015 compiles successfully and probably should not

nagisa (Oct 11 2018 at 14:23, on Zulip):

yeah I’d expect at least a lint about an unused attribute

nagisa (Oct 11 2018 at 14:23, on Zulip):

not too keen about it erroring though

nagisa (Oct 11 2018 at 14:24, on Zulip):

I guess I just see attributes in a different way than other people :slight_smile:

oli (Oct 11 2018 at 14:24, on Zulip):

right, unused attribute lint and making sure it stays unused and does not try to process the item as a panic handler

nagisa (Oct 11 2018 at 14:25, on Zulip):

Okay so the piece of the code that I’ve linked applies to functions only by extension of being within check_fn.

nagisa (Oct 11 2018 at 14:31, on Zulip):

For the sake of simiplicity I would be fine with making this an error.

nagisa (Oct 11 2018 at 14:31, on Zulip):

but the underlying issue seems to be that for language items the declaration and definition needn’t to match (i.e. language items are not type checked properly)

nagisa (Oct 11 2018 at 14:32, on Zulip):

i.e. it is possible to have extern { #[lang = "foo"] fn banana(); } and then somewhere else a #[lang = "foo"] static X;.

nagisa (Oct 11 2018 at 14:33, on Zulip):

Since the panic_handler attribute is just a stable alias for #[lang="panic_impl"]... perhaps it should be responsible for checking what kind of item it is applied on top of and only expand if applied to a function?

nagisa (Oct 11 2018 at 14:34, on Zulip):

in which case the right piece of code to look at would be this

davidtwco (Oct 11 2018 at 14:37, on Zulip):

Alright, I think I know what needs changed. Just for my own understanding, is there a reason you didn't opt to check in the check_attr module?

nagisa (Oct 11 2018 at 14:38, on Zulip):

No reason. It is not like the attribute does not get used as the implementation stands right now

nagisa (Oct 11 2018 at 14:38, on Zulip):

(it is always noted and always ends up being replaced with a language item)

davidtwco (Oct 11 2018 at 17:42, on Zulip):

@nagisa Hope you don't mind, but I took a slightly different approach in #54997 and extended the language item code to assert for any given language item that the attribute is being placed on the appropriate language construct. However, that means that for this case, we're getting an error rather than a warning that you would have preferred. I'm happy to scrap this and take an alternative approach to just fix this for #[panic_handler] if you want.

Last update: Nov 22 2019 at 04:55UTC