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

Silence one build-time warning #18671

Merged
merged 1 commit into from Apr 1, 2021
Merged

Conversation

jkeenan
Copy link
Contributor

@jkeenan jkeenan commented Mar 30, 2021

Observed in clang 9 and 10.

Partial solution for #17015

Observed in clang 9 and 10.

Partial solution for #17015
@jkeenan
Copy link
Contributor Author

jkeenan commented Mar 30, 2021

Given the discussion in #17015, I'm surprised that a solution to one of the build-time warnings discussed in that ticket was as simple as putting parentheses around one term.

However, smoke-test results suggest that this did no harm.

@jkeenan jkeenan requested review from hvds and tonycoz March 30, 2021 14:44
@hvds
Copy link
Contributor

hvds commented Mar 30, 2021

Oh! I had completely misunderstood what was being warned about here, how embarrassing - and my apologies for repeatedly denigrating clang's warnings based on that misunderstanding.

I now realise that what it is really complaining about is this bit of C, and I have no idea how it would be interpreted:

#define UVuf "lu"  /* typically */
#define UTF8f "d%" UVuf "%4p"
(UTF8f + 2)

I've never understood C string concatenation; but if the last line is not interpreted the same as ((UTF8f) + 2), then perhaps a more correct and complete fix would instead be to move the parens into the macro definition:

#define UTF8f ("d%" UVuf "%4p")

However I don't know if that risks causing problems for the more common case where we're concatenating it further.

I'll defer to those who know the C (or preprocessor?) spec better.

@tonycoz
Copy link
Contributor

tonycoz commented Apr 1, 2021

I've never understood C string concatenation; but if the last line is not interpreted the same as ((UTF8f) + 2), then perhaps a more correct and complete fix would instead be to move the parens into the macro definition:

#define UTF8f ("d%" UVuf "%4p")

However I don't know if that risks causing problems for the more common case where we're concatenating it further.

I'll defer to those who know the C (or preprocessor?) spec better.

I'm pretty sure that would break concatenating it into larger string literals.

The relevant part of the standard (6.4.5 paragraph 5)

In translation phase 6, the multibyte character sequences specified by any sequence of adjacent character and identically-prefixed string literal tokens are concatenated into a single multibyte character sequence. If any of the tokens has an encoding prefix, the resulting multibyte character sequence is treated as having the same prefix; otherwise, it is treated as a character string literal. Whether differently-prefixed wide string literal tokens can be concatenated and, if so, the treatment of the resulting multibyte character sequence are implementation-defined.

and the ( between the string literal tokens would make them non-adjacent.

@jkeenan jkeenan merged commit efceac5 into blead Apr 1, 2021
@jkeenan jkeenan deleted the smoke-me/jkeenan/gh-17015-sv-c-warning branch April 1, 2021 01:19
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.

None yet

3 participants