-
-
Notifications
You must be signed in to change notification settings - Fork 120
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 #217 issue with compiling with newer compiler #223
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be a problem for you, but directly invoking the Constructor could invoke a problem for the Arduino cores using this API.
Please can you clarify the potential problem you see here so I can fix this another way. Are you concerned about the use of a delegating constructor like is used on line 52? |
Yes. Also, you are probably the only one that compiles the unit tests with clang. I'm reluctant to make this change to just accomodate a single user (sorry, but we've got to keep the bigger picture in mind, and clang is more exotic than plain old GCC). |
If you can fix your issue with |
The problem here isn't specific to clang but more to do with newer versions of the compilers. I get the same error with GCC-13
The warning is correct, So the simple fix here is just to remove the redundant check and I can update the PR to do that. However, I'm unclear on the concern with delegating constructors: they are a C++11 feature which is the language version used by the API, and they are already used elsewhere in the code, for example on line 52 of this file. |
Branch updated not to use constructor delegation. Should I create a new PR or would you reopen this one? |
Just reopen this PR. |
I don't see the buttons for that. I'm guessing I don't have permission and so it would need to be reopened by committer. |
Thanks for checking. I rebased and so had to force push, I guess that broke it. |
See comment on #217 about problem compiling tests on desktop (specifically with clang 15.0.0)