Skip to content
This repository has been archived by the owner on Jan 7, 2019. It is now read-only.

[architecture] Fix implicit conversion of Flags #230

Merged
merged 3 commits into from
Mar 25, 2017

Conversation

chris-durand
Copy link
Member

@chris-durand chris-durand commented Mar 24, 2017

Fix of the issues discussed in #223. Conversion of Flags to an associated FlagsGroup will now work correctly even when multiple enum constants or flag variables are combined with logical operators.

The bool conversion of flags classes is implemented in terms of explicit operator bool() to fix issues with function overloading on Flags parameters.

The explicit operator will only allow implicit casts where contextual conversion takes place, which is more restrictive than the previous implementation. It will occur in the following contexts:

  • conditions of if, while, for, do-while statements
  • logical operators (&&, ||)
  • negation (operator !)
  • static_assert

This may break user code that requires implicit bool conversions on other occasions, for instance in return statements but I don't expect a lot of people to rely on that. Outside of the flag implementation itself I only changed 5 lines of code in xpcc for this reason.

I had a look at the code that is not build in examples and unittests but uses flags. These changes shouldn't do any harm there but I wouldn't bet my life on that.

The bool conversion of xpcc::Register and thus all derived flags classes
is implemented in terms of "explicit operator bool()". This fixes issues
with function overloading on flags parameters.

Implicit bool conversion without an explicit cast is only applied in
the following contexts:
* conditions of if, while, for, do-while statements
* logical operators (&&, ||)
* negation (operator !)
* static_assert

This will probably break user code that relies on implicit bool
conversion of flags in return statements or in assignments to bool
variables.
@salkinium
Copy link
Member

Thanks for getting to the bottom of this, this is a much nicer solution than using = delete for everything.

I don't expect a lot of people to rely on that. Outside of the flag implementation itself I only changed 5 lines of code in xpcc for this reason.

I suspect the RCA code may break at some points, so keep an eye out for that.

@chris-durand
Copy link
Member Author

I suspect the RCA code may break at some points, so keep an eye out for that.

I've just built most of our current repos with these changes. Everything compiled without errors.

@salkinium salkinium merged commit ad206d9 into roboterclubaachen:develop Mar 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants