Skip to content

Drop o2::byte - #6253

Merged
ktf merged 1 commit into
AliceO2Group:devfrom
ktf:drop-byte
Jun 2, 2021
Merged

Drop o2::byte#6253
ktf merged 1 commit into
AliceO2Group:devfrom
ktf:drop-byte

Conversation

@ktf

@ktf ktf commented May 26, 2021

Copy link
Copy Markdown
Member

Replace it with C++17 std::byte. This does not endorse the actual
idea behind std::byte usage, merely gets rid of an O2 specific type
and header for it.

@ktf

ktf commented May 26, 2021

Copy link
Copy Markdown
Member Author

Even better then this would be to simply replace o2::byte with just char. When do we expect the extra type safety to be actually useful?

Replace it with C++17 std::byte. This does not endorse the actual
idea behind std::byte usage, merely gets rid of an O2 specific type
and header for it.
@Barthelemy

Copy link
Copy Markdown
Collaborator

Concerning your second remark: what is the drawback of using std::byte ?

@ktf

ktf commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

@ironMann how would you like to proceed here? Shall we simply merge this and fix DataDistribution at the same time?

@ktf

ktf commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

@Barthelemy I missed you commented here. std::byte is a separate type from char, so there is casting involved everywhere this is not taken into account (e.g. when printing / formatting), which can catch some errors, but also might introduce a lot of boilerplate. As usual I guess it's a matter of tradeoffs.

@ironMann

ironMann commented Jun 2, 2021

Copy link
Copy Markdown
Contributor

@kft
As far as I've seen, we mostly (only?) use byte*, and never use dereference of byte* directly. We still randomly cast byte* to different pointers and deref that (which is broken). From the standard, byte* has the same aliasing rules as char*, so there should be no surprises.

Go ahead and merge if this is a simple rename.

@ktf
ktf merged commit bc86bb4 into AliceO2Group:dev Jun 2, 2021
@ktf
ktf deleted the drop-byte branch June 2, 2021 07:08
@ktf

ktf commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

Ok, I did, could you tag DataDistribution with AliceO2Group/DataDistribution#94 included?

cortesep pushed a commit to cortesep/AliceO2 that referenced this pull request Jun 11, 2021
Replace it with C++17 std::byte. This does not endorse the actual
idea behind std::byte usage, merely gets rid of an O2 specific type
and header for it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants