Merge pull request #3405 from jepler/implicit-fallthrough-diagnostic

Enable implicit fallthrough diagnostic, note intentional fallthroughs.

15 files changed, 68 insertions, 74 deletions

diff --git a/devices/ble_hci/common-hal/_bleio/att.c b/devices/ble_hci/common-hal/_bleio/att.c
index 591e508a6..6528361cb 100644
--- a/devices/ble_hci/common-hal/_bleio/att.c
+++ b/devices/ble_hci/common-hal/_bleio/att.c
@@ -1748,6 +1748,7 @@ void check_att_err(uint8_t err) {
             break;
         case BT_ATT_ERR_ENCRYPTION_KEY_SIZE:
             msg = translate("Encryption key size");
+            break;
         case BT_ATT_ERR_INVALID_ATTRIBUTE_LEN:
             msg = translate("Invalid attribute length");
             break;
diff --git a/extmod/re1.5/recursiveloop.c b/extmod/re1.5/recursiveloop.c
index bb337decf..5c1f37a5a 100644
--- a/extmod/re1.5/recursiveloop.c
+++ b/extmod/re1.5/recursiveloop.c
@@ -22,6 +22,7 @@ recursiveloop(char *pc, const char *sp, Subject *input, const char **subp, int n
 		case Char:
 			if(*sp != *pc++)
 				return 0;
+                        /* FALLTHROUGH */
 		case Any:
 			sp++;
 			continue;
diff --git a/locale/circuitpython.pot b/locale/circuitpython.pot
index 581442172..038cdacfc 100644
--- a/locale/circuitpython.pot
+++ b/locale/circuitpython.pot
@@ -8,7 +8,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2020-09-13 22:53+0530\n"
+"POT-Creation-Date: 2020-09-13 14:21-0500\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -1923,7 +1923,7 @@ msgstr ""
 msgid "bad format string"
 msgstr ""
 
-#: py/binary.c
+#: py/binary.c py/objarray.c
 msgid "bad typecode"
 msgstr ""
 
@@ -1976,6 +1976,10 @@ msgstr ""
 msgid "bytes > 8 bits not supported"
 msgstr ""
 
+#: py/objarray.c
+msgid "bytes length not a multiple of item size"
+msgstr ""
+
 #: py/objstr.c
 msgid "bytes value out of range"
 msgstr ""
@@ -2941,6 +2945,11 @@ msgstr ""
 msgid "overflow converting long int to machine word"
 msgstr ""
 
+#: py/modstruct.c
+#, c-format
+msgid "pack expected %d items for packing (got %d)"
+msgstr ""
+
 #: shared-bindings/_stage/Layer.c shared-bindings/_stage/Text.c
 msgid "palette must be 32 bytes long"
 msgstr ""
diff --git a/ports/atmel-samd/Makefile b/ports/atmel-samd/Makefile
index 489f3a7af..72c7d9688 100644
--- a/ports/atmel-samd/Makefile
+++ b/ports/atmel-samd/Makefile
@@ -255,7 +255,7 @@ SRC_ASF += \
 	hal/src/hal_mci_sync.c \
 	hpl/sdhc/hpl_sdhc.c \
 
-$(BUILD)/asf4/$(CHIP_FAMILY)/hpl/sdhc/hpl_sdhc.o: CFLAGS += -Wno-cast-align
+$(BUILD)/asf4/$(CHIP_FAMILY)/hpl/sdhc/hpl_sdhc.o: CFLAGS += -Wno-cast-align -Wno-implicit-fallthrough
 endif
 
 $(BUILD)/asf4/$(CHIP_FAMILY)/hpl/sercom/hpl_sercom.o: CFLAGS += -Wno-maybe-uninitialized
diff --git a/ports/nrf/common-hal/_bleio/PacketBuffer.c b/ports/nrf/common-hal/_bleio/PacketBuffer.c
index a8773f961..6d587984c 100644
--- a/ports/nrf/common-hal/_bleio/PacketBuffer.c
+++ b/ports/nrf/common-hal/_bleio/PacketBuffer.c
@@ -129,14 +129,12 @@ STATIC bool packet_buffer_on_ble_client_evt(ble_evt_t *ble_evt, void *param) {
             }
             break;
         }
-        case BLE_GATTC_EVT_WRITE_CMD_TX_COMPLETE: {
+        case BLE_GATTC_EVT_WRITE_CMD_TX_COMPLETE:
             queue_next_write(self);
             break;
-        }
-        case BLE_GATTC_EVT_WRITE_RSP: {
+        case BLE_GATTC_EVT_WRITE_RSP:
             queue_next_write(self);
             break;
-        }
         default:
             return false;
             break;
@@ -171,14 +169,14 @@ STATIC bool packet_buffer_on_ble_server_evt(ble_evt_t *ble_evt, void *param) {
             }
             break;
         }
