New issue
Advanced search Search tips

Issue 641033 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 578344



Sign in to add a comment

rewrite_to_chrome_style: rename of enum values in DisplayItem.h clashes with macro token concatenation

Project Member Reported by lukasza@chromium.org, Aug 25 2016

Issue description

Old:
  enum Type {
    ClipFirst
    ClipLast
    EndClipFirst
    EndClipLast
    FloatClipFirst
    FloatClipLast
    EndFloatClipFirst
    EndFloatClipLast
    ScrollFirst
    ScrollLast
    EndScrollFirst
    EndScrollLast

New: |k| gets prepended to all the identifiers below.

Problem: This clashes with how macros try to reconstruct the identifiers below and requires the following patch after running the tool:

     // See comments of enum Type for usage of the following macros.
 #define DEFINE_CATEGORY_METHODS(Category) \
-    static bool is##Category##Type(Type type) { return type >= Category##First && type <= Category##Last; } \
+    static bool is##Category##Type(Type type) { return type >= k##Category##First && type <= k##Category##Last; } \
     bool is##Category() const { return is##Category##Type(GetType()); }
 
 #define DEFINE_CONVERSION_METHODS(Category1, category1, Category2, category2) \
     static Type category1##TypeTo##Category2##Type(Type type) \
     { \
-        static_assert(Category1##Last - Category1##First == Category2##Last - Category2##First, \
+        static_assert(k##Category1##Last - k##Category1##First == k##Category2##Last - k##Category2##First, \
             "Categories " #Category1 " and " #Category2 " should have same number of enum values. See comments of DisplayItem::Type"); \
         ASSERT(is##Category1##Type(type)); \
-        return static_cast<Type>(type - Category1##First + Category2##First); \
+        return static_cast<Type>(type - k##Category1##First + k##Category2##First); \
     } \
     static Type category2##TypeTo##Category1##Type(Type type) \
     { \
         ASSERT(is##Category2##Type(type)); \
-        return static_cast<Type>(type - Category2##First + Category1##First); \
+        return static_cast<Type>(type - k##Category2##First + k##Category1##First); \
     }
 
 #define DEFINE_PAIRED_CATEGORY_METHODS(Category, category) \
@@ -271,9 +271,9 @@ public:
 #define DEFINE_PAINT_PHASE_CONVERSION_METHOD(Category) \
     static Type paintPhaseTo##Category##Type(int paint_phase) \
     { \
-        static_assert(Category##PaintPhaseLast - Category##PaintPhaseFirst == kPaintPhaseMax, \
+        static_assert(k##Category##PaintPhaseLast - k##Category##PaintPhaseFirst == kPaintPhaseMax, \
             "Invalid paint-phase-based category " #Category ". See comments of DisplayItem::Type"); \
-        return static_cast<Type>(paint_phase + Category##PaintPhaseFirst); \
+        return static_cast<Type>(paint_phase + k##Category##PaintPhaseFirst); \
     }

 

Comment 1 by dcheng@chromium.org, Aug 25 2016

Let's just pre-emptively rename this one so we can fix the macro while we're at it.
Manual renaming proposed in https://codereview.chromium.org/2286843002 (no self-review, still running CQ so not yet ready for review).  Notes (please shout if anything doesn't look right):

- Only renaming enum constants under DisplayItem::Type enum

    - Leaving method names created by token concatenation intact

        - This doesn't hurt the tool, because unlike constant names such method names do not occur outside of token concatenation and therefore won't get renamed).

        - OTOH, the style of those methods for now remains as isClipType rather than IsClipType.  Maybe those can be tackled in a separate CL?

    - Leaving other enums intact (because these others enums are tackled by the tool correctly AFAIR)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 30 2016

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

commit 690c132cbec6ad770573f9157105fc6a041abd52
Author: lukasza <lukasza@chromium.org>
Date: Tue Aug 30 22:45:29 2016

Rename DisplayItem::Type enum constants to Chromium style.

This CL renames constants in
third_party/WebKit/Source/platform/graphics/paint/DisplayItem.h under
blink::DisplayItem::Type enum, so that the constants follow Chromium
naming style (i.e. kPageOverlay instead of PageOverlay).

This is done as a separate CL, because the automated, clang-based
renaming tool cannot handle names built via macro token concatenation
(and this macro feature is used heavily in
platform/graphics/paint/DisplayItem.h).

BUG= 641033 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2286843002
Cr-Commit-Position: refs/heads/master@{#415471}

[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/editing/CaretBase.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/editing/DragCaretController.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/editing/FrameCaret.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/layout/LayoutScrollbarTheme.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/page/PrintContextTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/BlockPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/BoxClipper.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/BoxPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/BoxReflectionUtils.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/FileUploadControlPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/FilterPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/FramePainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/HTMLCanvasPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/ImagePainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/InlineFlowBoxPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/LayerClipRecorderTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/LayoutObjectDrawingRecorderTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/MultiColumnSetPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/ObjectPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/PaintControllerPaintTest.h
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/PaintLayerPainterTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/PaintPhase.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/ReplacedPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/RoundedInnerRectClipper.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/SVGClipPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/SVGFilterPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/SVGMaskPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/ScrollableAreaPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/TableCellPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/TableCellPainterTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/TableRowPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/TableSectionPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/VideoPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/paint/ViewPainter.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/exported/WebFont.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/ClipPathDisplayItem.h
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/CompositingDisplayItem.h
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/CompositingRecorder.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/DisplayItem.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/DisplayItem.h
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/DisplayItemListTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/DisplayItemTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/FilterDisplayItem.h
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/PaintChunkTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/PaintChunkerTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/ScopedPaintChunkProperties.h
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/SubsequenceDisplayItem.h
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/graphics/paint/TransformDisplayItem.h
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/scroll/ScrollbarTheme.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/scroll/ScrollbarThemeAura.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/scroll/ScrollbarThemeMac.mm
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/scroll/ScrollbarThemeMock.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlay.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/platform/testing/TestPaintArtifact.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/web/PageOverlayTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/web/PageWidgetDelegate.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/web/WebPluginContainerImpl.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/web/tests/DocumentLoadingRenderingTest.cpp
[modify] https://crrev.com/690c132cbec6ad770573f9157105fc6a041abd52/third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp

Status: Fixed (was: Untriaged)

Sign in to add a comment