Skip to content

[2.0.x] Squelch compiler warnings with -Wall#11889

Merged
thinkyhead merged 3 commits intoMarlinFirmware:bugfix-2.0.xfrom
marcio-ao:pr-eliminate-compiler-warnings
Sep 22, 2018
Merged

[2.0.x] Squelch compiler warnings with -Wall#11889
thinkyhead merged 3 commits intoMarlinFirmware:bugfix-2.0.xfrom
marcio-ao:pr-eliminate-compiler-warnings

Conversation

@marcio-ao
Copy link
Contributor

@marcio-ao marcio-ao commented Sep 21, 2018

SUMMARY OF WARNINGS

  • EepromEmulation_Due.cpp:
    • Fixed several comparisons between signed and unsigned values
    • Added parenthesis around "&" operator where compiler suggested they be added.
  • stepper_indirection.cpp:
    • Corrected unused variables
  • M911-M915.cpp:
    • Corrected unused variables.

DETAILED WARNINGS

src/HAL/HAL_DUE/EepromEmulation_Due.cpp: In function 'bool ee_PageWrite(uint16_t, const void*)':
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:210:33: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses]
   efc->EEFC_FMR = efc->EEFC_FMR & (~EEFC_FMR_FWS_Msk) | EEFC_FMR_FWS(6);
                                 ^
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:223:35: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses]
     efc->EEFC_FMR = efc->EEFC_FMR & (~EEFC_FMR_FWS_Msk) | EEFC_FMR_FWS(orgWS);
                                   ^
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:238:56: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   for (i = 0; i < (IFLASH0_PAGE_SIZE / sizeof(uint32_t)); ++i) {
                                                        ^
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:250:35: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses]
     efc->EEFC_FMR = efc->EEFC_FMR & (~EEFC_FMR_FWS_Msk) | EEFC_FMR_FWS(orgWS);
                                   ^
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:263:33: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses]
   efc->EEFC_FMR = efc->EEFC_FMR & (~EEFC_FMR_FWS_Msk) | EEFC_FMR_FWS(orgWS);
                                 ^
src/HAL/HAL_DUE/EepromEmulation_Due.cpp: In function 'bool ee_PageErase(uint16_t)':
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:341:33: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses]
   efc->EEFC_FMR = efc->EEFC_FMR & (~EEFC_FMR_FWS_Msk) | EEFC_FMR_FWS(6);
                                 ^
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:353:35: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses]
     efc->EEFC_FMR = efc->EEFC_FMR & (~EEFC_FMR_FWS_Msk) | EEFC_FMR_FWS(orgWS);
                                   ^
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:367:56: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   for (i = 0; i < (IFLASH0_PAGE_SIZE / sizeof(uint32_t)); ++i) {
                                                        ^
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:378:35: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses]
     efc->EEFC_FMR = efc->EEFC_FMR & (~EEFC_FMR_FWS_Msk) | EEFC_FMR_FWS(orgWS);
                                   ^
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:391:33: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses]
   efc->EEFC_FMR = efc->EEFC_FMR & (~EEFC_FMR_FWS_Msk) | EEFC_FMR_FWS(orgWS);
                                 ^