-        case BLE_GAP_EVT_DISCONNECTED: {
+        case BLE_GAP_EVT_DISCONNECTED:
             if (self->conn_handle == ble_evt->evt.gap_evt.conn_handle) {
                 self->conn_handle = BLE_CONN_HANDLE_INVALID;
             }
-        }
-        case BLE_GATTS_EVT_HVN_TX_COMPLETE: {
+            break;
+        case BLE_GATTS_EVT_HVN_TX_COMPLETE:
             queue_next_write(self);
-        }
+            break;
         default:
             return false;
             break;
diff --git a/py/binary.c b/py/binary.c
index cd0f1aa4d..b85edba62 100644
--- a/py/binary.c
+++ b/py/binary.c
@@ -126,7 +126,6 @@ mp_obj_t mp_binary_get_val_array(char typecode, void *p, mp_uint_t index) {
             break;
         case BYTEARRAY_TYPECODE:
         case 'B':
-        case 'x':   // value will be discarded
             val = ((unsigned char*)p)[index];
             break;
         case 'h':
@@ -330,7 +329,11 @@ void mp_binary_set_val(char struct_type, char val_type, mp_obj_t val_in, byte **
         }
     }
 
-    mp_binary_set_int(MIN((size_t)size, sizeof(val)), struct_type == '>', p, val);
+    if (val_type == 'x') {
+        memset(p, 0, 1);
+    } else {
+        mp_binary_set_int(MIN((size_t)size, sizeof(val)), struct_type == '>', p, val);
+    }
 }
 
 void mp_binary_set_val_array(char typecode, void *p, mp_uint_t index, mp_obj_t val_in) {
@@ -379,8 +382,6 @@ void mp_binary_set_val_array_from_int(char typecode, void *p, mp_uint_t index, m
         case 'B':
             ((unsigned char*)p)[index] = val;
             break;
-        case 'x':
-            ((unsigned char*)p)[index] = 0;
         case 'h':
             ((short*)p)[index] = val;
             break;
diff --git a/py/circuitpy_defns.mk b/py/circuitpy_defns.mk
index 04e38d81e..0e87287c1 100644
--- a/py/circuitpy_defns.mk
+++ b/py/circuitpy_defns.mk
@@ -31,6 +31,7 @@ BASE_CFLAGS = \
 	-fsingle-precision-constant \
 	-fno-strict-aliasing \
 	-Wdouble-promotion \
+	-Wimplicit-fallthrough=2 \
 	-Wno-endif-labels \
 	-Wstrict-prototypes \
 	-Werror-implicit-function-declaration \
diff --git a/py/modstruct.c b/py/modstruct.c
index fe766a4de..7675de275 100644
--- a/py/modstruct.c
+++ b/py/modstruct.c
@@ -183,16 +183,21 @@ MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(struct_unpack_from_obj, 2, 3, struct_unpack_
 
 // This function assumes there is enough room in p to store all the values
 STATIC void struct_pack_into_internal(mp_obj_t fmt_in, byte *p, size_t n_args, const mp_obj_t *args) {
+    size_t size;
+    size_t count = calc_size_items(mp_obj_str_get_str(fmt_in), &size);
+    if (count != n_args) {
+#if MICROPY_ERROR_REPORTING == MICROPY_ERROR_REPORTING_TERSE
+        mp_raise_ValueError(NULL);
+#else
+        mp_raise_ValueError_varg(translate("pack expected %d items for packing (got %d)"), count, n_args);
+#endif
+    }
     const char *fmt = mp_obj_str_get_str(fmt_in);
     char fmt_type = get_fmt_type(&fmt);
 
     size_t i;
     for (i = 0; i < n_args;) {
         mp_uint_t cnt = 1;
-        if (*fmt == '\0') {
-            // more arguments given than used by format string; CPython raises struct.error here
-            break;
-        }
         if (unichar_isdigit(*fmt)) {
             cnt = get_fmt_num(&fmt);
         }
@@ -208,8 +213,7 @@ STATIC void struct_pack_into_internal(mp_obj_t fmt_in, byte *p, size_t n_args, c
             memset(p + to_copy, 0, cnt - to_copy);
             p += cnt;
         } else {
-            // If we run out of args then we just finish; CPython would raise struct.error
-            while (cnt-- && i < n_args) {
+            while (cnt--) {
                 mp_binary_set_val(fmt_type, *fmt, args[i], &p);
                 // Pad bytes don't have a corresponding argument.
                 if (*fmt != 'x') {
@@ -222,7 +226,6 @@ STATIC void struct_pack_into_internal(mp_obj_t fmt_in, byte *p, size_t n_args, c
 }
 
 STATIC mp_obj_t struct_pack(size_t n_args, const mp_obj_t *args) {
-    // TODO: "The arguments must match the values required by the format exactly."
     mp_int_t size = MP_OBJ_SMALL_INT_VALUE(struct_calcsize(args[0]));
     vstr_t vstr;
     vstr_init_len(&vstr, size);
diff --git a/py/objarray.c b/py/objarray.c
index 5d83f0697..e0b4cbd55 100644
--- a/py/objarray.c
+++ b/py/objarray.c
@@ -97,6 +97,9 @@ STATIC void array_print(const mp_print_t *print, mp_obj_t o_in, mp_print_kind_t
 
 #if MICROPY_PY_BUILTINS_BYTEARRAY || MICROPY_PY_ARRAY
 STATIC mp_obj_array_t *array_new(char typecode, size_t n) {
+    if (typecode == 'x') {
+        mp_raise_ValueError(translate("bad typecode"));
+    }
     int typecode_size = mp_binary_get_size('@', typecode, NULL);
     mp_obj_array_t *o = m_new_obj(mp_obj_array_t);
     #if MICROPY_PY_BUILTINS_BYTEARRAY && MICROPY_PY_ARRAY
@@ -126,8 +129,10 @@ STATIC mp_obj_t array_construct(char typecode, mp_obj_t initializer) {
                 || (MICROPY_PY_BUILTINS_BYTEARRAY && MP_OBJ_IS_TYPE(initializer, &mp_type_bytearray)))))
         && mp_get_buffer(initializer, &bufinfo, MP_BUFFER_READ)) {
         // construct array from raw bytes
-        // we round-down the len to make it a multiple of sz (CPython raises error)
         size_t sz = mp_binary_get_size('@', typecode, NULL);
+        if (bufinfo.len % sz) {
+            mp_raise_ValueError(translate("bytes length not a multiple of item size"));
+        }
         size_t len = bufinfo.len / sz;
         mp_obj_array_t *o = array_new(typecode, len);
         memcpy(o->items, bufinfo.buf, len * sz);
diff --git a/py/objset.c b/py/objset.c
index b8f0f07ed..45b5c1260 100644
--- a/py/objset.c
+++ b/py/objset.c
@@ -450,6 +450,7 @@ STATIC mp_obj_t set_unary_op(mp_unary_op_t op, mp_obj_t self_in) {
                 return MP_OBJ_NEW_SMALL_INT(hash);
             }
 #endif
+            /* FALLTHROUGH */
         default: return MP_OBJ_NULL; // op not supported
     }
 }
diff --git a/tests/basics/struct1.py b/tests/basics/struct1.py
index db34342a1..107006cc3 100644
--- a/tests/basics/struct1.py
+++ b/tests/basics/struct1.py
@@ -39,6 +39,28 @@ print(v == (10, 100, 200, 300))
 # network byte order
 print(struct.pack('!i', 123))
 
+# too short / too long arguments
+buf = bytearray(b'>>>123<<<')
+try:
+    struct.pack_into('bb', buf, 0, 3)
+except:
+    print('struct.error')
+
+try:
+    struct.pack_into('bb', buf, 0, 3, 1, 4)
+except:
+    print('struct.error')
+
+try:
+    struct.pack('bb', 3)
+except:
+    print('struct.error')
+
+try:
+    struct.pack('bb', 3, 1, 4)
+except:
+    print('struct.error')
+
 # check that we get an error if the buffer is too small
 try:
     struct.unpack('I', b'\x00\x00\x00')
@@ -96,3 +118,6 @@ try:
     print(struct.unpack_from('<b', buf, -11))
 except:
     print('struct.error')
+
+# check padding bytes
+print(struct.pack("xb", 3))
diff --git a/tests/basics/struct_micropython.py b/tests/basics/struct_micropython.py
deleted file mode 100644
index e72153071..000000000
--- a/tests/basics/struct_micropython.py
+++ /dev/null
@@ -1,41 +0,0 @@
-# test MicroPython-specific features of struct
-
-try:
-    import ustruct as struct
-except:
-    try:
-        import struct
-    except ImportError:
-        print("SKIP")
-        raise SystemExit
-
-try:
-    struct.pack('O', None)
-except ValueError:
-    print("SKIP")
-    raise SystemExit
-
-class A():
-    pass
-
-# pack and unpack objects
-o = A()
-s = struct.pack("<O", o)
-o2 = struct.unpack("<O", s)
-print(o is o2[0])
-
-# pack can accept less arguments than required for the format spec
-print(struct.pack('<2I', 1))
-
-# pack and unpack pointer to a string
-# This requires uctypes to get the address of the string and instead of
-# putting this in a dedicated test that can be skipped we simply pass
-# if the import fails.
-try:
-    import uctypes
-    o = uctypes.addressof('abc')
-    s = struct.pack("<S", o)
-    o2 = struct.unpack("<S", s)
-    assert o2[0] == 'abc'
-except ImportError:
-    pass
diff --git a/tests/basics/struct_micropython.py.exp b/tests/basics/struct_micropython.py.exp
deleted file mode 100644
index 55b7b6623..000000000
--- a/tests/basics/struct_micropython.py.exp
+++ /dev/null
@@ -1,2 +0,0 @@
-True
-b'\x01\x00\x00\x00\x00\x00\x00\x00'
diff --git a/tests/misc/non_compliant.py b/tests/misc/non_compliant.py
index 99633416a..9a746a8b0 100644
--- a/tests/misc/non_compliant.py
+++ b/tests/misc/non_compliant.py
@@ -105,12 +105,6 @@ try:
 except NotImplementedError:
     print('NotImplementedError')
 
-# struct pack with too many args, not checked by uPy
-print(struct.pack('bb', 1, 2, 3))
-
-# struct pack with too few args, not checked by uPy
-print(struct.pack('bb', 1))
-
 # array slice assignment with unsupported RHS
 try:
     bytearray(4)[0:1] = [1, 2]
diff --git a/tests/misc/non_compliant.py.exp b/tests/misc/non_compliant.py.exp
index 3f15a1440..8518828ec 100644
--- a/tests/misc/non_compliant.py.exp
+++ b/tests/misc/non_compliant.py.exp
@@ -14,8 +14,6 @@ NotImplementedError
 NotImplementedError
 NotImplementedError
 NotImplementedError
-b'\x01\x02'
-b'\x01\x00'
 NotImplementedError
 AttributeError
 TypeError