Closed cbiffle closed 2 years ago
On first glance, I'm guessing this is from https://github.com/rust-embedded/cortex-m-rt/pull/224 in 0.7.0; presumably this broke on a 0.6->0.7 upgrade?
On first glance, I'm guessing this is from rust-embedded/cortex-m-rt#224 in 0.7.0; presumably this broke on a 0.6->0.7 upgrade?
Yeah, I was going through bumping deps in lilos when I hit this.
From the description in rust-embedded/cortex-m-rt#224 it looks like it was intended to be backwards compatible, but also appears to be designed to pick up whatever exception
is defined in the module where the macro is used? Those goals may be incompatible, as this breakage suggests.
Yep, agreed. I think we could keep the part of that PR that exports the Exception enum publicly (which is what the changelog talks about, too) but revert the part that uses a full name for the macro, since it seems like teensy4-rt no longer exists.
Whether we do that in a 0.7.x patch release I'm not completely sure yet, though. It would presumably break anyone else relying on this behaviour, but it's not really part of the documented interface either.
cortex-m-rt
version changes in a library (such as an operating system) probably need to happen on library major version changes anyway, to ensure that user code and the library are not linking in different versions of cortex-m-rt
. So, I can bump major versions and work around this breakage. I would suggest maybe noting this in the release notes.
which is what the changelog talks about, too
I'm not sure what changelog you're referring to, but when I hit this issue the first place I went was the release notes, which list only a single change in the entire release.
Edit: Ah, I see, github is only showing me the 0.7.1 patch release -- if I go digging I can find 0.7.0 notes as well. It's too bad that they're not on the page below the 0.7.1 notes, but, that's Github.
Where are you looking? The GitHub release notes have all the releases, and the CHANGELOG file in the repo likewise. I think it's mainly that this change happened in 0.7.0, not 0.7.1.
I'm considering having a cortex-m-rt 0.7.2 that fixes this if it's no longer needed for teensy4-rt, though, as as you noted I don't think breaking this use of the macro was deliberately intended. I don't think we can also make it so that people can use cortex-m-rt-macros without using cortex-m-rt, but it seems teensy4-rs no longer have that use case, it's not something we've directly supported (beyond that PR), and I don't know of anyone else doing it.
Where are you looking?
Went to the github repo and clicked on the only thing shown in the Releases column to the right, which goes to this 0.7.1-specific page without any other changes listed. This is a Github thing, I think, and not anything this project has control over.
I don't think we can also make it so that people can use cortex-m-rt-macros without using cortex-m-rt
FWIW, switching to my other hat, we also don't need this in Hubris -- the kernel uses cortex-m-rt
with its macros, and tasks use neither.
It looks like the PAC crate interrupt
macros often have this same hygiene issue -- this is true for at least stm32l4
and stm32h7
.
On the svd2rust PAC side, the macro is directly exported from cortex-m-rt here, so all svd2rust PACs will be the same I expect.
In this case the (lack of) hygiene is even more deliberate, because the interrupt macro needs an interrupt
enum to exist from the PAC; see https://docs.rs/cortex-m-rt/latest/cortex_m_rt/attr.interrupt.html. I don't think there's much to be done about that short of changing how the macros tie in to the PACs (which is not necessarily a bad idea, but is a bigger change).
Previously, one could apply
#[cortex_m_rt::exception]
fully qualified, as shown here.With the release of 0.7.1, this has stopped working. For the macro to work, one must now
use cortex_m_rt::exception
and then use#[exception]
. The error message is not great, but suggests that it's a problem with the macro referring to itself by an unqualified name internally:(That message sent me on a bit of a goose chase figuring out why
exception
was not defined incortex_m_rt
, until I worked it out.)