New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 802890 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

gcc (C+11) does not like constexpr function whose body is more than just return statement

Project Member Reported by js...@chromium.org, Jan 16 2018

Issue description

https://chromium-review.googlesource.com/c/chromium/deps/icu/+/836554
added a local change to cut down the ICU binary size.  

However, gcc does not like it:

From v8 trybots: I got this compile-time errors:

https://logs.chromium.org/v/?s=v8%2Fbuildbucket%2Fcr-buildbucket.appspot.com%2F8957206256878974800%2F%2B%2Fsteps%2Fcompile%2F0%2Fstdout


FAILED: obj/third_party/icu/icui18n/smallintformatter.o 
/b/swarming/w/ir/cache/goma_client/gomacc g++ -MMD -MF obj/third_party/icu/icui18n/smallintformatter.o.d -DU_I18N_IMPLEMENTATION -DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DCOMPONENT_BUILD -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -D_GLIBCXX_DEBUG=1 -DHAVE_DLOPEN=0 -DUCONFIG_ONLY_HTML_CONVERSION=1 -DU_CHARSET_IS_UTF8=1 -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t -I../.. -Igen -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin -pthread -m64 -march=x86-64 -O0 -fno-omit-frame-pointer -g2 -gsplit-dwarf -fno-builtin-abs -fvisibility=hidden -Wno-unused-local-typedefs -Wno-maybe-uninitialized -Wno-deprecated-declarations -fno-delete-null-pointer-checks -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-function -Wno-deprecated-declarations -std=gnu++11 -fno-exceptions -nostdinc++ -isystem../../buildtools/third_party/libc++/trunk/include -isystem../../buildtools/third_party/libc++abi/trunk/include --sysroot=../../build/linux/debian_stretch_amd64-sysroot -fvisibility-inlines-hidden -frtti -Wno-narrowing -c ../../third_party/icu/source/i18n/smallintformatter.cpp -o obj/third_party/icu/icui18n/smallintformatter.o
../../third_party/icu/source/i18n/smallintformatter.cpp: In function 'constexpr int digitCount(int32_t)':
../../third_party/icu/source/i18n/smallintformatter.cpp:30:1: error: body of constexpr function 'constexpr int digitCount(int32_t)' not a return-statement
 }
 ^
../../third_party/icu/source/i18n/smallintformatter.cpp: At global scope:
../../third_party/icu/source/i18n/smallintformatter.cpp:32:1: error: non-constant condition for static assertion
 static_assert(digitCount(0) == 1, "digitCount() is invalid");
 ^
../../third_party/icu/source/i18n/smallintformatter.cpp:32:27: error: 'constexpr int digitCount(int32_t)' called in a constant expression
 static_assert(digitCount(0) == 1, "digitCount() is invalid");
                           ^
../../third_party/icu/source/i18n/smallintformatter.cpp:33:1: error: non-constant condition for static assertion
 static_assert(digitCount(1) == 1, "digitCount() is invalid");
 ^
../../third_party/icu/source/i18n/smallintformatter.cpp:33:27: error: 'constexpr int digitCount(int32_t)' called in a constant expression
 static_assert(digitCount(1) == 1, "digitCount() is invalid");
                           ^


 

Comment 1 by js...@chromium.org, Jan 16 2018

Cc: ajwong@chromium.org brucedaw...@chromium.org

Comment 2 by js...@chromium.org, Jan 16 2018

Summary: gcc (C+11) does not like constexpr function whose body is just return statement (was: gcc does not like constexpr function whose body is just return statement)

Comment 3 by js...@chromium.org, Jan 16 2018

C++ 14 allows more complex constexpr functions, but C++ 11 does not. Apparently, gcc on problematic v8 trybots are configured to support only C++ 11. 

( http://en.cppreference.com/w/cpp/language/constexpr ). 

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/deps/icu.git/+/c8ca2962b46670ec89071ffd1291688983cd319c

commit c8ca2962b46670ec89071ffd1291688983cd319c
Author: Jungshik Shin <jshin@chromium.org>
Date: Tue Jan 16 23:19:54 2018

Revert "SmallIntFormatter: simplify implementation."

This reverts commit 2e0c90b4c98f3fa7d5b7a71cf4058afa589cb6e0.

It is to make gcc with '-std=gnu+11' happy. v8 trybots with gcc(C++ 11)
failed to compile with this CL.

Bug:  802890 
Test: None
Change-Id: I4c77d07281ebaefec9bb03fd4fba92e436009bd2
TBR=digit@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/869110
Reviewed-by: Jungshik Shin <jshin@chromium.org>

[modify] https://crrev.com/c8ca2962b46670ec89071ffd1291688983cd319c/README.chromium
[delete] https://crrev.com/899a6154c27b7c31445c25f2a2021f2716ba1960/patches/simplify_smallintformatter.patch
[modify] https://crrev.com/c8ca2962b46670ec89071ffd1291688983cd319c/source/i18n/smallintformatter.cpp

Comment 5 by js...@chromium.org, Jan 17 2018

Summary: gcc (C+11) does not like constexpr function whose body is more than just return statement (was: gcc (C+11) does not like constexpr function whose body is just return statement)

Comment 6 by digit@chromium.org, Jan 17 2018

Status: Started (was: Assigned)
Reland of the CL with a C++11 compatible constexpr:
https://chromium-review.googlesource.com/c/chromium/deps/icu/+/870039

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/363e1ec60c11f8037fbeca0addeb6eb68d3503fa

commit 363e1ec60c11f8037fbeca0addeb6eb68d3503fa
Author: Jungshik Shin <jshin@chromium.org>
Date: Mon Feb 05 23:26:23 2018

Roll ICU to d888fd2

$ git log c8ca296..d888fd2 --date=short --no-merges --format='%ad %ae
%s'
2018-02-05 jshin@chromium.org Update IANA tzdata to 2018c
2018-01-23 digit@google.com SmallIntFormatter: simplify implementation.

In JS console:
> new Date(Date.UTC(2018, 9,31, 5)).toLocaleString("en", {timeZone: 'America/Sao_Paulo'})
"10/31/2018, 2:00:00 AM"
> new Date(Date.UTC(2017, 9,31, 5)).toLocaleString("en", {timeZone: 'America/Sao_Paulo'})
"10/31/2017, 3:00:00 AM"

TBR=mark@chromium.org

Bug: 473288, 802890 
Test: See the above
Change-Id: I341d11a5b62bd7fa2d5473575591f50060c04870
Reviewed-on: https://chromium-review.googlesource.com/902806
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534533}
[modify] https://crrev.com/363e1ec60c11f8037fbeca0addeb6eb68d3503fa/DEPS

Comment 8 by js...@chromium.org, Feb 8 2018

Status: Fixed (was: Started)

Sign in to add a comment