Skip to content

CI Update default arg tests #1263

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

Merged
merged 2 commits into from
Sep 23, 2024
Merged

Conversation

jarzec
Copy link
Contributor

@jarzec jarzec commented Aug 27, 2024

As stated in the title.

@@ -1,4 +1,4 @@
calling: int main(int, char**)
012
a newer compiler
an older compiler
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this intended? 14 is the most recent version of GCC released only a few months back.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a bug in

return gcc_ver > gcc || clang_ver > clang || msvc_ver > msvc;
and they should be >= rather than >.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does smell like it. @hsutter do you confirm, should I fix it in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

This test does not test whether gcc-14 is modern but whether you can test its version at compile time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but it is still a bit strange to see a very recent compiler marked as old.
To me the name gcc_clang_msvc_min_versions suggest that if the actual version is equal to the value stored internally the version should be accepted.

@jarzec jarzec force-pushed the ci-update-def-arg-test branch from 354e011 to bb433a2 Compare August 28, 2024 20:06
@@ -34,3 +34,6 @@ true
true
false
false

Make sure views::take works:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that GCC 14 is considered newer compiler it is allowed to do this as well.

@jarzec
Copy link
Contributor Author

jarzec commented Sep 4, 2024

@hsutter If you agree with the outcome of the discussions this test update is ready to go.
I will address other test failures on the main branch in a separate PR.

@hsutter
Copy link
Owner

hsutter commented Sep 23, 2024

Thanks!

@hsutter hsutter merged commit b3e7ef1 into hsutter:main Sep 23, 2024
28 of 29 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.

4 participants