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

Issue 26303002: Make Clang plugin check WeakPtrFactory member order (Closed)

Created:
7 years, 2 months ago by dmichael (off chromium)
Modified:
7 years, 2 months ago
Reviewers:
Elliot Glaysher
CC:
chromium-reviews
Visibility:
Public.

Description

Add Clang plugin check for WeakPtrFactory member order Also consolidates some of the FindBadConstruct options in to a small struct for readability. The WeakPtrFactory member order is behind a flag, because there are many violations in Chromium right now. Also, there might need to be exceptions in the checker. For example, we might want to allow more than one WeakPtrFactory to refer to the outer class, as long as all such factories appear after all other members. That's TBD. BUG=303818 R=erg@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=227528

Patch Set 1 #

Patch Set 2 : Clean up for review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+152 lines, -33 lines) Patch
M tools/clang/plugins/FindBadConstructs.cpp View 1 12 chunks +86 lines, -23 lines 0 comments Download
A + tools/clang/plugins/tests/weak_ptr_factory.h View 1 chunk +9 lines, -10 lines 0 comments Download
A tools/clang/plugins/tests/weak_ptr_factory.cpp View 1 chunk +49 lines, -0 lines 0 comments Download
A tools/clang/plugins/tests/weak_ptr_factory.flags View 1 chunk +1 line, -0 lines 0 comments Download
A tools/clang/plugins/tests/weak_ptr_factory.txt View 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
dmichael (off chromium)
7 years, 2 months ago (2013-10-07 18:11:02 UTC) #1
Elliot Glaysher
lgtm, but please note that there's going to be a delay between now and this ...
7 years, 2 months ago (2013-10-07 20:19:00 UTC) #2
dmichael (off chromium)
On 2013/10/07 20:19:00, Elliot Glaysher wrote: > lgtm, but please note that there's going to ...
7 years, 2 months ago (2013-10-07 20:30:56 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dmichael@chromium.org/26303002/9001
7 years, 2 months ago (2013-10-07 20:31:47 UTC) #4
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=206289
7 years, 2 months ago (2013-10-08 01:34:05 UTC) #5
dmichael (off chromium)
7 years, 2 months ago (2013-10-08 16:48:55 UTC) #6
Message was sent while issue was closed.
Committed patchset #2 manually as r227528 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698