Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1820583002:
Use C++11 / gnu++11, not 0x (Closed)
DescriptionUse C++11 / gnu++11, not 0x
Cleanup, and matches Chromium's build.
Committed: https://crrev.com/890f3dd7c5df5ec330a010928db0b302b8e005f0
Cr-Commit-Position: refs/heads/master@{#34993}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address comment; more in v8config.h #
Messages
Total messages: 31 (14 generated)
The CQ bit was checked by jfb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1820583002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1820583002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
jfb@chromium.org changed reviewers: + machenbach@google.com
machenbach: is there are reason we can't update these? Bots seem happy. There are a few other places in the code I can fix that mention 0x.
machenbach@chromium.org changed reviewers: + jochen@chromium.org, machenbach@chromium.org, thakis@chromium.org
I'm fine with this, but I'd like to get some opinions from Nico and Jochen.
This lgtm. I kind of thought v8 was on the newer spelling already after https://codereview.chromium.org/1637473003/ Chromium currently uses gnu++11 instead of c++11 because breakpad still uses typeof on linux, but long-term we want to use c++11 everywhere (https://bugs.chromium.org/p/chromium/issues/detail?id=427584). v8 builds fine with std=c++11 afaik, but jochen said previously he wants to match chromium's config, so I'd only switch to c++11 on mac like mentioned below https://codereview.chromium.org/1820583002/diff/1/build/standalone.gypi File build/standalone.gypi (right): https://codereview.chromium.org/1820583002/diff/1/build/standalone.gypi#newco... build/standalone.gypi:984: 'CLANG_CXX_LANGUAGE_STANDARD': 'gnu++11', # -std=gnu++11 chromium uses c++11 on mac, and v8 tries to match chromium, so i'd change this to std=c++11
(...is v8 built with the nacl compiler? i think gcc4.6 -- which nacl is based on -- doesn't understand the gnu++11 spelling and needs gnu++0x. But if the bots are happy, I suppose it's worth trying to change this.)
The CQ bit was checked by jfb@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org Link to the patchset: https://codereview.chromium.org/1820583002/#ps20001 (title: "Address comment; more in v8config.h")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1820583002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1820583002/20001
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. Committers are members of the group "project-v8-committers". Note that this has nothing to do with OWNERS files.
I also adjusted v8config.h accordingly. I think there are a few other places where we can trim things, but I'd rather do these simplifications in their own follow-up patches if this change sticks. thakis: I thought the V8 NaCl build had bitrotted and wasn't supported anymore? That would be a pretty big holdback, if it's still expected to work then we could move it to the PNaCl LLVM toolchain (which has a nacl-clang mode). https://codereview.chromium.org/1820583002/diff/1/build/standalone.gypi File build/standalone.gypi (right): https://codereview.chromium.org/1820583002/diff/1/build/standalone.gypi#newco... build/standalone.gypi:984: 'CLANG_CXX_LANGUAGE_STANDARD': 'gnu++11', # -std=gnu++11 On 2016/03/21 14:40:42, Nico wrote: > chromium uses c++11 on mac, and v8 tries to match chromium, so i'd change this > to std=c++11 Done.
The CQ bit was checked by jfb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1820583002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1820583002/20001
Description was changed from ========== Try C++11 not 0x ========== to ========== Use C++11 / gnu++11, not 0x Cleanup, and matches Chromium's build. ==========
I'm guessing you know way more about v8 nacl builds then me :-)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I'll need an lgtm from machenbach or someone else who's a V8 owner.
jkummerow@chromium.org changed reviewers: + jkummerow@chromium.org
lgtm
The CQ bit was checked by jfb@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1820583002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1820583002/20001
Message was sent while issue was closed.
Description was changed from ========== Use C++11 / gnu++11, not 0x Cleanup, and matches Chromium's build. ========== to ========== Use C++11 / gnu++11, not 0x Cleanup, and matches Chromium's build. ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Use C++11 / gnu++11, not 0x Cleanup, and matches Chromium's build. ========== to ========== Use C++11 / gnu++11, not 0x Cleanup, and matches Chromium's build. Committed: https://crrev.com/890f3dd7c5df5ec330a010928db0b302b8e005f0 Cr-Commit-Position: refs/heads/master@{#34993} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/890f3dd7c5df5ec330a010928db0b302b8e005f0 Cr-Commit-Position: refs/heads/master@{#34993} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
