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

Avoid a space in the UDL definition #3610

Merged
merged 9 commits into from
Sep 18, 2023
Merged

Conversation

danakj
Copy link
Contributor

@danakj danakj commented Aug 24, 2023

Clang 18 has grown a warning about the space being deprecated which
is enabled by default in their nightly binaries. However GCC before 4.9
will reject the UDL definition unless there is a space there, so we disable
the UDLs before GCC 4.9.

Closes #3607

Clang 18 has grown a warning about the space being deprecated which
is enabled by default in their nightly binaries. However GCC before 4.9
will reject the UDL definition unless there is a space there, so we need
to keep the space conditionally for it.
include/fmt/format.h Outdated Show resolved Hide resolved
GCC before 4.9 rejects the syntax that is now
rejected on more modern compilers.
@danakj danakj changed the title Avoid a space in the UDL definition except on GCC before 4.9 Avoid a space in the UDL definition Aug 25, 2023
danakj added 3 commits August 24, 2023 22:51
The compile tests should expect the UDL to
be there on GCC only for version 4.9 and above.
@danakj
Copy link
Contributor Author

danakj commented Aug 25, 2023

It's a bit tricky to get the UDL tests to correctly fail to compile on gcc 4.8, but probably just because I am very unfamiliar with this type of test harness.

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

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

Let's exclude gcc < 4.9 from building compile-error-test here:

if (FMT_PEDANTIC AND NOT WIN32)

and revert other changes to tests. It doesn't support compile-time checks anyway so the test is basically a noop there.

@vitaut
Copy link
Contributor

vitaut commented Sep 8, 2023

@danakj do you plan to update this PR?

@danakj
Copy link
Contributor Author

danakj commented Sep 8, 2023

Yes, sorry for the delay here.

@vitaut vitaut mentioned this pull request Sep 8, 2023
This reverts commit db49ecb.
This avoids the UDL tests failing as GCC < 4.9 can not parse UDLs
without a space, but the space is malformed in modern compilers.
@danakj danakj requested a review from vitaut September 18, 2023 02:53
@danakj
Copy link
Contributor Author

danakj commented Sep 18, 2023

Let's exclude gcc < 4.9 from building compile-error-test here:

if (FMT_PEDANTIC AND NOT WIN32)

and revert other changes to tests. It doesn't support compile-time checks anyway so the test is basically a noop there.

Done, and looks like gcc 4.8 and gcc 10 are both happy.

@vitaut vitaut merged commit f6ca4ea into fmtlib:master Sep 18, 2023
40 checks passed
@vitaut
Copy link
Contributor

vitaut commented Sep 18, 2023

Thank you!

@danakj
Copy link
Contributor Author

danakj commented Sep 18, 2023

No problem, thank you as well.

@danakj danakj deleted the udl-whitespace branch September 18, 2023 16:10
ckerr pushed a commit to transmission/fmt that referenced this pull request Nov 7, 2023
* Avoid a space in the UDL definition except on GCC before 4.9

Clang 18 has grown a warning about the space being deprecated which
is enabled by default in their nightly binaries. However GCC before 4.9
will reject the UDL definition unless there is a space there, so we need
to keep the space conditionally for it.

* Remove UDLs on GCC before 4.9 to simplify things

GCC before 4.9 rejects the syntax that is now
rejected on more modern compilers.

* Disable compile-error-test on GCC < 4.9

This avoids the UDL tests failing as GCC < 4.9 can not parse UDLs
without a space, but the space is malformed in modern compilers.
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.

Clang 18 rejects operator"" _a
2 participants