From 9024427afc55e6a59bd99e1dd4fedc95c7a5b9f0 Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Sun, 23 Mar 2025 05:06:08 +0900 Subject: [PATCH 01/15] Add declarative macro guideline: avoid specializing --- src/coding-guidelines/macros.rst | 153 +++++++++++++++++++++++++++++-- 1 file changed, 146 insertions(+), 7 deletions(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index 91bd38b..4fa3d82 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -6,15 +6,13 @@ Macros ====== -.. guideline:: Shall not use Declarative Macros - :id: gui_h0uG1C9ZjryA - :category: mandatory +.. guideline:: Avoid specialized, fixed patterns within declarative macros + :id: gui_FSpI084vbwmJ :status: draft - :release: todo - :fls: fls_xa7lp0zg1ol2 - :decidability: decidable - :scope: system + :fls: fls_w44hav7mw3ao :tags: reduce-human-error + :category: macros + :recommendation: encouraged Description of the guideline goes here. @@ -302,3 +300,144 @@ Macros // Compliant implementation } +.. guideline:: Avoid specialized, fixed patterns within declarative macros + :id: gui_FSpI084vbwmJ + :status: draft + :fls: fls_w44hav7mw3ao + :tags: reduce-human-error + :category: macros + :recommendation: encouraged + + Description of the guideline goes here. + + .. rationale:: + + :id: rat_zqr9uEqP6nzW + :status: draft + + It is common to use declarative macros to implement traits which would + otherwise involve repetitive code. + + In a declarative macro the ordering of the patterns will be the order that + they are matched against which can lead to unexpected behavior in the case + where we have unique behavior intended for a particular expression. + + If needing to specialize logic within the macro based on a particular + expression's value, it may be better to not use a declarative macro. + + Limitation: Note that following this rule means that we are unable to support + variadic declarative macros with one or more arguments. + + .. bad_example:: + :id: bad_ex_5vK0CCmePkef + :status: draft + + We have two macro match rules at the same level of nesting. Since the + matching is done macro rule by macro rule and this process is stopped as soon + as a macro matcher is matched we will not reach the specialized case for EmergencyValve. + + .. 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); + + .. good_example:: + :id: good_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); From 8e252eb6323faeed0ad63a681e05b503057800c3 Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Sun, 23 Mar 2025 23:51:06 +0900 Subject: [PATCH 02/15] Updated with Julius' feedback --- src/coding-guidelines/macros.rst | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index 4fa3d82..e5f87d9 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -315,8 +315,9 @@ Macros :id: rat_zqr9uEqP6nzW :status: draft - It is common to use declarative macros to implement traits which would - otherwise involve repetitive code. + 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. In a declarative macro the ordering of the patterns will be the order that they are matched against which can lead to unexpected behavior in the case @@ -325,9 +326,6 @@ Macros If needing to specialize logic within the macro based on a particular expression's value, it may be better to not use a declarative macro. - Limitation: Note that following this rule means that we are unable to support - variadic declarative macros with one or more arguments. - .. bad_example:: :id: bad_ex_5vK0CCmePkef :status: draft From 426142eb1bbbaa111b280005c26847b4ced0033c Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Sat, 29 Mar 2025 23:58:19 +0900 Subject: [PATCH 03/15] Update macro coding guideline to fit compliant / non-compliant change and new options --- src/coding-guidelines/macros.rst | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index e5f87d9..bf0fde8 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -8,11 +8,12 @@ Macros .. guideline:: Avoid specialized, fixed patterns within declarative macros :id: gui_FSpI084vbwmJ + :category: required :status: draft :fls: fls_w44hav7mw3ao + :decidability: decidable + :scope: module :tags: reduce-human-error - :category: macros - :recommendation: encouraged Description of the guideline goes here. @@ -326,8 +327,8 @@ Macros If needing to specialize logic within the macro based on a particular expression's value, it may be better to not use a declarative macro. - .. bad_example:: - :id: bad_ex_5vK0CCmePkef + .. non_compliant_example:: + :id: non_compl_ex_5vK0CCmePkef :status: draft We have two macro match rules at the same level of nesting. Since the @@ -383,8 +384,8 @@ Macros impl_safety_trait!(PressureSensor); impl_safety_trait!(TemperatureSensor); - .. good_example:: - :id: good_ex_ILBlY8DKB6Vs + .. compliant_example:: + :id: compl_ex_ILBlY8DKB6Vs :status: draft For the specialized implementation we implement the trait directly. From 0e74043ed94bc0970513c5464ec9d3bd3e6c3c97 Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Sat, 29 Mar 2025 11:05:38 -0400 Subject: [PATCH 04/15] Update wording in rationale Co-authored-by: Jordan McQueen --- src/coding-guidelines/macros.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index bf0fde8..105fc34 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -331,9 +331,9 @@ Macros :id: non_compl_ex_5vK0CCmePkef :status: draft - We have two macro match rules at the same level of nesting. Since the - matching is done macro rule by macro rule and this process is stopped as soon - as a macro matcher is matched we will not reach the specialized case for EmergencyValve. + 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. .. code-block:: rust From 2260143f3e3e7b7d2255645526c6d0962809fd5c Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Sat, 29 Mar 2025 11:06:19 -0400 Subject: [PATCH 05/15] Add guideline overview Co-authored-by: Jordan McQueen --- src/coding-guidelines/macros.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index 105fc34..12c8070 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -15,7 +15,10 @@ Macros :scope: module :tags: reduce-human-error - Description of the guideline goes here. + Matchers within macro rules are evaluated sequentially and short-circuit on + the first match. If a specialized fixed matcher follows a broader matcher, + it may be unreachable. This can lead to subtle and surprising bugs. It is + encouraged to avoid the use of specialized, fixed matchers. .. rationale:: :id: rat_U3AEUPyaUhcb From ad57b1d0523402e53daa58ff60cfe3644e6af7d1 Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Sat, 29 Mar 2025 11:09:08 -0400 Subject: [PATCH 06/15] Fix whitespace formatting Co-authored-by: Jordan McQueen --- src/coding-guidelines/macros.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index 12c8070..42134a3 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -320,8 +320,8 @@ Macros :status: draft 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. + implementations. It's possible to use derive macros or declarative macros + to do so. In a declarative macro the ordering of the patterns will be the order that they are matched against which can lead to unexpected behavior in the case From 3888e67e5fe571ea7ad04ea876d150397b772889 Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Sun, 30 Mar 2025 00:15:13 +0900 Subject: [PATCH 07/15] Incorporate Sasha's feedback to clarify ordering is not what matters in non-compliant example --- src/coding-guidelines/macros.rst | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index 42134a3..bbabc72 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -328,7 +328,7 @@ Macros where we have unique behavior intended for a particular expression. If needing to specialize logic within the macro based on a particular - expression's value, it may be better to not use a declarative macro. + expression's value, it is better to not use a declarative macro. .. non_compliant_example:: :id: non_compl_ex_5vK0CCmePkef @@ -338,6 +338,11 @@ Macros 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)] From c2fa75646a9155afc8d6bfa15db02b0e07e9144b Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Mon, 21 Apr 2025 23:59:09 +0900 Subject: [PATCH 08/15] Add release option to comply --- src/coding-guidelines/macros.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index bbabc72..c7813f8 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -10,6 +10,7 @@ Macros :id: gui_FSpI084vbwmJ :category: required :status: draft + :release: 1.85.0;1.85.1 :fls: fls_w44hav7mw3ao :decidability: decidable :scope: module From f91cd2a715e1ec1bc53cdb5e82129f8628495c1c Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Mon, 28 Apr 2025 13:13:39 -0400 Subject: [PATCH 09/15] Clarify using multiple macros for specialization purposes is fine! Co-authored-by: Lukas Wirth --- src/coding-guidelines/macros.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index c7813f8..bf926db 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -329,7 +329,7 @@ Macros where we have unique behavior intended for a particular expression. If needing to specialize logic within the macro based on a particular - expression's value, it is better to not use a declarative macro. + expression's value, it is better to not use a declarative macro with multiple rules. .. non_compliant_example:: :id: non_compl_ex_5vK0CCmePkef From 5cd15f41d96f089a29c93b4ee6f9833cc742e7a8 Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Mon, 28 Apr 2025 13:14:54 -0400 Subject: [PATCH 10/15] Clarify it's possible to use procedural macros more generally to accomplish Co-authored-by: Lukas Wirth --- src/coding-guidelines/macros.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index bf926db..30e1951 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -321,7 +321,7 @@ Macros :status: draft 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 + implementations. It's possible to use procedural macros or declarative macros to do so. In a declarative macro the ordering of the patterns will be the order that From 879d93d8315845dbb7ea9573b419310fc82fbc33 Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Tue, 29 Apr 2025 02:17:09 +0900 Subject: [PATCH 11/15] Add further rationale behind why no specialization in multiple macro match rules for issues that could come up during refactors. --- src/coding-guidelines/macros.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index 30e1951..81b4680 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -326,7 +326,10 @@ Macros In a declarative macro the ordering of the patterns will be the order that they are matched against which can lead to unexpected behavior in the case - where we have unique behavior intended for a particular expression. + where we have unique behavior intended for a particular expression. The concern + in particular is that while the 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. From 7d11cb887998bc99ca7af622a077379068a1e008 Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Thu, 8 May 2025 05:26:26 +0900 Subject: [PATCH 12/15] Fixup after rebase --- src/coding-guidelines/macros.rst | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index 81b4680..f15b464 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -6,20 +6,17 @@ Macros ====== -.. guideline:: Avoid specialized, fixed patterns within declarative macros - :id: gui_FSpI084vbwmJ - :category: required +.. guideline:: Shall not use Declarative Macros + :id: gui_h0uG1C9ZjryA + :category: mandatory :status: draft - :release: 1.85.0;1.85.1 - :fls: fls_w44hav7mw3ao + :release: todo + :fls: fls_xa7lp0zg1ol2 :decidability: decidable - :scope: module + :scope: system :tags: reduce-human-error - Matchers within macro rules are evaluated sequentially and short-circuit on - the first match. If a specialized fixed matcher follows a broader matcher, - it may be unreachable. This can lead to subtle and surprising bugs. It is - encouraged to avoid the use of specialized, fixed matchers. + Description of the guideline goes here. .. rationale:: :id: rat_U3AEUPyaUhcb @@ -31,7 +28,7 @@ Macros :id: non_compl_ex_Gb4zimei8cNI :status: draft - Explanation of code example. + (example of a simple expansion using a proc-macro) .. code-block:: rust @@ -43,7 +40,7 @@ Macros :id: compl_ex_Pw7YCh4Iv47Z :status: draft - Explanation of code example + (example of the same simple expansion using a declarative macro) .. code-block:: rust @@ -307,16 +304,17 @@ Macros .. 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 - :category: macros - :recommendation: encouraged Description of the guideline goes here. .. rationale:: - :id: rat_zqr9uEqP6nzW :status: draft From 00d92688cd3bca14b9f5490ddec26c631f907949 Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Thu, 8 May 2025 05:33:54 +0900 Subject: [PATCH 13/15] Replace explanation of ordering with one from PR feedback --- src/coding-guidelines/macros.rst | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index f15b464..9e01f61 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -322,16 +322,20 @@ Macros implementations. It's possible to use procedural macros or declarative macros to do so. - In a declarative macro the ordering of the patterns will be the order that - they are matched against which can lead to unexpected behavior in the case - where we have unique behavior intended for a particular expression. The concern - in particular is that while the ordering may be done correctly when the - macro match rules are written, it's possible in a refactor for them to + 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 From efcce347bdf9933df51de57b89adcdc145e387cf Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Wed, 7 May 2025 16:40:19 -0400 Subject: [PATCH 14/15] Fix mistakes during rebase --- src/coding-guidelines/macros.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index 9e01f61..5b6b112 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -28,7 +28,7 @@ Macros :id: non_compl_ex_Gb4zimei8cNI :status: draft - (example of a simple expansion using a proc-macro) + Explanation of code example. .. code-block:: rust From 640c28be5a146a7032683105acc3d575e1002aaf Mon Sep 17 00:00:00 2001 From: Pete LeVasseur Date: Wed, 7 May 2025 16:40:44 -0400 Subject: [PATCH 15/15] Fix mistakes during rebase --- src/coding-guidelines/macros.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coding-guidelines/macros.rst b/src/coding-guidelines/macros.rst index 5b6b112..55c6ba1 100644 --- a/src/coding-guidelines/macros.rst +++ b/src/coding-guidelines/macros.rst @@ -40,7 +40,7 @@ Macros :id: compl_ex_Pw7YCh4Iv47Z :status: draft - (example of the same simple expansion using a declarative macro) + Explanation of code example .. code-block:: rust