🔨 Refactorio #0: Fix all compiler warnings ♻️ (#268)

* Remove unused parameter stack

* Make psram [[maybe_unused]] since it is not used on some targets

* Use static_cast instead of c style cast

* Formatting

* Fix compiler warnings in src/Primitives/emulated.cpp

* Fix warnings + static_cast + fixed memory leak by using std::string copy constructor with length

* Use zu for size type

* Change one more lu to zu

* Remove unused variables from socket.h

* Mark some things as maybe_unused when only used in debug mode

* Reformat

* Fix compiler warnings in src/Debug/debugger.cpp

* Ran clang-format

* Even more clang-format

* Fix linker errors for obscure platforms

* Oops, we forgot Zephyr

---------

Co-authored-by: MaartenS11 <maarten.steevens@gmail.com>

Author: jay-tux

src/Debug/debugger.cpp

@@ -119,7 +119,7 @@ class Debugger {
 
     // Private methods
 
-    void printValue(StackValue *v, uint32_t idx, bool end) const;
+    void printValue(const StackValue *v, uint32_t idx, bool end) const;
 
     // TODO Move parsing to WARDuino class?
     void parseDebugBuffer(size_t len, const uint8_t *buff);
@@ -128,25 +128,25 @@ class Debugger {
 
     //// Handle REPL interrupts
 
-    void handleInvoke(Module *m, uint8_t *interruptData);
+    void handleInvoke(Module *m, uint8_t *interruptData) const;
 
     //// Handle Interrupt Types
 
-    void handleInterruptRUN(Module *m, RunningState *program_state);
+    void handleInterruptRUN(const Module *m, RunningState *program_state);
 
-    void handleSTEP(Module *m, RunningState *program_state);
+    void handleSTEP(const Module *m, RunningState *program_state);
 
-    void handleSTEPOver(Module *m, RunningState *program_state);
+    void handleSTEPOver(const Module *m, RunningState *program_state);
 
     void handleInterruptBP(Module *m, uint8_t *interruptData);
 
     //// Information dumps
 
     void dump(Module *m, bool full = false) const;
 
-    void dumpStack(Module *m) const;
+    void dumpStack(const Module *m) const;
 
-    void dumpLocals(Module *m) const;
+    void dumpLocals(const Module *m) const;
 
     void dumpBreakpoints(Module *m) const;
 
@@ -158,27 +158,28 @@ class Debugger {
 
     void dumpCallbackmapping() const;
 
-    void inspect(Module *m, uint16_t sizeStateArray, uint8_t *state);
+    void inspect(Module *m, uint16_t sizeStateArray,
+                 const uint8_t *state) const;
 
     //// Handle live code update
 
-    static bool handleChangedFunction(Module *m, uint8_t *bytes);
+    static bool handleChangedFunction(const Module *m, uint8_t *bytes);
 
-    bool handleChangedLocal(Module *m, uint8_t *bytes) const;
+    bool handleChangedLocal(const Module *m, uint8_t *bytes) const;
 
-    bool handleUpdateModule(Module *m, uint8_t *data);
+    static bool handleUpdateModule(Module *m, uint8_t *data);
 
-    bool handleUpdateGlobalValue(Module *m, uint8_t *data);
+    bool handleUpdateGlobalValue(const Module *m, uint8_t *data) const;
 
-    bool handleUpdateStackValue(Module *m, uint8_t *bytes);
+    bool handleUpdateStackValue(const Module *m, uint8_t *bytes) const;
 
-    bool reset(Module *m);
+    bool reset(Module *m) const;
 
     //// Handle out-of-place debugging
 
     void freeState(Module *m, uint8_t *interruptData);
 
-    static uint8_t *findOpcode(Module *m, Block *block);
+    static uint8_t *findOpcode(Module *m, const Block *block);
 
     bool saveState(Module *m, uint8_t *interruptData);
 
@@ -213,7 +214,7 @@ class Debugger {
 
     void stop();
 
-    void pauseRuntime(Module *m);  // pause runtime for given module
+    void pauseRuntime(const Module *m);  // pause runtime for given module
 
     // Interrupts
 
@@ -235,20 +236,20 @@ class Debugger {
 
     // Out-of-place debugging: EDWARD
 
-    void snapshot(Module *m);
+    void snapshot(Module *m) const;
 
-    void enableSnapshots(uint8_t *interruptData);
+    void enableSnapshots(const uint8_t *interruptData);
 
-    void sendAsyncSnapshots(Module *m);
+    void sendAsyncSnapshots(Module *m) const;
 
     void proxify();
 
     void handleProxyCall(Module *m, RunningState *program_state,
-                         uint8_t *interruptData);
+                         uint8_t *interruptData) const;
 
-    RFC *topProxyCall();
+    RFC *topProxyCall() const;
 
-    void sendProxyCallResult(Module *m);
+    void sendProxyCallResult(Module *m) const;
 
     bool isProxy() const;
 
@@ -258,11 +259,11 @@ class Debugger {
 
     bool proxy_connected() const;
 
-    void disconnect_proxy();
+    void disconnect_proxy() const;
 
     // Pull-based
 
-    void handleMonitorProxies(Module *m, uint8_t *interruptData);
+    void handleMonitorProxies(const Module *m, uint8_t *interruptData) const;
 
     // Push-based
 

src/Debug/debugger.h

@@ -1261,7 +1261,9 @@ bool i_instr_conversion(Module *m, uint8_t opcode) {
 /**
  * 0xe0 ... 0xe3 callback operations
  */
-bool i_instr_callback(Module *m, uint8_t opcode) {
+// TODO: Remove [[maybe_unused]] when implemented
+bool i_instr_callback([[maybe_unused]] Module *m,
+                      [[maybe_unused]] uint8_t opcode) {
     // TODO
     return true;
 }

src/Interpreter/instructions.cpp

@@ -461,7 +461,8 @@ bool Interpreter::interpret(Module *m, bool waiting) {
     return success;
 }
 
-void Interpreter::report_overflow(Module *m, uint8_t *maddr) {
+void Interpreter::report_overflow([[maybe_unused]] Module *m,
+                                  [[maybe_unused]] uint8_t *maddr) {
     dbg_warn("memory start: %p, memory end: %p, maddr: %p\n", m->memory.bytes,
              m->memory.bytes + m->memory.pages * (uint32_t)PAGE_SIZE, maddr);
     sprintf(exception, "out of bounds memory access");

src/Interpreter/interpreter.cpp

@@ -10,8 +10,9 @@
 #endif
 
 // Assert calloc
-void *acalloc(size_t nmemb, size_t size, const char *name, bool psram) {
-    if ((int)(nmemb * size) == 0) {
+void *acalloc(size_t nmemb, size_t size, const char *name,
+              [[maybe_unused]] bool psram) {
+    if (static_cast<int>(nmemb * size) == 0) {
         return nullptr;
     } else {
         debug("IN Acalloc count: %zu, size: %zu for %s \n", nmemb, size, name);
@@ -28,8 +29,8 @@ void *acalloc(size_t nmemb, size_t size, const char *name, bool psram) {
         debug("Done ... Acalloc\n");
         if (res == nullptr) {
             debug("FAILED ... Acalloc\n");
-            FATAL("Could not allocate %d bytes for %s \n", (int)(nmemb * size),
-                  name);
+            FATAL("Could not allocate %d bytes for %s \n",
+                  static_cast<int>(nmemb * size), name);
         }
         debug("NOT FAILED ... Acalloc\n");
         return res;
@@ -38,7 +39,7 @@ void *acalloc(size_t nmemb, size_t size, const char *name, bool psram) {
 
 // Assert realloc/calloc
 void *arecalloc(void *ptr, size_t old_nmemb, size_t nmemb, size_t size,
-                const char *name, bool psram) {
+                const char *name, [[maybe_unused]] bool psram) {
 #ifdef ARDUINO
     void *res;
     if (psramInit() && psram) {
@@ -47,10 +48,11 @@ void *arecalloc(void *ptr, size_t old_nmemb, size_t nmemb, size_t size,
         res = (size_t *)calloc(nmemb, size);
     }
 #else
-    auto *res = (size_t *)calloc(nmemb, size);
+    auto *res = static_cast<size_t *>(calloc(nmemb, size));
 #endif
     if (res == nullptr) {
-        FATAL("Could not allocate %d bytes for %s", (int)(nmemb * size), name);
+        FATAL("Could not allocate %d bytes for %s",
+              static_cast<int>(nmemb * size), name);
     }
     memset(res, 0, nmemb * size);  // initialize memory with 0
     memmove(res, ptr, old_nmemb * size);

src/Memory/mem.cpp

@@ -1079,7 +1079,7 @@ bool resolve_external_memory(char *symbol, Memory **val) {
 // Restore external state when restoring a snapshot
 //------------------------------------------------------
 void restore_external_state(Module *m,
-                            std::vector<IOStateElement> external_state) {
+                            const std::vector<IOStateElement> &external_state) {
     uint8_t opcode = *m->pc_ptr;
     // TODO: Maybe primitives can also be called using the other call
     // instructions such as call_indirect

src/Primitives/arduino.cpp

@@ -63,7 +63,7 @@ double sensor_emu = 0;
 
 #define def_prim(function_name, type) \
     Type function_name##_type = type; \
-    bool function_name(Module *m)
+    bool function_name([[maybe_unused]] Module *m)
 
 #define def_prim_reverse(function_name)     \
     void function_name##_reverse(Module *m, \
@@ -270,7 +270,8 @@ def_prim(test, oneToNoneU32) {
     Callback c = Callback(m, topic, fidx);
     CallbackHandler::add_callback(c);
     auto *payload = reinterpret_cast<const unsigned char *>("TestPayload");
-    CallbackHandler::push_event(topic, (const char *)payload, 11);
+    CallbackHandler::push_event(topic, reinterpret_cast<const char *>(payload),
+                                11);
     pop_args(1);
     return true;
 }
@@ -318,11 +319,11 @@ def_prim(wifi_connected, NoneToOneU32) {
 
 def_prim(wifi_localip, twoToOneU32) {
     uint32_t buff = arg1.uint32;
-    uint32_t size = arg0.uint32;
+    // uint32_t size = arg0.uint32; // never used in emulator
     std::string ip = "192.168.0.181";
 
     for (unsigned long i = 0; i < ip.length(); i++) {
-        m->memory.bytes[buff + i] = (uint32_t)ip[i];
+        m->memory.bytes[buff + i] = *reinterpret_cast<uint8_t *>(&ip[i]);
     }
     pop_args(2);
     pushInt32(buff);
@@ -345,7 +346,8 @@ def_prim(http_get, fourToOneU32) {
         return false;  // TRAP
     }
     for (unsigned long i = 0; i < answer.length(); i++) {
-        m->memory.bytes[response + i] = (uint32_t)answer[i];
+        m->memory.bytes[response + i] =
+            *reinterpret_cast<unsigned char *>(&answer[i]);
     }
 
     // Pop args and return response address
@@ -365,7 +367,7 @@ def_prim(http_post, tenToOneU32) {
     uint32_t authorization = arg3.uint32;
     uint32_t authorization_len = arg2.uint32;
     int32_t response = arg1.uint32;
-    uint32_t size = arg0.uint32;
+    // uint32_t size = arg0.uint32; // never used in emulator
 
     std::string url_parsed = parse_utf8_string(m->memory.bytes, url_len, url);
     std::string body_parsed =
@@ -398,14 +400,14 @@ def_prim(chip_digital_write, twoToNoneU32) {
 }
 
 def_prim(chip_digital_read, oneToOneU32) {
-    uint8_t pin = arg0.uint32;
+    // uint8_t pin = arg0.uint32; // never used in emulator
     pop_args(1);
     pushUInt32(1);  // HIGH
     return true;
 }
 
 def_prim(chip_analog_read, oneToOneI32) {
-    uint8_t pin = arg0.uint32;
+    // uint8_t pin = arg0.uint32; // never used in emulator
     pop_args(1);
     pushInt32(sin(sensor_emu) * 100);
     sensor_emu += .25;
@@ -460,11 +462,11 @@ def_prim(write_spi_bytes_16, twoToNoneU32) {
 def_prim(subscribe_interrupt, threeToNoneU32) {
     uint8_t pin = arg2.uint32;   // GPIOPin
     uint8_t tidx = arg1.uint32;  // Table Idx pointing to Callback function
-    uint8_t mode = arg0.uint32;
+    // uint8_t mode = arg0.uint32; // never used in emulator
 
     debug("EMU: subscribe_interrupt(%u, %u, %u) \n", pin, tidx, mode);
 
-    if (tidx < 0 || m->table.size < tidx) {
+    if (m->table.size < tidx) {
         debug("subscribe_interrupt: out of range table index %i\n", tidx);
         return false;
     }
@@ -554,6 +556,7 @@ void install_primitives() {
 //------------------------------------------------------
 // resolving the primitives
 //------------------------------------------------------
+// ReSharper disable once CppDFAConstantFunctionResult
 bool resolve_primitive(char *symbol, Primitive *val) {
     debug("Resolve primitives (%d) for %s  \n", ALL_PRIMITIVES, symbol);
 
@@ -566,34 +569,35 @@ bool resolve_primitive(char *symbol, Primitive *val) {
         }
     }
     FATAL("Could not find primitive %s \n", symbol);
-    return false;
+    // return false; // unreachable
 }
 
 Memory external_mem{};
 
+// ReSharper disable once CppDFAConstantFunctionResult
 bool resolve_external_memory(char *symbol, Memory **val) {
     if (!strcmp(symbol, "memory")) {
         if (external_mem.bytes == nullptr) {
             external_mem.initial = 256;
             external_mem.maximum = 256;
             external_mem.pages = 256;
-            external_mem.bytes = (uint8_t *)acalloc(
-                external_mem.pages * PAGE_SIZE, sizeof(uint32_t),
-                "Module->memory.bytes primitive");
+            external_mem.bytes = static_cast<uint8_t *>(
+                acalloc(external_mem.pages * PAGE_SIZE, sizeof(uint32_t),
+                        "Module->memory.bytes primitive"));
         }
         *val = &external_mem;
         return true;
     }
 
     FATAL("Could not find memory %s \n", symbol);
-    return false;
+    // return false; // unreachable
 }
 
 //------------------------------------------------------
 // Restore external state when restoring a snapshot
 //------------------------------------------------------
 void restore_external_state(Module *m,
-                            std::vector<IOStateElement> external_state) {
+                            const std::vector<IOStateElement> &external_state) {
     uint8_t opcode = *m->pc_ptr;
     // TODO: Maybe primitives can also be called using the other call
     // instructions such as call_indirect
@@ -617,7 +621,7 @@ void restore_external_state(Module *m,
     }
 }
 
-std::vector<IOStateElement *> get_io_state(Module *m) {
+std::vector<IOStateElement *> get_io_state(Module *) {
     std::vector<IOStateElement *> ioState;
     for (auto &primitive : primitives) {
         if (primitive.f_serialize_state) {

src/Primitives/emulated.cpp

@@ -298,7 +298,7 @@ bool resolve_external_memory(char *symbol, Memory **val) {
 // Restore external state when restoring a snapshot
 //------------------------------------------------------
 void restore_external_state(Module *m,
-                            std::vector<IOStateElement> external_state) {
+                            const std::vector<IOStateElement> &external_state) {
     uint8_t opcode = *m->pc_ptr;
     // TODO: Maybe primitives can also be called using the other call
     // instructions such as call_indirect

src/Primitives/idf.cpp

@@ -30,9 +30,9 @@ void install_primitives();
 std::vector<IOStateElement *> get_io_state(Module *m);
 
 void restore_external_state(Module *m,
-                            std::vector<IOStateElement> external_state);
+                            const std::vector<IOStateElement> &external_state);
 
-inline void create_stack(std::vector<StackValue> *stack) {}
+inline void create_stack(std::vector<StackValue> *) {}
 
 template <typename T, typename... Ts>
 void create_stack(std::vector<StackValue> *stack, T value, Ts... args) {

src/Primitives/primitives.h

@@ -316,7 +316,7 @@ bool resolve_external_memory(char *symbol, Memory **val) {
 // Restore external state when restoring a snapshot
 //------------------------------------------------------
 void restore_external_state(Module *m,
-                            std::vector<IOStateElement> external_state) {
+                            const std::vector<IOStateElement> &external_state) {
     uint8_t opcode = *m->pc_ptr;
     // TODO: Maybe primitives can also be called using the other call
     // instructions such as call_indirect