Skip to content

Replace volatile stopSignalCounter ++ operator#31

Merged
teddych merged 1 commit intoteddych:masterfrom
ekohl:fix-volatile-deprecation-warning
Jan 21, 2026
Merged

Replace volatile stopSignalCounter ++ operator#31
teddych merged 1 commit intoteddych:masterfrom
ekohl:fix-volatile-deprecation-warning

Conversation

@ekohl
Copy link
Contributor

@ekohl ekohl commented Jan 17, 2026

C++20 has deprecated the ++ and -- operations on volatile and C++26 is looking at removing them. This replaces them with an explicit update.

Fixes #30

@teddych
Copy link
Owner

teddych commented Jan 19, 2026

Logger.h is not used in Railcontrol.h.
It is used in Railcontrol.cpp and there it is explicitly invoked.

@ekohl
Copy link
Contributor Author

ekohl commented Jan 19, 2026

Logger.h is not used in Railcontrol.h.

When I edited Railcontrol.h my editor complained about missing it because Logger::Logger wasn't defined but is used here:
https://github.com/teddych/railcontrol/blob/a50cf8f0653613ea5509477fabd4425b07c8dead/RailControl.h#L27C30-L27C44

That went away when I included Logger.h.

@teddych
Copy link
Owner

teddych commented Jan 19, 2026

The information of Logger.h or the content of class Logger is not need to compile. But to prevent a warning I expect that

namespace
{
class Logger;
}

will prevent the warning without including the complete header file.

@ekohl
Copy link
Contributor Author

ekohl commented Jan 19, 2026

I can take the commit out of the PR since I think technically it isn't required to fix the issue I was after. I only included it because it's changing the same file and it would be a conflict later.

@teddych
Copy link
Owner

teddych commented Jan 19, 2026

Another word to volatile or atomic:
Here I need really a volatile variable. But it is not important if it is atomic. The ++ operator is not atomic, but it is intended to be atomic by a lot of programmers (that never worked on embedded systems). Because the intention is wrong it is removed completely.

What I really need in the code is:

stopSignalCounter = stopSignalCounter + 1;
if (stopSignalCounter < MaxStopSignalCounter)

So if you want to change, then do not remove/replace the volatile but remove the ++ operator, please.

@teddych teddych marked this pull request as draft January 19, 2026 12:35
@ekohl
Copy link
Contributor Author

ekohl commented Jan 19, 2026

Note I don't know for sure if this is correct so please verify it carefully.

And that's exactly what you did. Thanks!

@ekohl ekohl force-pushed the fix-volatile-deprecation-warning branch from 1b814c0 to 609c3d5 Compare January 19, 2026 12:55
@ekohl ekohl changed the title Replace volatile stopSignalCounter with std::atomic Replace volatile stopSignalCounter ++ operator Jan 19, 2026
@ekohl
Copy link
Contributor Author

ekohl commented Jan 19, 2026

I pushed an update, but I don't have an env where I can reproduce the failure at hand now so keeping it as draft.

@teddych
Copy link
Owner

teddych commented Jan 21, 2026

It is in state Draft. Therefore I can't merge. Can you change it?

C++20 has deprecated the ++ and -- operations on volatile and C++26 is
looking at removing them. This replaces them with an explicit update.
@ekohl ekohl force-pushed the fix-volatile-deprecation-warning branch from 609c3d5 to 1cceda5 Compare January 21, 2026 11:58
@ekohl ekohl marked this pull request as ready for review January 21, 2026 11:58
@ekohl
Copy link
Contributor Author

ekohl commented Jan 21, 2026

I just tested this in my tentative Fedora packaging build where I noticed it for the first time and I can see it now compiles.

@teddych teddych merged commit f9c512f into teddych:master Jan 21, 2026
5 checks passed
@ekohl ekohl deleted the fix-volatile-deprecation-warning branch January 21, 2026 12:05
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.

Build failure: error: ‘++’ expression of ‘volatile’-qualified type is deprecated [-Werror=volatile]

2 participants