Skip to content

IGNITE-17701 Add common C++ utilities#1090

Merged
isapego merged 7 commits intoapache:mainfrom
gridgain:ignite-17701
Sep 19, 2022
Merged

IGNITE-17701 Add common C++ utilities#1090
isapego merged 7 commits intoapache:mainfrom
gridgain:ignite-17701

Conversation

@ademakov
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@isapego isapego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. My concern though is that it won't compile on Windows and so I will need to implement Window versions of some functions from bits.

Comment on lines +82 to +84
LITTLE = std::endian::little,
BIG = std::endian::big,
NATIVE = std::endian::native,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::endian is C++20 feature

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's conditionally compiled. With a C++-20 compiler it should use standard constants otherwise it falls back to its own substitutes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just implemented a couple of functions for MS compiler. But I haven't checked that it actually works.

@isapego isapego merged commit 48abcbe into apache:main Sep 19, 2022
@isapego isapego deleted the ignite-17701 branch September 19, 2022 08:06
lowka pushed a commit to gridgain/apache-ignite-3 that referenced this pull request Mar 18, 2023
lowka pushed a commit to gridgain/apache-ignite-3 that referenced this pull request Apr 19, 2023
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