New issue
Advanced search Search tips

Issue 643820 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Dec 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

rewrite_to_chrome_style: backslashes in macros are no longer lined up nicely

Project Member Reported by lukasza@chromium.org, Sep 2 2016

Issue description

In third_party/WebKit/Source/platform/heap/ThreadState.cpp
the tool rewrites something like this (with relatively nice alignment of the backslashes):

#define SNAPSHOT_HEAP(ArenaType)                                                                \
    {                                                                                          \
        numberOfHeapsReported++;                                                               \
        switch (type) {                                                                        \
        case SnapshotType::HeapSnapshot:                                                       \
            m_arenas[BlinkGC::ArenaType##ArenaIndex]->takeSnapshot(heapsDumpName + "/" #ArenaType, info);   \
            break;                                                                             \
        case SnapshotType::FreelistSnapshot:                                                   \
            m_arenas[BlinkGC::ArenaType##ArenaIndex]->takeFreelistSnapshot(heapsDumpName + "/" #ArenaType); \
            break;                                                                             \
        default:                                                                               \
            ASSERT_NOT_REACHED();                                                              \
        }                                                                                      \
    }

into something like this (notice the unaligned backslashes at the end of lines):

#define SNAPSHOT_HEAP(ArenaType)                                                                \
    {                                                                                          \
        number_of_heaps_reported++;                                                               \
        switch (type) {                                                                        \
        case SnapshotType::kHeapSnapshot:                                                       \
            arenas_[BlinkGC::ArenaType##ArenaIndex]->TakeSnapshot(heaps_dump_name + "/" #ArenaType, info);   \
            break;                                                                             \
        case SnapshotType::kFreelistSnapshot:                                                   \
            arenas_[BlinkGC::ArenaType##ArenaIndex]->TakeFreelistSnapshot(heaps_dump_name + "/" #ArenaType); \
            break;                                                                             \
        default:                                                                               \
            ASSERT_NOT_REACHED();                                                              \
        }                                                                                      \
    }
 
This seems low priority.  OTOH, I thought that we run 'git cl format' as part of running the rename tool (and that 'git cl format' would realign the backslashes).
I think we used to run git cl format but stopped, we will do that as a separate step. One tool to do one thing. Maybe this is just a note that we should git cl format the patch we commit after running the tool?

Comment 3 by dcheng@chromium.org, Dec 19 2016

Blocking: -578344
This isn't a blocker; we just need to git cl format (at some point) after running the tool.

Depending on the rebase strategy we end up using, we can include this with the giant renaming patch or not.

Comment 4 by danakj@chromium.org, Dec 19 2016

Status: WontFix (was: Untriaged)
Yeh everything will be poorly lined up with 80 cols after 

Sign in to add a comment