Stream: t-compiler

Topic: Reviewing #71205 fix check_attr()


LeSeulArtichaut (Apr 29 2020 at 14:49, on Zulip):

#71205 is currently assigned to eddyb but I think they have other things to do and other PRs to review. I expect #71205 to be pretty quick to review, and it would unblock my progress on #69274. There is nothing urgent, but it would be nice if someone had some time to review!

Jonas Schievink (Apr 29 2020 at 18:09, on Zulip):

Code looks good to me. What kind of code is rejected by this that wasn't before? Only code using #[target_feature], or other code as well?

LeSeulArtichaut (Apr 29 2020 at 18:10, on Zulip):

I think that it concerns attributes that might be rejected

LeSeulArtichaut (Apr 29 2020 at 18:10, on Zulip):

Take a look at the codegen_attrs

LeSeulArtichaut (Apr 29 2020 at 18:11, on Zulip):

For certain attributes, there are additional checks, and errors emitted

Jonas Schievink (Apr 29 2020 at 18:17, on Zulip):

The only concern I see is that this might cause currently accepted code (on stable) to be rejected, for example malformed #[inline] attributes.

Jonas Schievink (Apr 29 2020 at 18:17, on Zulip):

Probably not worth starting a crater run just for this, but it's something to keep in mind

LeSeulArtichaut (Apr 29 2020 at 18:22, on Zulip):

Code that might break with this PR is code that should be invalid though

LeSeulArtichaut (Apr 29 2020 at 18:24, on Zulip):

If it breaks too much code we can also introduce a warning lint to have some transition

Jonas Schievink (Apr 29 2020 at 18:24, on Zulip):

Yes, but often we've introduced these kinds of breaking changes by starting with a warning period. This doesn't really seem to be worth it in this case though, since the code is trivial to fix (remove the invalid attribute, or fix its syntax).

Jonas Schievink (Apr 29 2020 at 18:24, on Zulip):

Yeah, seems fine. I've approved it.

LeSeulArtichaut (Apr 29 2020 at 18:24, on Zulip):

Crater is running for the beta, if we want to run crater we’ll have to wait quite long :slight_smile:

LeSeulArtichaut (Apr 29 2020 at 18:25, on Zulip):

Thanks @Jonas Schievink!

Last update: Jun 04 2020 at 18:45UTC