1509926 - clang warns about UTF-8 arrays having changed type to char8_t ("warning: type of UTF-8 string literal will change from array of const char to array of const char8_t in C++2a [-Wc++2a-compat]" and "note: remove 'u8' prefix to avoid a change of behavior")

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Daniel Holbert [:dholbert]]

Clang trunk had a recent change that has introduced some warning-spam for some of our tests.  Specifically, this warning:

======
xpcom/tests/gtest/TestStrings.cpp:1392:24: warning: type of UTF-8 string literal will change from array of const char to array of const char8_t in C++2a [-Wc++2a-compat]
   EXPECT_TRUE(s.Equals(u8"\uFFFD"));
                        ^
======


...and this "note" (and zillions of others like it for every ExpectValidCodePoint call in this test):
======
mfbt/tests/TestUtf8.cpp:430:24: note: remove 'u8' prefix to avoid a change of behavior; Clang encodes unprefixed narrow string literals as UTF-8
   ExpectValidCodePoint(u8"𝛗", 0x1D6D7); // MATHEMATICAL BOLD SMALL PHI
                        ^~
=====



This was introduced by this clang change:
  https://reviews.llvm.org/rL346892

We'll presumably need some serious code changes to correctly adapt to that commit (using char8_t in our codebase for compilers that support it -- perhaps with a typedef that falls back to `char` elsewhere, or something).  But for now, we might want to build with -fno-char8_t to turn off these warnings & the behavior that they're talking about...

Comment 1

[Daniel Holbert [:dholbert]]

(I ran into this with locally-built "clang version 8.0.0 (trunk 347571)", BTW.)

Comment 2

[Mike Hommey [:glandium]]

That clang change only enables -fno-char8_t when building with -std=c++2a, which we aren't doing. The problem here is that we're passing -Wc++2a-compat, which enables the warning, and along -Werror, that makes the build fail. Maybe we should use -Wno-error=c++2a-compat?

Comment 3

[Mike Hommey [:glandium]]

Oh, you were complaining about the amount of warnings. -Wno-error wouldn't help with that. (I had the same problem on automation when trying some clang patch with clang trunk, but because of -Werror, that was failing the build).

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Mike Hommey [:glandium] from comment #2)
> That clang change only enables -fno-char8_t when building with -std=c++2a,
> which we aren't doing. The problem here is that we're passing
> -Wc++2a-compat, which enables the warning, and along -Werror, that makes the
> build fail. Maybe we should use -Wno-error=c++2a-compat?

We should probably just stop enabling -Wc++2a-compat. Using -Wno-error=c++2a-compat will fix the compilation errors, but build logs will still be flooded with "zillions" of these warnings. That would be annoying and people would just ignore them.

When we eventually build with -std=c++2a, u8"𝛗" should work without any warnings. There is no code for us to fix now or then.

Comment 5

[Chris Peterson [:cpeterson]]

I only see ~70 uses of u8"" string literals:

* 54 from ExpectValidCodePoint() in TestUtf8.cpp
* 2 from widget/android/EventDispatcher.cpp
* and the rest from third-party ICU code

https://searchfox.org/mozilla-central/search?q=u8%22&case=true&regexp=false&path=.cpp

If there are only this many warnings, maybe we can just suppress the ExpectValidCodePoint() warnings locally in TestUtf8.cpp with:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wc++2a-compat"

#pragma clang diagnostic pop

Comment 6

[Mike Hommey [:glandium]]

and disable the c++2a-compat warnings in ICU.

Comment 7

[Chris Peterson [:cpeterson]]

Daniel, since you see the warnings, what option do you is most practical? I'm using Homebrew's clang on macOS, which is still version 7 so I don't see the char8_t warnings yet.

1. Remove -Wc++2a-compat altogether.
2. Use -Wno-error=c++2a-compat so they are not errors.
3. Suppress the char8_t warnings just for TestUtf8.cpp and ICU.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

I know you didn't ask me :) but I prefer option 3). 
We don't know what else will come under the -Wc++2a-compat umbrella and only the third option will correctly notify us.

Comment 9

[Daniel Holbert [:dholbert]]

The noise is what bothers me, so I'd rule out option (2).

I don't have a strong/educated leaning between options 1 vs 3.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1509926 - Disable the warning -Wc++2a-compat on some files r?dholbert

Fails with clang trunk:
"type of UTF-8 string literal will change from array of const char to array of const char8_t in C++2a"
otherwise

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f0b091b21fdd
Disable the warning -Wc++2a-compat on some files r=dholbert

Comment 12

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/f0b091b21fdd