Add declarative macro guideline: avoid specializing

[PLeVasseur]

No description provided.

[PLeVasseur]

Hey @pellico, @vccjgust -- I tried to mock up a coding guideline for declarative macros.

Could y'all take a look when able?

[jgust]

Just a few initial comments. I will continue my review tomorrow. Good work so far.

[jgust]

I would say that a derive macro is the more idiomatic way of generating trait implementations, while a declarative macro is more suitable as a syntax extension mechanism as well as for generating boiler plate code outside of places where derive macros are usable (trait implementations can of course be considered as boiler plate at times).

Perhaps we should phrase this so that we don't give the impression that generating trait implementations via macro_rules! is somehow a preferred or recommended way?

[PLeVasseur]

Good point. I think that a derive macro will often be the more idiomatic thing. There is a bit of trade-off to be had here where that requires bringing in external crates (proc-macro2, syn, quote) which could make safety-qualification more challenging.

I think this gets into the discussion we had in the last coding guidelines meeting that Rust is a sophisticated language with a number of ways to do things.

I could see rephrasing this to the below to avoid biasing one way or another in this guideline:

It's common to use macros to avoid writing repetitive code, such as trait implementations. It's possible to use derive macros or declarative macros to do so.

I think we could also have another guideline which said just what you pointed out that we'd recommend using a derive macro for repetitive trait implementation, if the additional external crates are not of issue. What do you think?

[manthonyaiello]

I think that we should actively avoid anything in the coding standard that would impose a requirement on additional crates. libcore/liballoc/libstd are already very large from a runtime-to-be-certified point of view. And speaking on behalf of AdaCore, as a Rust toolchain vendor, support for 3rd-party crates is at least several years out on our roadmap - to say nothing of certification of 3rd-party crates.

[PLeVasseur]

Thanks for the feedback @manthonyaiello! I feel like we're in one of those "depends on your context" scenarios.

This coding guideline is setting out to suggest something for declarative macros specifically, which would be in libcore.

How do you feel about the text that's there currently more as background in this coding guideline?

It's common to use macros to avoid writing repetitive code, such as trait implementations. It's possible to use derive macros or declarative macros to do so.

On another note, we could have another coding guideline we can give rationale on the circumstances:

• if you need safety-certification, probably stick to libcore and declarative macros for use cas of DRY
• if you are comfortable without safety-certification, derive macros can offer a bit more ergonomics

[manthonyaiello]

If the purpose of the coding guideline/standard is overall safety-critical use cases, then I think certification can/should be assumed and we should focus on those needs. I would lean away from recommending alternative approaches until/unless they can be met by a certifiable/certified runtime. Just my 2¢.

[PLeVasseur]

Gotcha, thanks for sharing. We'll take that into consideration in this week's coding guidelines meeting as a criteria. Not planning to merge this until we review live in that meeting and more review accumulates.

[jgust]

My point was not intended to advocate for one approach over another, but rather that we should avoid giving the impression that a non-idiomatic way is recommended. I know it was not the intention but the original text could maybe have been interpreted that way, was my concern.

We can discuss it at the meeting how to best approach these types of situations where the idiomatic way is perhaps more difficult to certify since that is obviously an important point. But just a note: neither declarative or derive macros impose any additional run-time components on their own since both are compile-time code generation mechanisms. But the latter method adds (in practice) additional "semi-standard" third party crates as Pete pointed out.

[PLeVasseur]

Yup, this is going to take some nuance. If either of @jgust or @pellico would like to write a little bit of an introduction to this chapter to flesh that out it'd be great!

[scrabsha]

Do we want to warn against this as well:

macro_rules! impl_safety_trait {
    // Special pattern for EmergencyValve
    (EmergencyValve) => {
        impl SafetyCheck for EmergencyValve {
            fn verify(&self) -> SafetyLevel {
                // Emergency valve must be open for safety
                if !self.open {
                    SafetyLevel::Red
                } else {
                    SafetyLevel::Green
                }
            }
        }
    };
    // Generic pattern matches any type - including EmergencyValve
    ($t:ty) => {
        impl SafetyCheck for $t {
            fn verify(&self) -> SafetyLevel {
                SafetyLevel::Green
            }
        }
    };
}