src/HAL/HAL_DUE/EepromEmulation_Due.cpp: In function 'bool ee_Write(uint32_t, uint8_t)':
src/HAL/HAL_DUE/EepromEmulation_Due.cpp:845:76: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
             (buffer[inext] | (buffer[inext + 1] << 8)) == (baddr + blen + 1)) {

src/module/stepper_indirection.cpp: In function 'void tmc2130_init_to_defaults()':
src/module/stepper_indirection.cpp:264:23: warning: unused variable 'extruder' [-Wunused-variable]
       { constexpr int extruder = 1; _TMC2130_INIT(E1, planner.axis_steps_per_mm[E_AXIS_N]); }

src/gcode/feature/trinamic/M911-M915.cpp: In static member function 'static void GcodeSuite::M913()':
src/gcode/feature/trinamic/M911-M915.cpp:165:52: warning: unused variable 'extruder' [-Wunused-variable]
     #define TMC_SET_PWMTHRS_E(E) do{ const uint8_t extruder = E; tmc_set_pwmthrs(stepperE##E, value, planner.axis_steps_per_mm[E_AXIS_N]); }while(0)
                                                    ^
src/gcode/feature/trinamic/M911-M915.cpp:203:23: note: in expansion of macro 'TMC_SET_PWMTHRS_E'
               case 0: TMC_SET_PWMTHRS_E(0); break;
                       ^
src/gcode/feature/trinamic/M911-M915.cpp:165:52: warning: unused variable 'extruder' [-Wunused-variable]
     #define TMC_SET_PWMTHRS_E(E) do{ const uint8_t extruder = E; tmc_set_pwmthrs(stepperE##E, value, planner.axis_steps_per_mm[E_AXIS_N]); }while(0)
                                                    ^
src/gcode/feature/trinamic/M911-M915.cpp:206:23: note: in expansion of macro 'TMC_SET_PWMTHRS_E'
               case 1: TMC_SET_PWMTHRS_E(1); break;
                       ^
src/gcode/feature/trinamic/M911-M915.cpp:164:52: warning: unused variable 'extruder' [-Wunused-variable]
     #define TMC_SAY_PWMTHRS_E(E) do{ const uint8_t extruder = E; tmc_get_pwmthrs(stepperE##E, TMC_E##E, planner.axis_steps_per_mm[E_AXIS_N]); }while(0)
                                                    ^
src/gcode/feature/trinamic/M911-M915.cpp:248:9: note: in expansion of macro 'TMC_SAY_PWMTHRS_E'
         TMC_SAY_PWMTHRS_E(0);
         ^
src/gcode/feature/trinamic/M911-M915.cpp:164:52: warning: unused variable 'extruder' [-Wunused-variable]
     #define TMC_SAY_PWMTHRS_E(E) do{ const uint8_t extruder = E; tmc_get_pwmthrs(stepperE##E, TMC_E##E, planner.axis_steps_per_mm[E_AXIS_N]); }while(0)
                                                    ^
src/gcode/feature/trinamic/M911-M915.cpp:251:9: note: in expansion of macro 'TMC_SAY_PWMTHRS_E'
         TMC_SAY_PWMTHRS_E(1);

@marcio-ao marcio-ao force-pushed the pr-eliminate-compiler-warnings branch from dc22ca8 to 61c117b Compare September 21, 2018 21:17
- EepromEmulation_Due.cpp:
    - Fixed several comparisons between signed and unsigned values
    - Added parenthesis around "&" operator where compiler suggested they be added.
- stepper_indirection.cpp:
    - Corrected unused variables
- M911-M915.cpp:
    - Corrected unused variables.
#define TMC_SAY_PWMTHRS_E(E) do{ const uint8_t extruder = E; tmc_get_pwmthrs(stepperE##E, TMC_E##E, planner.axis_steps_per_mm[E_AXIS_N]); }while(0)
#define TMC_SET_PWMTHRS_E(E) do{ const uint8_t extruder = E; tmc_set_pwmthrs(stepperE##E, value, planner.axis_steps_per_mm[E_AXIS_N]); }while(0)
#define TMC_SAY_PWMTHRS_E(E) do{tmc_get_pwmthrs(stepperE##E, TMC_E##E, planner.axis_steps_per_mm[E_AXIS_N]); }while(0)
#define TMC_SET_PWMTHRS_E(E) do{tmc_set_pwmthrs(stepperE##E, value, planner.axis_steps_per_mm[E_AXIS_N]); }while(0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E_AXIS_N needs extruder to be defined when DISTINCT_E_FACTORS is enabled, so this can't be left out. However, an UNUSED(extruder) could be added to the end of each of these blocks to suppress the warning, without affecting the case where DISTINCT_E_FACTORS is enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E_AXIS_N needs extruder to be defined when DISTINCT_E_FACTORS is enabled, so this can't be left out.

Oh, I see. I was blindsided by the macros.

Copy link
Contributor Author

@marcio-ao marcio-ao Sep 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, just curious, why the do{}while(0)? Isn't this just equivalent to plain old curly braces? Like this:

#define TMC_SAY_PWMTHRS_E(E) {const uint8_t extruder = E; tmc_get_pwmthrs(stepperE##E, TMC_E##E, planner.axis_steps_per_mm[E_AXIS_N]); }

Copy link
Member

@thinkyhead thinkyhead Sep 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compilers have been known to complain about semicolons in strange locations, but these days they don't seem to care as much — and will even ignore the trailing comma in an array declaration. Using do{...}while(0) is just the most-compatible old-school method to group code for even the strictest compiler, and I stick to it because it requires the semicolon and promotes good coding practice.

#define TMC_SAY_PWMTHRS_E(E) do{ ... }while(0)
TMC_SAY_PWMTHRS_E(1)  // compiler complains because no semicolon
TMC_SAY_PWMTHRS_E(2); // compiler happy because semicolon

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. I was not aware of older compilers complaining about the semicolon at the end of blocks. The idiomatic use of do/while makes a lot more sense now. Thank you for the explanation.

#endif
#if AXIS_DRIVER_TYPE(E4, TMC2130)
{ constexpr int extruder = 4; _TMC2130_INIT(E4, planner.axis_steps_per_mm[E_AXIS_N]); }
{ _TMC2130_INIT(E4, planner.axis_steps_per_mm[E_AXIS_N]); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with these.

@thinkyhead thinkyhead merged commit 5c8e20a into MarlinFirmware:bugfix-2.0.x Sep 22, 2018
@marcio-ao marcio-ao deleted the pr-eliminate-compiler-warnings branch October 9, 2018 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants