Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(12)

Issue 1727063002: gin: Add out-of-line copy ctors for complex classes. (Closed)

Created:
5 years ago by vmpstr
Modified:
5 years ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

gin: Add out-of-line copy ctors for complex classes. This patch adds out of line copy constructors for classes that our clang-plugin considers heavy. This is an effort to enable copy constructor checks by default. BUG=436357 R=aa@chromium.org, dcheng@chromium.org, thakis@chromium.org Committed: https://crrev.com/8ed6f86e3e08945205747a74de58972d57851968 Cr-Commit-Position: refs/heads/master@{#377655}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -0 lines) Patch
M gin/dictionary.h View 1 chunk +1 line, -0 lines 0 comments Download
M gin/dictionary.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M gin/object_template_builder.h View 1 chunk +1 line, -0 lines 0 comments Download
M gin/object_template_builder.cc View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (3 generated)
vmpstr
Please take a look.
5 years ago (2016-02-24 00:28:44 UTC) #1
dcheng
lgtm (this directory needs update OWNERS, aa@ and abarth@ both don't work on chromium anymore)
5 years ago (2016-02-24 01:23:49 UTC) #2
Nico
lgtm2 jochen, can you a) owner-stamp b) update gin/OWNERS?
5 years ago (2016-02-24 01:30:50 UTC) #4
jochen (gone - plz use gerrit)
again, shouldn't those classes now also get operator=?
5 years ago (2016-02-24 08:22:54 UTC) #5
vmpstr
On 2016/02/24 08:22:54, jochen wrote: > again, shouldn't those classes now also get operator=? That's ...
5 years ago (2016-02-25 00:03:44 UTC) #6
vmpstr
On 2016/02/25 00:03:44, vmpstr wrote: > On 2016/02/24 08:22:54, jochen wrote: > > again, shouldn't ...
5 years ago (2016-02-25 00:05:53 UTC) #7
jochen (gone - plz use gerrit)
lgtm
5 years ago (2016-02-25 11:56:34 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1727063002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1727063002/1
5 years ago (2016-02-25 18:57:08 UTC) #10
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years ago (2016-02-25 20:25:27 UTC) #11
commit-bot: I haz the power
5 years ago (2016-02-25 20:26:34 UTC) #13
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/8ed6f86e3e08945205747a74de58972d57851968
Cr-Commit-Position: refs/heads/master@{#377655}

Powered by Google App Engine
This is Rietveld 408576698