Skip to content

Add declarative macro guideline: avoid specializing #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 152 additions & 0 deletions src/coding-guidelines/macros.rst
Original file line number Diff line number Diff line change
Expand Up @@ -302,3 +302,155 @@ Macros
// Compliant implementation
}

.. guideline:: Avoid specialized, fixed patterns within declarative macros
:id: gui_FSpI084vbwmJ
:category: macros
:status: draft
:release: todo
:fls: fls_w44hav7mw3ao
:decidability: decidable
:scope: system
:tags: reduce-human-error

Description of the guideline goes here.

.. rationale::
:id: rat_zqr9uEqP6nzW
:status: draft

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

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.

The concern in particular is that while the declaration ordering may be done correctly
when the macro match rules are written, it's possible in a refactor for them to
unintentionally be moved around in order.

If needing to specialize logic within the macro based on a particular
expression's value, it is better to not use a declarative macro with multiple rules.

.. _transcriber: https://doc.rust-lang.org/reference/macros-by-example.html

.. non_compliant_example::
:id: non_compl_ex_5vK0CCmePkef
:status: draft

We have two macro match rules at the same level of nesting. Since macro
matching is done sequentially through the matchers and stops at the first
match, the specialized case for EmergencyValve is unreachable.

The example would also be non-compliant if the ordering of the matchers
were reversed as this introduces the possibility of future human-error
when refactoring the macro to place the specialized matcher after the
generic matcher.

.. code-block:: rust

#[derive(Debug)]
enum SafetyLevel {
Green,
Yellow,
Red
}

trait SafetyCheck {
fn verify(&self) -> SafetyLevel;
}

// Different device types that need safety checks
struct PressureSensor {/* ... */}
struct TemperatureSensor {/* ... */}
struct EmergencyValve {
open: bool,
}

// This macro has a pattern ordering issue
macro_rules! impl_safety_trait {
// Generic pattern matches any type - including EmergencyValve
($t:ty) => {
impl SafetyCheck for $t {
fn verify(&self) -> SafetyLevel {
SafetyLevel::Green
}
}
};

// Special pattern for EmergencyValve - but never gets matched
(EmergencyValve) => {
impl SafetyCheck for EmergencyValve {
fn verify(&self) -> SafetyLevel {
// Emergency valve must be open for safety
if !self.open {
SafetyLevel::Red
} else {
SafetyLevel::Green
}
}
}
};
}
impl_safety_trait!(EmergencyValve);
impl_safety_trait!(PressureSensor);
impl_safety_trait!(TemperatureSensor);

.. compliant_example::
:id: compl_ex_ILBlY8DKB6Vs
:status: draft

For the specialized implementation we implement the trait directly.

If we wish to use a declarative macro for a certain generic implementation
we are able to do this. Note there is a single macro rule at the level of
nesting within the declarative macro.

.. code-block:: rust

#[derive(Debug)]
enum SafetyLevel {
Green,
Yellow,
Red
}

trait SafetyCheck {
fn verify(&self) -> SafetyLevel;
}

// Different device types that need safety checks
struct PressureSensor {/* ... */}
struct TemperatureSensor {/* ... */}
struct EmergencyValve {
open: bool,
}

// Direct implementation for EmergencyValve
impl SafetyCheck for EmergencyValve {
fn verify(&self) -> SafetyLevel {
// Emergency valve must be open for safety
if !self.open {
SafetyLevel::Red
} else {
SafetyLevel::Green
}
}
}

// Use generic implementation for those without
// special behavior
macro_rules! impl_safety_traits_generic {
// Generic pattern for other types
($t:ty) => {
impl SafetyCheck for $t {
fn verify(&self) -> SafetyLevel {
SafetyLevel::Green
}
}
};
}
impl_safety_traits_generic!(PressureSensor);
impl_safety_traits_generic!(TemperatureSensor);