Add guideline for issue #174

Co-authored-by: felix91gr <[email protected]>

Author: 2 people

src/coding-guidelines/expressions.rst

@@ -399,3 +399,144 @@ Expressions
           /* ... */
         }
 
+
+.. guideline:: Do not shift an expression by a negative number of bits or by greater than or equal to the number of bits that exist in the operand
+    :id: gui_8BiWvapv0lUa 
+    :category: mandatory
+    :status: draft
+    :release: 1.0.0-latest
+    :fls: fls_sru4wi5jomoe
+    :decidability: undecidable
+    :scope: module
+    :tags: numerics, surprising-behavior, defect
+
+    In particular, the user should limit the Right Hand Side (RHS) parameter used for left shifts and right shifts (i.e. the ``<<`` and ``>>`` binary operators) to only the range ``0..=N-1``\ , where ``N`` is the number of bits of the Left Hand Side (LHS) parameter. For example, in ``a << b``\ , if ``a`` is of type ``u32``\ , then ``b`` **must belong to** the range ``0..=31``.
+
+    This rule applies to all types which implement the ``core::ops::Shl`` and / or ``core::ops::Shr`` traits, for Rust Version greater than or equal to ``1.6.0``.
+
+    For versions prior to ``1.6.0``\ , this rule applies to all types for which the ``<<`` and ``>>`` operators are valid. That is, it applies to the following primitive types:
+
+
+    * ``i8``
+    * ``i16``
+    * ``i32``
+    * ``i64``
+    * ``i128``
+    * ``isize``
+    * ``u8``
+    * ``u16``
+    * ``u32``
+    * ``u64``
+    * ``u128``
+    * ``usize``
+
+    .. rationale:: 
+        :id: rat_aMguLhw0ORnD 
+        :status: draft
+
+        This is a Defect Avoidance rule, directly inspired by `INT34-C. Do not shift an expression by a negative number of bits or by greater than or equal to the number of bits that exist in the operand <https://wiki.sei.cmu.edu/confluence/x/ItcxBQ>`_.
+
+        In Rust these out-of-range shifts don't give rise to Undefined Behavior; however, they are still problematic in Safety Critical contexts for two reasons.
+
+        Reason 1: inconsistent behavior
+        ===============================
+
+        The behavior of shift operations depends on the compilation mode. Say for example, that we have a number ``x`` of type ``uN``\ , and we perform the operation
+
+        ``x << M`` 
+
+        Then, it will behave like this:
+
+        .. list-table::
+           :header-rows: 1
+
+           * - **Compilation Mode**
+             - **\ ``0 <= M < N``\ **
+             - **\ ``M < 0``\ **
+             - **\ ``N <= M``\ **
+           * - Debug
+             - Shifts normally
+             - Panics
+             - Panics
+           * - Release
+             - Shifts normally
+             - Shifts by ``M mod N``
+             - Shifts by ``M mod N``
+
+
+        ..
+
+           Note: the behavior is exactly the same for the ``>>`` operator.
+
+
+        Panicking in ``Debug`` is an issue by itself, however, a perhaps larger issue there is that its behavior is different from that of ``Release``. Such inconsistencies aren't acceptable in Safety Critical scenarios.
+
+        Reason 2: programmer intent
+        ===========================
+
+        There is no scenario in which it makes sense to perform a shift of negative length, or of more than ``N - 1`` bits. The operation itself becomes meaningless.
+
+        For both of these reasons, the programmer must ensure the RHS operator stays in the range ``0..=N-1``.
+
+    .. non_compliant_example::
+        :id: non_compl_ex_4YFDofvjh9eV 
+        :status: draft
+
+        As seen in the example below:
+
+
+        * A ``Debug`` build **panics**\ , 
+        * 
+          Whereas a ``Release`` build prints the values:
+
+          .. code-block::
+
+             61 << -1 = 2147483648
+             61 << 4 = 976
+             61 << 40 = 15616
+
+        This shows **Reason 1** prominently.
+
+        **Reason 2** is not seen in the code, because it is a reason of programmer intent: shifts by less than 0 or by more than ``N - 1`` (\ ``N`` being the bit-length of the value being shifted) are both meaningless.
+
+        .. code-block:: rust
+
+            let bits : u32 = 61;
+              let shifts = vec![-1, 4, 40];
+
+              for sh in shifts {
+                  println!("{bits} << {sh} = {}", bits << sh);
+              }
+
+    .. compliant_example::
+        :id: compl_ex_3BkrRwwK5zhX 
+        :status: draft
+
+        As seen in the example below:
+
+
+        * Both ``Debug`` and ``Release`` give the same exact output, which addresses **Reason 1**.
+        * Out-of-range shifts are caught and avoided before they happen.
+        * 
+          The output shows what's happening:
+
+          .. code-block::
+
+             Performing 61 << -1 would be meaningless and crash-prone; we avoided it!
+             61 << 4 = 976
+             Performing 61 << 40 would be meaningless and crash-prone; we avoided it!
+
+        The output shows how this addresses **Reason 2**.
+
+        .. code-block:: rust
+
+            let bits : u32 = 61;
+              let shifts = vec![-1, 4, 40];
+
+              for sh in shifts {
+                  if 0 <= sh && sh < 32 {
+                      println!("{bits} << {sh} = {}", bits << sh);
+                  } else {
+                      println!("Performing {bits} << {sh} would be meaningless and crash-prone; we avoided it!");
+                  }
+              }