(The two rules have been swapped)

Both macros (the one from the PR and mine) use a specialized pattern, but your example suggests the code is bad because the specialized rule is dead code.

Could we either:

• (If we intend to forbid the example from my comment) Add a sentence that explicitly says that this rule applies regardless of the ordering of the rule
• (If we intend to allow the example from my comment) state that this guideline only applies to specialized patterns that are dead code (and maybe change the guideline name)

[PLeVasseur]

Do we want to warn against this as well:
... snip ...
Both macros (the one from the PR and mine) use a specialized pattern, but your example suggests the code is bad because the specialized rule is dead code.

Good catch!

Could we either:

• (If we intend to forbid the example from my comment) Add a sentence that explicitly says that this rule applies regardless of the ordering of the rule
• (If we intend to allow the example from my comment) state that this guideline only applies to specialized patterns that are dead code (and maybe change the guideline name)

I would rather prevent all specialized patterns for the sake of preventing accidents from occurring when someone is refactoring the code / adding new macro rule and so on. What do y'all think?

(By the way the code example is intended to showcase what can go wrong if the guideline is not followed, not be exhaustive in my opinion. I suppose I could be explicit that it doesn't matter on ordering, it's always disallowed if we think necessary?)

[pellico]

I am fine with present rule but I don't feel comfortable to start with a very specific rule with a little bigger context.
I think it would be helpful some direct discussion to align on general concept direction.
I just try to write shortly some general consideration about macros in safety context.
I think that macros can be abused and lead to a code that is difficult to test and review.

Declarative macro

Pro

easy to write, little compilation cost

Cons

If they are too complex, it is difficult to review them.
If you use cargo expand, the generated could could be not compiled.
How to measure test coverage ?

Procedural macro

Pro

Expanded code can be compiled
Easier to measure coverage
Easier to create tests for procedural macro itself.

Cons

As reported by @PLeVasseur and confirmed by a compiler vendor, procedural macro can be an issue in compiler assessment. I think watt could be a solution for that.
Slow compilation time.

Some general ideas for guidelines

Declaration macros

Declarative macros should be short and easily understandable. --> Create some complexity index, line of code, number of arguments etc...
No recursive macros
Test coverage shall be measured on generated code (is this possible ?)

Procedural macros

Procedural macro package shall be tested and qualified standalone. If it is not possible to qualify the package for the appropriate level the generated code shall be reviewed and then compiled.
Generated code of procedural macro shall follow the safety coding guidelines as for human written code. (Rationale: It should be reviewed by a human)
No recursive macro
Test coverage shall be measured on generated code (I think this should be easily achievable)

[PLeVasseur]

I am fine with present rule but I don't feel comfortable to start with a very specific rule with a little bigger context. I think it would be helpful some direct discussion to align on general concept direction. I just try to write shortly some general consideration about macros in safety context. I think that macros can be abused and lead to a code that is difficult to test and review.
... snip ...

These are great considerations! Putting an idea out there -- perhaps @pellico you could "scaffold out" some of these thoughts on the lead-in section of the Macros chapter before we get to the guidelines themselves as a PR we can review. What do you think?

[jgust]

Do we want to warn against this as well:
... snip ...
Both macros (the one from the PR and mine) use a specialized pattern, but your example suggests the code is bad because the specialized rule is dead code.

Good catch!

Could we either:

• (If we intend to forbid the example from my comment) Add a sentence that explicitly says that this rule applies regardless of the ordering of the rule
• (If we intend to allow the example from my comment) state that this guideline only applies to specialized patterns that are dead code (and maybe change the guideline name)

I would rather prevent all specialized patterns for the sake of preventing accidents from occurring when someone is refactoring the code / adding new macro rule and so on. What do y'all think?

