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

Issue 878273002: Revert of Add explicit destructors to DOM classes containing smart pointers to incomplete types (Closed)

Created:
5 years, 11 months ago by Nico
Modified:
5 years, 11 months ago
Reviewers:
Reid Kleckner
CC:
blink-reviews, ed+blinkwatch_opera.com, krit, blink-reviews-html_chromium.org, fs, Raymond Toy, dglazkov+blink, f(malita), gyuyoung.kim_webkit.org, Stephen Chennney, kouhei+svg_chromium.org, pdr., rwlbuis
Project:
blink
Visibility:
Public.

Description

Revert of Add explicit destructors to DOM classes containing smart pointers to incomplete types (patchset #2 id:20001 of https://codereview.chromium.org/412483007/) Reason for revert: This should no longer be necessary; clang should be able to handle this now. Reverting this to test that theory. Original issue's description: > Add explicit destructors to DOM classes containing smart pointers to incomplete types > > Clang on Windows tries to declare the implicit virtual destructor of > these classes so that it can diagnose mismatched exceptions > specifications. This ends up requiring that the type of the smart > pointer be complete, which it is not. This codepath does not fire on > non-Windows platforms due to the Itanium C++ ABI key function > optimization. > > It is possible to fix Clang to avoid performing semantic analysis of the > destructor when it only needs to compute the exception specification, > but it is prohibitively difficult. http://llvm.org/PR20337 > > Another possible fix on the Blink side would be to make the smart > pointer type complete by including the relevant header, which has the > cost of increasing transitive header bloat. Instead, simply define the > destructor out of line. This is also a minor object file size > optimization, as now the destructor is only emitted once. > > R=thakis@chromium.org > BUG=82385 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178838 TBR=rnk@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=82385 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189052

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -15 lines) Patch
M Source/core/html/HTMLOptionElement.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/html/HTMLOptionElement.cpp View 1 chunk +0 lines, -4 lines 0 comments Download
M Source/core/svg/SVGScriptElement.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/svg/SVGScriptElement.cpp View 1 chunk +0 lines, -4 lines 0 comments Download
M Source/platform/audio/AudioDSPKernelProcessor.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/platform/audio/AudioDSPKernelProcessor.cpp View 1 chunk +0 lines, -4 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Nico
Created Revert of Add explicit destructors to DOM classes containing smart pointers to incomplete types
5 years, 11 months ago (2015-01-27 19:42:49 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/878273002/1
5 years, 11 months ago (2015-01-27 19:43:30 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://src.chromium.org/viewvc/blink?view=rev&revision=189052
5 years, 11 months ago (2015-01-27 19:44:28 UTC) #3
Reid Kleckner
5 years, 11 months ago (2015-01-27 21:19:09 UTC) #4
Message was sent while issue was closed.
lgtm

Powered by Google App Engine
This is Rietveld 408576698