Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(538)

Issue 1389723002: Remove unnecessary friend decls; fix Win-Clang builds (Closed)

Created:
5 years, 2 months ago by hans
Modified:
5 years, 2 months ago
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Remove unnecessary friend decls; fix Win-Clang builds Clang builds on Windows were failing with: ..\..\v8\src\register-configuration.cc(85,17) : error: unqualified friend declaration referring to type outside of the nearest enclosing namespace is a Microsoft extension; add a nested name specifier [-Werror,-Wmicrosoft-unqualified-friend] friend struct Register; ^ ::v8::internal:: How did it work on non-Windows? The friend declarations were declaring new Register and DoubleRegister structs in the current namespace, instead of refering the existing classes in the outer namespce. The code isn't referencing any private members of these classes anyway, so let's drop the friend declarations. BUG=82385 LOG=n Committed: https://crrev.com/57ca0f36c4ad575105f3a35505fd430874d575c9 Cr-Commit-Position: refs/heads/master@{#31113}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -3 lines) Patch
M src/register-configuration.cc View 1 chunk +0 lines, -3 lines 0 comments Download

Messages

Total messages: 13 (5 generated)
hans
Please take a look. This is currently breaking all Win-Clang builds, so it would be ...
5 years, 2 months ago (2015-10-05 18:05:16 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1389723002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1389723002/1
5 years, 2 months ago (2015-10-05 19:08:36 UTC) #4
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: v8_presubmit on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/6385)
5 years, 2 months ago (2015-10-05 19:11:48 UTC) #6
Michael Achenbach
Rubber-stamp lgtm to unblock you.
5 years, 2 months ago (2015-10-05 19:23:46 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1389723002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1389723002/1
5 years, 2 months ago (2015-10-05 19:24:40 UTC) #10
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 2 months ago (2015-10-05 19:34:39 UTC) #11
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/57ca0f36c4ad575105f3a35505fd430874d575c9 Cr-Commit-Position: refs/heads/master@{#31113}
5 years, 2 months ago (2015-10-05 19:35:05 UTC) #12
hans
5 years, 2 months ago (2015-10-05 19:40:50 UTC) #13
Message was sent while issue was closed.
On 2015/10/05 19:23:46, Michael Achenbach wrote:
> Rubber-stamp lgtm to unblock you.

Thanks!

Powered by Google App Engine
This is Rietveld 408576698