Skip to content

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Oct 8, 2022

Instead of disabling a compiler warning for the entire codebase, it is safer to disable it for a single line of code.

The only line of code is guarded with `#pragma diagnostic ignored`.
@sipa
Copy link
Collaborator

sipa commented Oct 19, 2022

@hebasto Ugh, that is actually UB (in theory, and only for fields < 8 bits in size, which are in practice only used for testing). We shouldn't just be silencing it.

See #74 for an alternative.

@hebasto
Copy link
Member Author

hebasto commented Oct 19, 2022

Closing in favour of #74.

@hebasto hebasto closed this Oct 19, 2022
sipa added a commit that referenced this pull request Apr 11, 2024
fe1040f Drop -Wno-shift-count-overflow compile flag (Pieter Wuille)
154bcd4 Avoid >> above type width in BitWriter (Pieter Wuille)

Pull request description:

  For tiny fields (bits <= 8), the table type is `uint8_t`, which results in `BitWriter` using uint8_t logic as well. This is technically UB, as it performs right shifts of up to 8 positions.

  Avoid this by performing the BitWriter logic in either the table type, or `unsigned`, whichever is larger.

  This is an alternative to #71.

ACKs for top commit:
  hebasto:
    re-ACK fe1040f. Only rebased since my [recent](#74 (review)) review.

Tree-SHA512: fb41e125253d11f1662af77e9fc6e89ea7f4712f4fa5d500f483da3c6b325760d3ffedabedb7f15a7702223c86bc651b26004763e44c98d379cc2966ea0919a3
@hebasto hebasto deleted the 221008-shift branch April 11, 2024 08:21
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.

2 participants