(By the way the code example is intended to showcase what can go wrong if the guideline is not followed, not be exhaustive in my opinion. I suppose I could be explicit that it doesn't matter on ordering, it's always disallowed if we think necessary?)

It would be good if we can prevent these kind of specialized patterns, but I suspect you can still run into cases where you have two or more generic patterns where one is still a subset of the other so that match order is important. But wouldn't clippy or the compiler be able to warn about that in that case? Perhaps an additional guideline could be to always provide a unit test that ensures that all patterns are being matched properly?

[jgust]

I am fine with present rule but I don't feel comfortable to start with a very specific rule with a little bigger context. I think it would be helpful some direct discussion to align on general concept direction. I just try to write shortly some general consideration about macros in safety context. I think that macros can be abused and lead to a code that is difficult to test and review.
... snip ...

These are great considerations! Putting an idea out there -- perhaps @pellico you could "scaffold out" some of these thoughts on the lead-in section of the Macros chapter before we get to the guidelines themselves as a PR we can review. What do you think?

I agree, very good considerations! I think we should also mention (either in the lead-in or even as its own guideline), that since writing macros is a relatively rare event, compared to writing normal Rust code, and it has slightly different syntax it is important to properly document each match pattern, where the expected input and corresponding intended output is clarified. This helps both during review as well when you come back to a macro months later and have already forgetting everything about macro writing :-) What do think?

[PLeVasseur]

Perhaps an additional guideline could be to always provide a unit test that ensures that all patterns are being matched properly?

This seems like a very practical solution. I like it!

[adfernandes]

@PLeVasseur Very nice - thank you - and the commentary here is great!

One small sentence I would suggest adding at the very top is a quick "Rust macros are not C-like preprocessor macros"

The salient points are:

• Rust macros are hygenic, meaning that Rust's macro system prevents errors due to names introduced within a macro
• Rust macros operate on streams of typed tokens as part of the AST, as opposed to the un-typed token streams that the C preprocessor operates on

(Well, one sentence and two bullet points 😆)

I think those two points are important because a lot of people have "PTSD"-like responses from past C-macro insanity, and we want to emphasize that "Rust macro" and "C macro" are as equivalent as "car" and "carpet".

[PLeVasseur]

One small sentence I would suggest adding at the very top is a quick "Rust macros are not C-like preprocessor macros"
... snip ...
I think those two points are important because a lot of people have "PTSD"-like responses from past C-macro insanity, and we want to emphasize that "Rust macro" and "C macro" are as equivalent as "car" and "carpet".

I think that's fair to add as some preamble ahead of the guidelines proper. I need to read the The Little Book of Rust Macros more to be able to capture the nuance here probably.

I had read for example that declarative macros are partially hygienic recently for example, link.

Perhaps we should provide links to these sorts of resources in each chapter for more details. What do you y'all think?

[pellico]

@PLeVasseur Very nice - thank you - and the commentary here is great!

One small sentence I would suggest adding at the very top is a quick "Rust macros are not C-like preprocessor macros"

The salient points are:

• Rust macros are hygenic, meaning that Rust's macro system prevents errors due to names introduced within a macro
• Rust macros operate on streams of typed tokens as part of the AST, as opposed to the un-typed token streams that the C preprocessor operates on

(Well, one sentence and two bullet points 😆)

I think those two points are important because a lot of people have "PTSD"-like responses from past C-macro insanity, and we want to emphasize that "Rust macro" and "C macro" are as equivalent as "car" and "carpet".

Definitely declarative macros are "safer" than C macros but procedural macro has greater power than C macro. It can take a piece of code and transform it in a completely different and surprising code. Bad procedural macro can create more damage than C macro. I think that in functional safety scope the usage of macros shall be restricted a lot. Doesn't from great power come a great responsibility ?
Moreover I think that there is a tendency to use too much procedural macro see below picture from lib.rs site.

[jgust]

