Closed Bug 1509926 Opened 5 years ago Closed 5 years ago
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")
47 bytes, text/x-phabricator-request
|Details | Review|
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...
(I ran into this with locally-built "clang version 8.0.0 (trunk 347571)", BTW.)
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?
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).
(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.
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®exp=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
and disable the c++2a-compat warnings in ICU.
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.
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.
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.
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
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/f0b091b21fdd Disable the warning -Wc++2a-compat on some files r=dholbert
You need to log in before you can comment on or make changes to this bug.