[XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

[XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

[Federico Serafini]

Update ECLAIR configuration to take into account the deviations
agreed during MISRA meetings for Rule 16.4.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl |  8 ++++++++
 docs/misra/deviations.rst                        | 13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index d21f112a9b..f09ad71acf 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -384,6 +384,14 @@ explicit comment indicating the fallthrough intention is present."
 -config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
 -doc_end
 
+-doc_begin="Switch statements having a controlling expression of enum type deliberately do not have a default case: gcc -Wall enables -Wswitch which warns (and breaks the build as we use -Werror) if one of the enum labels is missing from the switch."
+-config=MC3R1.R16.4,reports+={deliberate,'any_area(kind(context)&&^.* has no `default.*$&&stmt(node(switch_stmt)&&child(cond,skip(__non_syntactic_paren_stmts,type(canonical(enum_underlying_type(any())))))))'}
+-doc_end
+
+-doc_begin="A switch statement with a single switch clause and no default label may be used in place of an equivalent if statement if it is considered to improve readability."
+-config=MC3R1.R16.4,switch_clauses+={deliberate,"switch(1)&&default(0)"}
+-doc_end
+
 -doc_begin="A switch statement with a single switch clause and no default label may be used in place of an equivalent if statement if it is considered to improve readability."
 -config=MC3R1.R16.6,switch_clauses+={deliberate, "default(0)"}
 -doc_end
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index ed0c1e8ed0..df87239b7d 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -302,6 +302,19 @@ Deviations related to MISRA C:2012 Rules:
        leave such files as is.
      - Tagged as `deliberate` for ECLAIR.
 
+   * - R16.4
+     - Switch statements having a controlling expression of enum type
+       deliberately do not have a default case: gcc -Wall enables -Wswitch
+       which warns (and breaks the build as we use -Werror) if one of the enum
+       labels is missing from the switch.
+     - Tagged as `deliberate` for ECLAIR.
+
+   * - R16.4
+     - A switch statement with a single switch clause and no default label may
+       be used in place of an equivalent if statement if it is considered to
+       improve readability."
+     - Tagged as `deliberate` for ECLAIR.
+
    * - R16.3
      - Switch clauses ending with continue, goto, return statements are safe.
      - Tagged as `safe` for ECLAIR.
-- 
2.34.1

Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

[Jan Beulich]

On 23.04.2024 12:02, Federico Serafini wrote:
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -302,6 +302,19 @@ Deviations related to MISRA C:2012 Rules:
>         leave such files as is.
>       - Tagged as `deliberate` for ECLAIR.
>  
> +   * - R16.4
> +     - Switch statements having a controlling expression of enum type
> +       deliberately do not have a default case: gcc -Wall enables -Wswitch
> +       which warns (and breaks the build as we use -Werror) if one of the enum
> +       labels is missing from the switch.
> +     - Tagged as `deliberate` for ECLAIR.
> +
> +   * - R16.4
> +     - A switch statement with a single switch clause and no default label may
> +       be used in place of an equivalent if statement if it is considered to
> +       improve readability."

First a terminology related comment here: I'm afraid "switch clause" can be
interpreted multiple ways, when I think we want to leave no room for
interpretation here. It's not even clear to me whether

    switch ( x )
    {
    case 1: case 2: case 3: case 4:
        ...
        break;
    }

would be covered by the deviation, or whether the multiple case labels
wouldn't already be too much.

And then it is not clear to me why

    switch ( x )
    {
    case 1:
        ...
        break;
    default:
        ...
        break;
    }

shouldn't also be covered, as potentially a readability improvement /
future change simplification over

    if ( x == 1 )
    {
        ...
    }
    else
    {
        ...
    }

Jan

Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

[Federico Serafini]

On 23/04/24 12:26, Jan Beulich wrote:
> On 23.04.2024 12:02, Federico Serafini wrote:
>> --- a/docs/misra/deviations.rst
>> +++ b/docs/misra/deviations.rst
>> @@ -302,6 +302,19 @@ Deviations related to MISRA C:2012 Rules:
>>          leave such files as is.
>>        - Tagged as `deliberate` for ECLAIR.
>>   
>> +   * - R16.4
>> +     - Switch statements having a controlling expression of enum type
>> +       deliberately do not have a default case: gcc -Wall enables -Wswitch
>> +       which warns (and breaks the build as we use -Werror) if one of the enum
>> +       labels is missing from the switch.
>> +     - Tagged as `deliberate` for ECLAIR.
>> +
>> +   * - R16.4
>> +     - A switch statement with a single switch clause and no default label may
>> +       be used in place of an equivalent if statement if it is considered to
>> +       improve readability."

(I placed Rule 16.4 before Rule 16.3.
I will propose a new version with the correct ordering.)

> 
> First a terminology related comment here: I'm afraid "switch clause" can be
> interpreted multiple ways, when I think we want to leave no room for
> interpretation here. It's not even clear to me whether
> 
>      switch ( x )
>      {
>      case 1: case 2: case 3: case 4:
>          ...
>          break;
>      }
> 
> would be covered by the deviation, or whether the multiple case labels
> wouldn't already be too much.

The MISRA C document, within Rule 16.1 ("A switch statement shall be
well-formed") defines the syntax rules that can be used to define a
"well formed" switch statement.
When I say "switch clause", I refer to the same entity the MISRA
document refers to in the definition of such syntax rules.
In the example above, we have a single switch clause with multiple
labels and no default label: this is a violation of Rule 16.4
("Every `switch' statement shall have a `default' label") which will
be covered by the deviation.
Do you think inserting the example in rules.rst or deviations.rst could
be useful?

> 
> And then it is not clear to me why
> 
>      switch ( x )
>      {
>      case 1:
>          ...
>          break;
>      default:
>          ...
>          break;
>      }
> 
> shouldn't also be covered, as potentially a readability improvement /
> future change simplification over
> 
>      if ( x == 1 )
>      {
>          ...
>      }
>      else
>      {
>          ...
>      }

Here there are two switch clauses,
each of them terminated by a break statement,
and the default label is present:
the switch is well formed, no violations of series 16 will
be reported.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)

Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

[Jan Beulich]

On 23.04.2024 17:52, Federico Serafini wrote:
> On 23/04/24 12:26, Jan Beulich wrote:
>> On 23.04.2024 12:02, Federico Serafini wrote:
>>> --- a/docs/misra/deviations.rst
>>> +++ b/docs/misra/deviations.rst
>>> @@ -302,6 +302,19 @@ Deviations related to MISRA C:2012 Rules:
>>>          leave such files as is.
>>>        - Tagged as `deliberate` for ECLAIR.
>>>   
>>> +   * - R16.4
>>> +     - Switch statements having a controlling expression of enum type
>>> +       deliberately do not have a default case: gcc -Wall enables -Wswitch
>>> +       which warns (and breaks the build as we use -Werror) if one of the enum
>>> +       labels is missing from the switch.
>>> +     - Tagged as `deliberate` for ECLAIR.
>>> +
>>> +   * - R16.4
>>> +     - A switch statement with a single switch clause and no default label may
>>> +       be used in place of an equivalent if statement if it is considered to
>>> +       improve readability."
> 
> (I placed Rule 16.4 before Rule 16.3.
> I will propose a new version with the correct ordering.)
> 
>>
>> First a terminology related comment here: I'm afraid "switch clause" can be
>> interpreted multiple ways, when I think we want to leave no room for
>> interpretation here. It's not even clear to me whether
>>
>>      switch ( x )
>>      {
>>      case 1: case 2: case 3: case 4:
>>          ...
>>          break;
>>      }
>>
>> would be covered by the deviation, or whether the multiple case labels
>> wouldn't already be too much.
> 
> The MISRA C document, within Rule 16.1 ("A switch statement shall be
> well-formed") defines the syntax rules that can be used to define a
> "well formed" switch statement.
> When I say "switch clause", I refer to the same entity the MISRA
> document refers to in the definition of such syntax rules.
> In the example above, we have a single switch clause with multiple
> labels and no default label: this is a violation of Rule 16.4
> ("Every `switch' statement shall have a `default' label") which will
> be covered by the deviation.
> Do you think inserting the example in rules.rst or deviations.rst could
> be useful?

No, I don't think there should be examples in those documents. But those
documents should also not (blindly) rely on terminology in the Misra
spec, as not everyone has access to that (licensed copies had to be
obtained for quite a few of us).

Jan

Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

[Federico Serafini]

On 23/04/24 18:06, Jan Beulich wrote:
> On 23.04.2024 17:52, Federico Serafini wrote:
>> On 23/04/24 12:26, Jan Beulich wrote:
>>> On 23.04.2024 12:02, Federico Serafini wrote:
>>>> +
>>>> +   * - R16.4
>>>> +     - A switch statement with a single switch clause and no default label may
>>>> +       be used in place of an equivalent if statement if it is considered to
>>>> +       improve readability."
> 
> No, I don't think there should be examples in those documents. But those
> documents should also not (blindly) rely on terminology in the Misra
> spec, as not everyone has access to that (licensed copies had to be
> obtained for quite a few of us).

In deviations.rst there is an identical deviation for Rule 16.6
("Every switch statement shall have at least two switch-clauses").
I think we should remain consistent.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)

Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

[Jan Beulich]

On 24.04.2024 09:37, Federico Serafini wrote:
> On 23/04/24 18:06, Jan Beulich wrote:
>> On 23.04.2024 17:52, Federico Serafini wrote:
>>> On 23/04/24 12:26, Jan Beulich wrote:
>>>> On 23.04.2024 12:02, Federico Serafini wrote:
>>>>> +
>>>>> +   * - R16.4
>>>>> +     - A switch statement with a single switch clause and no default label may
>>>>> +       be used in place of an equivalent if statement if it is considered to
>>>>> +       improve readability."
>>
>> No, I don't think there should be examples in those documents. But those
>> documents should also not (blindly) rely on terminology in the Misra
>> spec, as not everyone has access to that (licensed copies had to be
>> obtained for quite a few of us).
> 
> In deviations.rst there is an identical deviation for Rule 16.6
> ("Every switch statement shall have at least two switch-clauses").
> I think we should remain consistent.

Sure, I'm all for consistency. Yet given the term "switch clause" doesn't
appear in the C standard (afaics), it wants defining somewhere.

Jan