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

Updated github CI only runs against C++11 standard #1987

Closed
jgopel opened this issue Nov 6, 2020 · 5 comments
Closed

Updated github CI only runs against C++11 standard #1987

jgopel opened this issue Nov 6, 2020 · 5 comments

Comments

@jgopel
Copy link
Contributor

jgopel commented Nov 6, 2020

When switching from Travis to github CI (roughly cba5970), it seems like the testing for standards other than C++11 was lost.

This issue is relevant because compilers behave differently at different standards. For example, with clang 11.0.0, a pedantic build of fmt succeeds in C++11 mode but fails with the following error in C++20 mode.

In file included from /Users/jonathan/work/fmt/test/compile-test.cc:16:
/Users/jonathan/work/fmt/include/fmt/compile.h:518:25: error: implicit conversion changes signedness: 'long' to 'unsigned long' [-Werror,-Wsign-conversion]
  return {f, pos + (end - str.data()) + 1, ctx.next_arg_id()};
                 ~  ~~~~^~~~~~~~~~~~
/Users/jonathan/work/fmt/include/fmt/compile.h:538:31: note: in instantiation of function template specialization 'fmt::v7::detail::parse_specs<int, char>' requested here
      constexpr auto result = parse_specs<id_type>(str, POS + 2, ID);
                              ^
/Users/jonathan/work/fmt/include/fmt/compile.h:569:17: note: in instantiation of function template specialization 'fmt::v7::detail::compile_format_string<fmt::v7::detail::type_list<int>, 0, 0, FMT_COMPILE_STRING>' requested here
        detail::compile_format_string<detail::type_list<Args...>, 0, 0>(
                ^
/Users/jonathan/work/fmt/include/fmt/compile.h:648:37: note: in instantiation of function template specialization 'fmt::v7::detail::compile<int, FMT_COMPILE_STRING, 0>' requested here
  constexpr auto compiled = detail::compile<Args...>(S());
                                    ^
/Users/jonathan/work/fmt/test/compile-test.cc:140:24: note: in instantiation of function template specialization 'fmt::v7::format<FMT_COMPILE_STRING, int, 0>' requested here
  EXPECT_EQ("42", fmt::format(FMT_COMPILE("{:x}"), 0x42));
                       ^

I think the correct fix here is to add C++ standards to the build matrix and I would be happy to submit a PR for that work, if there's interest. If that's not palatable then I would like to see the setup should return to the way it was previously with C++11 being tested in g++-4.8, C++14 being tested in gcc-6, and C++17 being tested in gcc-8.

@vitaut
Copy link
Contributor

vitaut commented Nov 6, 2020

#1969

@vitaut vitaut closed this as completed Nov 6, 2020
@jgopel
Copy link
Contributor Author

jgopel commented Nov 6, 2020

As I read it, that issue is about moving to Actions. This issue is about how that move to actions changed the behavior of testing. Can you help me understand how this issue duplicates that one?

@vitaut
Copy link
Contributor

vitaut commented Nov 6, 2020

The migration from Travis CI to Github Actions is still in progress and is tracked in #1969. Configuration parity is just one minor aspect of it.

@jgopel
Copy link
Contributor Author

jgopel commented Nov 6, 2020

Thank you for the explanation. If you'd like help with that, I would be interested in contributing.

@vitaut
Copy link
Contributor

vitaut commented Nov 6, 2020

If you'd like help with that, I would be interested in contributing.

Sure, a PR to add a C++17 config is welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants