How would you feel about macrofying more of the implementation?

[HadrienG2]

While investigating #105 and #104, I noticed that there is a lot of duplicated patterns in the code, like this sort of function declaration header...

#[must_use]
#[inline(always)]
#[cfg_attr(docs_rs, doc(cfg(target_feature = <some feature>)))]

...and most implementations follow a very simple pattern like "pass through the elements to the intrinsic, possibly with some .0 access, and get the result back" or "get the immediate const and use it as the rightmost argument to the intrinsic".

Replacing as many of these patterns as possible with code generation macros would have some advantages:

• It would reduce the risk for copy-paste errors like #104
• It would simplify global changes (e.g. at some point you started using #[must_use], but you did not do it everywhere in a seemingly inconsistent way which suggests you forgot / did not find the motivation to propagate it).
• It would make the code more focused on its core business of wrapping intrinsics, with less line noise that the eye just scans through without reading even though it may actually contain important info.

It would also have some maintainability drawbacks: even declarative macros have quite arcane syntax that makes them hard to write, debug and extend. So more suffering when you or someone else needs to edit the macro. Also IDE-style tooling like rust-analyzer tends to just give up inside of macros, so things like autoformat and autocompletion may not work as nicely as usual when editing code.

On the neutral side, there are some things we can't do with (declarative) macros and still need to copy-paste by hand if we're not ready to go all the way to procedural macros (which I would not advise). This most importantly includes links to the intrinsics in the documentation.

To help gauge the tradeoff, I tried my hand at a quick prototype that does the codegen work for the small adx instruction set using declarative macros. The macro would obviously get more complex as more instruction sets are added, but I think this already gives a fairly good feeling of what the end result would look like : https://github.com/HadrienG2/safe_arch/compare/main...HadrienG2:safe_arch:macrofy .

How would you feel about this change?

[Lokathor]

Macros would probably be fine? Long ago when the crate was made RA was pretty bad at expanding macros and so couldn't give code suggestions of macro functions and stuff, but these days it's usually pretty good about it.

A particular function can always just not be a macro if necessary, but for the simple functions a macro is probably fine.

On a slightly similar note: I've seen inline(always) let LLVM do some inlines it shouldn't recently when working with GBA and the instruction_set attribute, so the new macro should probably relax that attribute to just inline.

[HadrienG2]

Out of curiosity, what issues did you encounter with inline(always)? I tend to use it in "not inlining would be a total performance disaster" situations, so if it can misbehave it's good to know how.

[Lokathor]

I'll admit I didn't investigate the situation extremely closely at the time, but by mixing inline(always) with instruction_set the compiler ended up inlining a different instruction set function into the caller's code (specifically arm::a32 into arm::t32). I wouldn't have noticed at all except that the arm code contained inline assembly which had an instruction that's not legal in t32 and so a compilation error occurred. I'd heard of this being possible before and didn't investigate too much, i just removed the (always) and moved on.

All that said, while typing this message I remembered that this crate is only cfg based, so it would be entirely impossible for a similar problem to affect this crate. If the cpu feature isn't available in the whole compilation then it wouldn't even be visible to call. There's no danger of a mis-inline in this situation.

[HadrienG2]

Mmm... sounds bug-report-worthy to me if you reproduce it again someday. Searching a bit through rustc issues, there seems to have been a number of issues concerning funky soundness issues linked to LLVM inlining before, hopefully that was one of those that got fixed...

At least one of them happened in the presence of #[inline] though, and could in principle have happened without any inline directives should the LLVM inliner decide that a function is worth inlining.