Definitely declarative macros are "safer" than C macros but procedural macro has greater power than C macro. It can take a piece of code and transform it in a completely different and surprising code. Bad procedural macro can create more damage than C macro. I think that in functional safety scope the usage of macros shall be restricted a lot. Doesn't from great power come a great responsibility ?

I agree, it should be used with care. But, it is relatively straight forward (and could probably be improved even further) to see what a given macro generates as its output (i.e. the actual code that goes into the binary) from a given input. I would say the focus should be on coming up with guidelines and procedures that give use maximal insight and control of the code being generated to ensure that it conforms to the same safety/quality goals as the handwritten code.

[adfernandes]

@PLeVasseur Nice catch on the partial hygenicity of macros!

@pellico I absolutely understand that macros can be abused, and I've certainly seen instances where they have been abused, but I'm not certain that things like "library percent" are great metrics for "good vs bad".

For example, the std library has a lot of instances of unsafe, much more than your "usual" Rust crate, but that's because the std library encapsulates tricky things so you don't have to deal with it.

Consider a proc_macro like the following: midpoint!(x, y). It computes (x + y)/2), which, seems just fine... except it has a hidden integer overflow.

Or another example: hypotenuse!(x, y) computed via sqrt(x*x + y*y) for floats, which has overflow and underflow for floats, which can be tricky to find.

Those are "bad" macros because the underlying algorithms are subtly incorrect, and that fact might be hidden somewhere in "macro language", not canonical Rust.

What about bake_a_pizza!() that actually tries to bake two pizzas and calls release_the_kraken()? That's a "bad" macro because it's name is misleading, and it has a surprising, global, side-effect. One does not expect to release the Kraken when one wants to simply bake a pizza.

Instead, what about guidelines along the lines of:

• Are the macros clear and unambiguous?
• Are they correctly and thoroughly and exhaustively documented?
• Are there side effects? If so, what scope? Local? Crate? "Global"?
• Are they thoroughly tested and validated?

These are much easier goals for macros in Rust vs macros in C!

[pellico]

@adfernandes

I absolutely understand that macros can be abused, and I've certainly seen instances where they have been abused, but I'm not certain that things like "library percent" are great metrics for "good vs bad".

I agree that the "library percent" is not a metric for "good vs bad". I just want to highlight that I have perception that devs in Rust use more macros than in C probably because they are more powerful.

In the C code that I read (typically quite low level) there are a tendency to avoid as much as possible macros because it mostly acknowledged that they are evil.

[jmqd]

Some in-line suggestions; I leave it up to your judgment on whether you agree / like the suggestions.

I want to think a bit on the code example. I have a feeling we could find a better example, but nothing comes to mind off the bat. Compelling code examples are indeed tricky out of context.

[PLeVasseur]

Thanks for the suggestions @jmqd! Incorporated them.

[PLeVasseur]

@scrabsha -- I've incorporated your feedback in commit 0fda. Thanks for calling this out.

[PLeVasseur]

I think I've addressed all open feedback to the PR. Looking for final reviews before merging!

[Veykril]

In a declarative macro the ordering of the patterns will be the order that they are matched against

This doesn't really say anything. What is the order of the patterns? As in, what do they express. I assume you are intending to describe the selection process of which rule is selected by the compiler? I'd rewrite this paragraph (not quite sure how) as the current writing is rather confusing to me (and doesn't really say much)

[PLeVasseur]

@Veykril -- you're right that this is the attempt made here to describe:

I assume you are intending to describe the selection process of which rule is selected by the compiler?

Could I have your help with a proposed wording here?

[Veykril]

The choice of which transcriber is being used in a declarative macro depends on their declaration order within the macro. This can lead to unexpected behavior changes in invocations of declarative macros if a new transciber is inserted before another due to invocations suddenly matching a different transcriber.

[PLeVasseur]

TIL that part is called a transcriber!

[PLeVasseur]

@Veykril -- I updated with your suggested replacement using transcriber instead. Could you take a look?