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

Issue 7817005: Convert some constants declared as anonymous enums into static consts so they have types. This d... (Closed)

Created:
9 years, 3 months ago by Peter Kasting
Modified:
9 years, 3 months ago
Reviewers:
agl
CC:
chromium-reviews, darin-cc_chromium.org, jam, Paweł Hajdan Jr.
Visibility:
Public.

Description

Convert some constants declared as anonymous enums into static consts so they have types. This defines the constants where they're declared to preserve the existing readability as well as allow us to do things like dimension arrays based on the values of the constants. The drawback to defining constants at their declaration point is that supplying them to a templated function, like what DCHECK_EQ() expands into, triggers an "undefined symbol" error on Mac/Linux (and adding explicit storage for them in the .cc file can cause duplicate symbol errors on Windows). Here I've worked around that by converting DCHECK_EQ(a, b) to DCHECK(b == a). The original motiviation for this change was to find a way to eliminate some cases of passing anonymous-typed values as template arguments (which happens when you use a value from the enum in e.g. EXPECT_EQ()), which is technically illegal in C++03, though we don't warn about it. Simply naming the enum would have done this, but in general naming enums used to declare constants like this is bizarre ("enum Constants { ... }"?). BUG=92247 TEST=Compiles Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=99087

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -30 lines) Patch
M ipc/file_descriptor_set_posix.h View 1 chunk +1 line, -3 lines 0 comments Download
M ipc/file_descriptor_set_posix.cc View 3 chunks +4 lines, -4 lines 0 comments Download
M ipc/file_descriptor_set_posix_unittest.cc View 1 chunk +1 line, -3 lines 0 comments Download
M ipc/ipc_channel.h View 1 chunk +5 lines, -7 lines 0 comments Download
M ipc/ipc_channel_posix.h View 1 chunk +6 lines, -7 lines 0 comments Download
M ipc/ipc_channel_posix.cc View 5 chunks +5 lines, -6 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Peter Kasting
9 years, 3 months ago (2011-08-31 21:16:28 UTC) #1
agl
9 years, 3 months ago (2011-08-31 21:24:57 UTC) #2
LGTM

Powered by Google App Engine
This is Rietveld 408576698