Assertion `overrides_blink != overrides_non_blink' failed |
||||
Issue descriptionThis fires for third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp and most files it seems when running the tool on bindings/core/v8.
,
Mar 9 2016
Ah it was destructors, which we exclude, but after we checked if they are a blink method. Reordering solves it.
,
Mar 9 2016
,
Mar 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e27ff645c0bfb460855b825f08beda583d71a8e5 commit e27ff645c0bfb460855b825f08beda583d71a8e5 Author: danakj <danakj@chromium.org> Date: Wed Mar 09 21:29:59 2016 rewrite_to_chrome_style: Exclude methods before checking IsBlinkOrWTF The IsBlinkOrWTFMethod asserts that the method is not doing crazy overrides, but it will fire for destructors which we aren't trying to rename anyways. So exclude those before trying to see if we want to rename it. R=dcheng BUG= 593446 Review URL: https://codereview.chromium.org/1778963003 Cr-Commit-Position: refs/heads/master@{#380228} [modify] https://crrev.com/e27ff645c0bfb460855b825f08beda583d71a8e5/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
,
Mar 9 2016
Still happening rewrite_to_chrome_style: /usr/local/google/home/danakj/s/c/src/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:121: bool (anonymous namespace)::IsBlinkOrWTFMethod(const clang::CXXMethodDecl &): Assertion `overrides_blink != overrides_non_blink' failed.
,
Mar 9 2016
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/render_view_impl.h&l=447 RenderViewImpl::convertViewportToWindow overrides: - A virtual method in WebWidgetClient (from blink) - A purevirtual method in RenderView (not from blink) - A overridden method in RenderWidget (not from blink) - A virtual method in WebWidgetClient (again?) - An overridden method in RenderViewImpl (not from blink) ... I feel like codesearch makes an inheritance cycle at some point.
,
Mar 9 2016
OK It's RenderWidget inherits WebWidgetClient - provides convertViewportToWindow RenderViewImpl inherits RenderView - wants to expose RenderWidget's convertViewportToWindow - does so by making a virtual on RenderView with the *SAME NAME* - the override in RenderViewImpl calls RenderWidget's T_T Going to rename the one on RenderView.
,
Mar 9 2016
Another one. InspectorInspectorAgent inherits a disable() method from: - InspectorAgent (which is in blink::) https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/inspector/InspectorBaseAgent.h&rcl=1457541024&l=63 - Dispatcher::InspectorCommandHandler (which is in blink::protocol:: and is in gen/ so we don't rename it) https://code.google.com/p/chromium/codesearch#chromium/src/out/Debug/gen/blink/platform/inspector_protocol/Dispatcher.h&rcl=1457541024&l=42
,
Mar 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dea2efb645e7ac6bae644658f963b096c2800652 commit dea2efb645e7ac6bae644658f963b096c2800652 Author: danakj <danakj@chromium.org> Date: Thu Mar 10 19:13:17 2016 content: Rename virtual method on RenderView to not (ab)use blink style The RenderView::convertViewportToWindow method uses blink style to create a horrorshow inheritance hierarchy problem, where the same named method in RenderViewImpl inherits this method from two places (one in blink one in content) from RenderView and from RenderWidget. Fix this by renaming the method on RenderView to not use the same name as the blink method (even after blink uses chromium style). R=avi@chromium.org,jochen@chromium.org BUG= 593446 Review URL: https://codereview.chromium.org/1781873002 Cr-Commit-Position: refs/heads/master@{#380441} [modify] https://crrev.com/dea2efb645e7ac6bae644658f963b096c2800652/content/public/renderer/render_view.h [modify] https://crrev.com/dea2efb645e7ac6bae644658f963b096c2800652/content/renderer/browser_plugin/browser_plugin.cc [modify] https://crrev.com/dea2efb645e7ac6bae644658f963b096c2800652/content/renderer/render_view_impl.cc [modify] https://crrev.com/dea2efb645e7ac6bae644658f963b096c2800652/content/renderer/render_view_impl.h [modify] https://crrev.com/dea2efb645e7ac6bae644658f963b096c2800652/extensions/renderer/extension_helper.cc
,
Mar 10 2016
And one in Source/core/testing/InternalSettings.h: InternalSettings defines a virtual trace() method via the DECLARE_VIRTUAL_TRACE() macro. InternalSettings derives from: 1) InternalSettingsGenerated, which uses DECLARE_VIRTUAL_TRACE() to make a trace() function also. This class lives in gen/ so it will not be renamed (we think, except it would cuz it's a macro..) 2)_HeapSupplement which derives from SupplementableBase<T, true> which derives from SupplementableTracing<T, isGarbageCollected>. - SupplementableTracing is in platform/ and uses DEFINE_INLINE_VIRTUAL_TRACE() to define a virtual trace() method.
,
Mar 10 2016
This one can be solved be teaching the tool to ignore trace() a little earlier.
,
Mar 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e8821e2f26fadc019c3b82807854d03ae203d5de commit e8821e2f26fadc019c3b82807854d03ae203d5de Author: danakj <danakj@chromium.org> Date: Fri Mar 11 20:43:51 2016 rewrite_to_chrome_style: Consistently decide if things are in blink. The matcher rules would check IsInBlink and then exclude stuff in gen/. But the method-matcher is special and it has to look at overrides, and it would look check IsInBlink but would not exclude gen/. 2ndly, the matcher rules would check that any ancestor namespace was blink or WTF, but the method-matcher would only check the first namespace. This makes them consistent. IsInBlinkOrWTF will always return false for things in gen/, both for the matcher rules and for the method-matcher. And both will check any ancestor namespace, not just the next namespace. R=dcheng BUG= 593446 Review URL: https://codereview.chromium.org/1785563002 Cr-Commit-Position: refs/heads/master@{#380720} [modify] https://crrev.com/e8821e2f26fadc019c3b82807854d03ae203d5de/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp [modify] https://crrev.com/e8821e2f26fadc019c3b82807854d03ae203d5de/tools/clang/rewrite_to_chrome_style/tests/gen/thing.h [modify] https://crrev.com/e8821e2f26fadc019c3b82807854d03ae203d5de/tools/clang/rewrite_to_chrome_style/tests/methods-expected.cc [modify] https://crrev.com/e8821e2f26fadc019c3b82807854d03ae203d5de/tools/clang/rewrite_to_chrome_style/tests/methods-original.cc
,
Mar 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/27f1728f9c2cdfb9995c671783edd55bd66470be commit 27f1728f9c2cdfb9995c671783edd55bd66470be Author: danakj <danakj@chromium.org> Date: Fri Mar 11 21:21:07 2016 rewrite_to_chrome_style: Remove blacklisted methods with matcher Consider blacklisted methods when looking at deciding to match against them or not rather than deciding to not rename them much later. R=dcheng BUG= 593446 Review URL: https://codereview.chromium.org/1783923003 Cr-Commit-Position: refs/heads/master@{#380735} [modify] https://crrev.com/27f1728f9c2cdfb9995c671783edd55bd66470be/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
,
Mar 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec30d6b644ed9cd739b3b65efca762e1b1846cfc commit ec30d6b644ed9cd739b3b65efca762e1b1846cfc Author: danakj <danakj@chromium.org> Date: Fri Mar 11 23:00:07 2016 rewrite_to_chrome_style: Blacklist InspectorAgent::disable This method overrides stuff in gen/ and stuff from blink:: and if we rename it without renaming both we'll break things. So don't rename it at all. R=dcheng BUG= 593446 Review URL: https://codereview.chromium.org/1782293003 Cr-Commit-Position: refs/heads/master@{#380773} [modify] https://crrev.com/ec30d6b644ed9cd739b3b65efca762e1b1846cfc/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
,
Mar 11 2016
,
Jan 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/535b89a267d880c41efed60526213483ae79e707 commit 535b89a267d880c41efed60526213483ae79e707 Author: lukasza <lukasza@chromium.org> Date: Thu Jan 19 19:33:55 2017 Make sure InspectorBaseAgent::disable is not renamed. When we started blacklisting renaming of "disable" method (in r380773) the virtual method was implemented by InspectorAgent class. Since then (in 396629) the virtual method was moved into InspectorBaseAgent class and consequently blacklisting stopped working - this CL fixes this. I've tested this CL by running the clang tool on third_party/WebKit/Source/modules/storage/StorageNamespaceController.cpp and verifying that "disable" method in core/inspector/InspectorBaseAgent.h was not renamed after this CL (and was renamed before this CL). BUG= 593446 Review-Url: https://codereview.chromium.org/2646803002 Cr-Commit-Position: refs/heads/master@{#444815} [modify] https://crrev.com/535b89a267d880c41efed60526213483ae79e707/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp |
||||
►
Sign in to add a comment |
||||
Comment 1 by danakj@chromium.org
, Mar 9 2016