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.

Sign up for GitHub

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

Jump to bottom

[bitmask.types] Potential undefined behavior in operator~ #1788

Closed
JohelEGP opened this issue Nov 1, 2017 · 9 comments
Closed

[bitmask.types] Potential undefined behavior in operator~ #1788

JohelEGP opened this issue Nov 1, 2017 · 9 comments
Labels
lwg Issue must be reviewed by LWG.

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented Nov 1, 2017

Wording relative to n4700.

[bitmask.types] specifies that a bitmask type's operator~ can be written as

constexpr bitmask operator~(bitmask X){
  return static_cast<bitmask>(~static_cast<int_type>(X));
}

for

// For exposition only.
// int_type is an integral type capable of representing all values of the bitmask type.
enum bitmask : int_type {
  V 0 = 1 << 0, V 1 = 1 << 1, V 2 = 1 << 2, V 3 = 1 << 3, .....
};

The suggested operator~ results in undefined behavior if sizeof(int_type) < sizeof(int) because static_cast<int_type>(X) has to be promoted to int before evaluating ~, and the outter static_cast<bitmask> would result in undefined behavior because its operand is not within the range of the enumeration values as explained in [expr.static.cast]/10.

My suggested solution is wrapping the ~ and its operand in static_cast<int_type> as follows:

constexpr bitmask operator~(bitmask X){
  return static_cast<bitmask>(static_cast<int_type>(~static_cast<int_type>(X)));
}

Greetings,
Johel
@jensmaurer
Copy link
Member

@zygoloid , is this editorial?

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 1, 2017

This is basically the same issue that I reported about std::byte operations. I would hope that I mentioned this case in that issue, too, but I'm not sure now.

And it's definitely not editorial.

@jensmaurer
Copy link
Member

@tkoeppe : Which LWG issue number?

@jensmaurer jensmaurer added the lwg Issue must be reviewed by LWG. label Nov 1, 2017
@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 1, 2017

@jensmaurer: LWG 2950. It looks like it's not mentioned there, though I'm sure I've brought both byte and bitmask up somewhere, perhaps on the reflector.

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 1, 2017

@jensmaurer: See "std::byte operations are hard to use correctly" on the lib reflector.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Nov 1, 2017 •
edited

Even though it's part of the normative text, I had assumed that it was an editorial issue because what the text does is offer a suggestion on how to write a bitmask type. After all, it uses can instead of shall in the introductory line (bitmask.types/2). So does the previous paragraph. If it actually meant that we can only choose among those (enumerated type, integer type, or bitset), and further, if it's an enumerated type that must be implemented as shown below, then enum class bitmask types are not contemplated.

Should I submit a defect report based on the OP?

@timsong-cpp
Copy link
Contributor

timsong-cpp commented Nov 1, 2017 •
edited

[bitmask.types] is part of [description], which is labeled informative.

Also, the resolution of Core issue 2338 in P0818R0 should obviate the problem.

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 1, 2017

Ah, that's where I had mentioned this problem.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Nov 1, 2017

Ah, I lost sight of the big picture.

Also, the resolution of Core issue 2338 in P0818R0 should obviate the problem.

Does CWG has a more actively updated issues repository like LWG does? I can't find one.

@JohelEGP JohelEGP closed this as completed Nov 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
lwg Issue must be reviewed by LWG.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@tkoeppe @timsong-cpp @JohelEGP @jensmaurer