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.

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

Support c++20 and clang compiler #86

Merged
merged 15 commits into from
Jan 29, 2024
Merged

Conversation

drewdzzz
Copy link
Collaborator

@drewdzzz drewdzzz commented Dec 28, 2023

The patch adds support of C++20 and clang compiler. Along the way, fixes some clang-format problems.
Note that we don't test Tarantool build with particular clang or gcc versions - default ones from Github runners are used.

Since we support clang compiler, the patch enables undefined behavior sanitizers (#73)

Closes #85
Closes #73

@drewdzzz drewdzzz force-pushed the support-c++20 branch 15 times, most recently from cb0a126 to 27a49a3 Compare December 28, 2023 12:50
@drewdzzz drewdzzz force-pushed the support-c++20 branch 8 times, most recently from 3553cb3 to 4c07c6c Compare January 12, 2024 12:39
@drewdzzz drewdzzz changed the title Support c++20 Support c++20 and clang compiler Jan 12, 2024
@drewdzzz drewdzzz force-pushed the support-c++20 branch 6 times, most recently from 014e902 to c7f5ba6 Compare January 12, 2024 13:09
Apparently, this type only exists in std namespace on clang.
This function was static in header so the warning "static function
defined in header must be inline" on clang was triggered. Let's make
this function private static method of libev - it will hide this
non-public function and solve the problem with warning.
Because of this, the project's build using clang was failing.
The header is required to use `uint*_t` types.
Pre-installed compilers are used, building with a compiler of
particular versions is not tested.

GCC on MacOS is not tested since it is pointless - it is just
an alias for clang there.

There was a problem in the way - ubuntu-22.04 runner has incompatible
clang and stdlibc++ versions. As an outcome, tntcxx build with clang
and c++20 standard fails, link to the corresponding issue can be found
in a workflow file. In order to work this problem around, let's remove
g++-13 and install g++-12 on this runner, which has compatible library
version.
@drewdzzz drewdzzz force-pushed the support-c++20 branch 2 times, most recently from 370d554 to 63fb239 Compare January 23, 2024 19:12
The job only builds the connector, without setting up Tarantool and
testing tntcxx. It is used to check building with different compilers
and c++ standards.
The linter asked me to make them sorted.
The commit makes clang-format config more suitable to our codebase.
Clang format can return "clang-format did not modify any files" - let's
support this string.
It is required to test tntcxx with undefined behavior sanitizer, which
is disabled on GCC because of bug.

Closes tarantool#73
Copy link
Member

@CuriousGeorgiy CuriousGeorgiy left a comment

Choose a reason for hiding this comment

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

I would still prefer using specific compiler versions in CI (fine to use the default available compiler versions). Up to @alyapunov.

@CuriousGeorgiy
Copy link
Member

CuriousGeorgiy commented Jan 24, 2024

@drewdzzz please take a look at the failing tests. It doesn't look like #75.

@drewdzzz drewdzzz force-pushed the support-c++20 branch 2 times, most recently from 3e4dc90 to b04171e Compare January 24, 2024 14:24
@drewdzzz
Copy link
Collaborator Author

@CuriousGeorgiy
The problem was that there is new stable Tarantool version in brew (3.0.0), which has case-sensitive SQL names. So, if brew was already updated (since we don't update it), Tarantool 3.0.0 was installed, and the test failed. Fixed.

In brew, new stable version of Tarantool is 3.0.0. It has a new feature:
case-sensitive names in SQL. So let's make ClientTest version-independent
and call SQL columns in upper case.

Along the way, let's print response sql metadata in ClientTest - it will
make it easy to find a problem if the test fails.
@drewdzzz
Copy link
Collaborator Author

I've spent some time trying to fix this linter error, but I haven't found anything. Let's just ignore it.

@CuriousGeorgiy
Copy link
Member

@drewdzzz I guess it just wants you to put all the operands in one line. There is no column limit by default in WebKit. Probably we should set it to 80, 120 or some other reasonable value.

@alyapunov
Copy link
Contributor

I would still prefer using specific compiler versions in CI (fine to use the default available compiler versions). Up to @alyapunov.

I think we should have a goal to build on any compiler (perhaps starting from certain version). But we cannot test any compiler. I think the best thing we can do is to build with unspecified (perhaps latest) compiler version in order to have a change to detect build problems.

@alyapunov alyapunov merged commit c54d703 into tarantool:master Jan 29, 2024
36 of 37 checks passed
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.

Minimal support of C++20 Enable the undefined behaviour sanitizer
3 participants