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

Issue 1154523004: Everyone gets a namespace {}. (Closed)

Created:
5 years, 7 months ago by mtklein_C
Modified:
5 years, 7 months ago
Reviewers:
mtklein
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Everyone gets a namespace {}. If we include Sk4px.h, SkPMFloat.h, or SkNx.h into files with different SIMD flags, that could cause different definitions of the same method. Normally that's moot, because all the code inlines, but in Debug it tends not to. So in Debug, the linker picks one definition for us. That breaks _someone_. Wrapping everything in a namespace {} keeps the definitions separate. Tested locally, it fixes this bug. BUG=skia:3861 This code is not yet enabled in Chrome, so shouldn't affect the roll. NOTREECHECKS=true Committed: https://skia.googlesource.com/skia/+/aa999cb952753d373c03befa7fce8c4700ef9308

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+60 lines, -2 lines) Patch
M src/core/Sk4px.h View 2 chunks +9 lines, -0 lines 0 comments Download
M src/core/SkNx.h View 2 chunks +8 lines, -0 lines 0 comments Download
M src/core/SkPMFloat.h View 2 chunks +9 lines, -0 lines 0 comments Download
M src/opts/Sk4px_NEON.h View 2 chunks +4 lines, -0 lines 0 comments Download
M src/opts/Sk4px_SSE2.h View 2 chunks +4 lines, -0 lines 0 comments Download
M src/opts/Sk4px_none.h View 2 chunks +3 lines, -0 lines 0 comments Download
M src/opts/SkNx_neon.h View 2 chunks +3 lines, -1 line 0 comments Download
M src/opts/SkNx_sse.h View 2 chunks +4 lines, -1 line 0 comments Download
M src/opts/SkPMFloat_SSE2.h View 2 chunks +4 lines, -0 lines 0 comments Download
M src/opts/SkPMFloat_SSSE3.h View 2 chunks +4 lines, -0 lines 0 comments Download
M src/opts/SkPMFloat_neon.h View 2 chunks +4 lines, -0 lines 0 comments Download
M src/opts/SkPMFloat_none.h View 2 chunks +4 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (2 generated)
mtklein
lgtm
5 years, 7 months ago (2015-05-23 00:12:24 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1154523004/1
5 years, 7 months ago (2015-05-23 00:12:38 UTC) #4
commit-bot: I haz the power
5 years, 7 months ago (2015-05-23 00:18:29 UTC) #5
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/aa999cb952753d373c03befa7fce8c4700ef9308

Powered by Google App Engine
This is Rietveld 408576698