Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build warnings/errors in C++20 #2883

Merged
merged 2 commits into from
Sep 23, 2024
Merged

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented Sep 14, 2024

FlashString static initialisation

The Object copy and move constructors are deleted as a precaution as these objects should not be copied.
C++20 doesn't like this so they're only compiled for C++17 and earlier.

std:is_pod deprecated

Use std::is_standard_layout instead (only one instance to correct).

Volatile

C++20 deprecates many uses of volatile which may be implemented in an ambiguous (non-deterministic) way.
Largely these uses are concerned with threaded applications. A general web search turns up plenty of articles on this.

One argument is related to code like this:

volatile int num;
++num; // Is this atomic? Or a read, followed by load?

So we should identify all such cases and code them more explicitly.

The FIFO/FILO classes define numberOfElements as volatile but I can see no reason why this needs to be the case.

Volatile bitwise operations

C++23 will, it seems, de-deprecate use of volatile for bitwise operations.
This is discussed here https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2327r1.pdf.
See also earlephilhower/arduino-pico#1379

These are genuine uses of volatile as values are used both in interrupt and task context. Some of these actually need to be atomic, and the assumption is probably that x |= n does that. Hence the ambiguity. These are the core Sming files affected:

  • Sming/Arch/Esp8266/Components/driver/i2s.cpp
  • Sming/Arch/Esp8266/Components/spi_flash/iram_precache.cpp
  • Sming/Arch/Esp8266/Components/gdbstub/appcode/gdb_hooks.cpp
  • Sming/Arch/Esp8266/Components/gdbstub/gdbstub.cpp
  • Sming/Arch/Esp8266/Components/gdbstub/gdbuart.cpp
  • Sming/Arch/Esp8266/Core/Digital.cpp

There will be many others in libraries.

Solution: Rather than attempting to 'correct' the code and introduce bugs - risks identified in the above paper - it is safer to just silence this particular warning. This has been done in the main Sming build.mk file so it's easy to revert if required. This is the volatile warning which covers other uses as well - future GCC revisions may add some granularity to this.

FIFO / FILO uses int instead of unsigned

No warnings but using int for the count makes no sense.
Not related to C++20 but since we're modifying the classes anyway may as well do it here.

@mikee47 mikee47 marked this pull request as draft September 14, 2024 11:28
@slaff slaff added this to the 6.0.0 milestone Sep 16, 2024
@mikee47 mikee47 marked this pull request as ready for review September 20, 2024 19:34
@SmingHub SmingHub deleted a comment from what-the-diff bot Sep 21, 2024
@slaff slaff merged commit e6ed3d9 into SmingHub:develop Sep 23, 2024
32 checks passed
berhoel added a commit to berhoel/Sming that referenced this pull request Sep 23, 2024
Fix build warnings/errors in C++20 (SmingHub#2883)
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.

2 participants