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

error: explicitly assigning value of variable of type 'ola::DmxBuffer' to itself #1564

Closed
peternewman opened this issue Jun 30, 2019 · 7 comments · Fixed by #1426
Closed

Comments

@peternewman
Copy link
Member

common/utils/DmxBufferTest.cpp:197:10: error: explicitly assigning value of
      variable of type 'ola::DmxBuffer' to itself
      [-Werror,-Wself-assign-overloaded]
  buffer = buffer;
  ~~~~~~ ^ ~~~~~~
1 error generated.

Is the error when I am up to date with origin/master (and for this branch as well)

Originally posted by @kecramer in #1561 (comment)

@janosvitok
Copy link

https://github.com/OpenLightingProject/ola/blob/master/common/utils/DmxBufferTest.cpp#L197
I guess this is intentional check whether assignment operation survives self-assignment.

However I miss there any explicit checks for consistency after self assignment. Only implicit ones are present (i.e. the code does not crash).

@peternewman
Copy link
Member Author

https://github.com/OpenLightingProject/ola/blob/master/common/utils/DmxBufferTest.cpp#L197
I guess this is intentional check whether assignment operation survives self-assignment.

I think that's the idea based on the comment @janosvitok .

However I miss there any explicit checks for consistency after self assignment. Only implicit ones are present (i.e. the code does not crash).

Do you mean checking the contents of buffer is still equal to what it was initialised to? Do you fancy trying to add some further tests around this?

I think to workaround the warning/error we might need to add this flag to the relevant bit of test code (but ideally just that bit, not everywhere), although I think when I looked before, GCC complains if passed this flag (or LLVM if it was GCC throwing it):
-Wno-self-assign-overloaded

@peternewman
Copy link
Member Author

I think to workaround the warning/error we might need to add this flag to the relevant bit of test code (but ideally just that bit, not everywhere), although I think when I looked before, GCC complains if passed this flag (or LLVM if it was GCC throwing it):
-Wno-self-assign-overloaded

We may be able to workaround this problem using the push/pop idea @yoe is using here:
67234e2

@peternewman
Copy link
Member Author

@kecramer or anyone else, which compiler does this happen on, I think it should be a fairly simple fix but it would be nice to test it.

@peternewman peternewman modified the milestones: 0.10.8, 0.11.0 Nov 19, 2020
@peternewman peternewman linked a pull request Nov 19, 2020 that will close this issue
@peternewman peternewman modified the milestones: 0.11.0, 0.10.8 Nov 21, 2020
@peternewman
Copy link
Member Author

https://github.com/OpenLightingProject/ola/blob/master/common/utils/DmxBufferTest.cpp#L197
I guess this is intentional check whether assignment operation survives self-assignment.

Yeah agreed.

However I miss there any explicit checks for consistency after self assignment. Only implicit ones are present (i.e. the code does not crash).

I've worked around the issue, and added some basic tests in https://github.com/OpenLightingProject/ola/pull/1426/files was that the sort of thing you meant @janosvitok ?

@kecramer or anyone else, which compiler does this happen on, I think it should be a fairly simple fix but it would be nice to test it.

We already had it in #1426 so ignore that!

@janosvitok
Copy link

However I miss there any explicit checks for consistency after self assignment. Only implicit ones are present (i.e. the code does not crash).

I've worked around the issue, and added some basic tests in https://github.com/OpenLightingProject/ola/pull/1426/files was that the sort of thing you meant @janosvitok ?

Yes, something like that.

@peternewman
Copy link
Member Author

Great, I'll close this then @janosvitok !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants