Skip to content

Fix various warnings associated with stricter compiler settings - #73

Merged
garethpotter merged 1 commit into
mainfrom
strict-warnings
Oct 7, 2026
Merged

garethpotter merged 1 commit into
mainfrom
strict-warnings

Conversation

@amitchone

Copy link
Copy Markdown
Collaborator

Fixes issues associated with various (stricter) compiler flags enabled:

  • -Wshadow
  • -Wignored-qualifiers
  • Removing a suppression of -Wnarrow
  • -Wconversion
  • -Wzero-as-null-pointer-constant
  • -Wextra-semi

Also fixes CMake warnings when the module is included in a west manifest but isn't built for a target.

@garethpotter

Copy link
Copy Markdown
Collaborator

I will have a look at this shortly. Does this compile on both clang and GCC? I have a feeling the narrowing warning was there for one of the compilers.

@amitchone

Copy link
Copy Markdown
Collaborator Author

This builds for me with clang 16.0.6 and 19.1.7. And of course GCC 12. A wee snippet of the clang-19 build:

user@77b9167b27ce:~/brill-zephyr-workspace/modules/thingset++$ cmake -S tests -B build-clang19 -DCMAKE_C_COMPILER=clang-19 -DCMAKE_CXX_COMPILER=clang++-19
-- The C compiler identification is Clang 19.1.7
-- The CXX compiler identification is Clang 19.1.7
...
-- Configuring done
-- Generating done
-- Build files have been written to: /home/user/brill-zephyr-workspace/modules/thingset++/build-clang19
user@77b9167b27ce:~/brill-zephyr-workspace/modules/thingset++$ cmake --build build-clang19 --target thingset++
[  3%] Building C object thingset++/CMakeFiles/zcbor.dir/zcbor/src/zcbor_common.c.o
...
[100%] Linking CXX shared library libthingset++.so
[100%] Built target thingset++

@garethpotter

garethpotter commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Right, I see the change now (I was on my phone) and the mask is a nice improvement. I think I was confusing it with something else (size of size_t on 64-bit *BSD and macOS, maybe...).


public:
const device *const getDevice();
const device *getDevice();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is the reason for this? The field is still const device *const.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It was flagged by -Wextra because the function is returning a copy of the pointer and so the extra constraint doesn't actually do anything - i.e., the user could do whatever they like with their own copy. But aye, the field itself is still indeed constraint properly

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh I see. Fair enough.

Comment on lines +9 to +12
/* Zephyr's C headers declare functions named after the structs they take (fs_statvfs,
* net_if_mcast_monitor). In C++ that hides the struct's implicit constructor, which
* -Wshadow reports in every file that includes this header. Nothing here uses those
* constructors, so the check is suspended for these includes only. */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice.

Comment on lines +21 to +24
/* ext_id is a 29-bit field and a CAN identifier is at most 29 bits, so the mask
* is lossless; it exists to make that explicit to the compiler
*/
ext_id = id.getId() & CAN_EXT_ID_MASK;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Marvellous.

Comment thread zephyr/CMakeLists.txt
@garethpotter
garethpotter merged commit d2c1eb3 into main Oct 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants