win: Work on reducing number of style plugin warnings.

win: Work on reducing number of style plugin warnings.

BUG=467287
Committed: https://crrev.com/9d2ba75cedbc180aa019b25c05ff39c5df6ca04b
Cr-Commit-Position: refs/heads/master@{#327867}

[Nico, 2015-05-01 00:24:12]

thakis@chromium.org changed reviewers:
+ dcheng@chromium.org

[Lei Zhang, 2015-05-01 01:34:38]

thestig@chromium.org changed reviewers:
+ thestig@chromium.org

[Lei Zhang, 2015-05-01 01:34:39]

chrome/ lgtm

[Nico, 2015-05-01 02:17:24]

Thanks. Wanna look at ui/ too? (I'm owner everywhere, just need a regular
old lg)
On Apr 30, 2015 6:34 PM, <thestig@chromium.org> wrote:

> chrome/ lgtm
>
> https://codereview.chromium.org/1121633002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dcheng, 2015-05-01 02:23:23]

lgtm

[Nico, 2015-05-01 02:38:20]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2015-05-01 02:38:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1121633002/20001

[Peter Kasting, 2015-05-01 02:42:01]

pkasting@chromium.org changed reviewers:
+ pkasting@chromium.org

[Peter Kasting, 2015-05-01 02:42:01]

https://codereview.chromium.org/1121633002/diff/20001/chrome/common/importer/...
File chrome/common/importer/importer_data_types.h (right):

https://codereview.chromium.org/1121633002/diff/20001/chrome/common/importer/...
chrome/common/importer/importer_data_types.h:63: ~ImporterIE7PasswordInfo();
Most of these changes make sense, but this one doesn't.  While we should have an
explicit destructor if there's an explicit constructor, in this case there
wasn't an explicit either.  It doesn't seem reasonable to require explicit
(empty) constructors for data-only structs, even when they have non-POD members.

[Nico, 2015-05-01 02:46:38]

https://codereview.chromium.org/1121633002/diff/20001/chrome/common/importer/...
File chrome/common/importer/importer_data_types.h (right):

https://codereview.chromium.org/1121633002/diff/20001/chrome/common/importer/...
chrome/common/importer/importer_data_types.h:63: ~ImporterIE7PasswordInfo();
On 2015/05/01 02:42:01, Peter Kasting wrote:
> Most of these changes make sense, but this one doesn't.  While we should have
an
> explicit destructor if there's an explicit constructor, in this case there
> wasn't an explicit either.  It doesn't seem reasonable to require explicit
> (empty) constructors for data-only structs, even when they have non-POD
members.

There are three classes with constructors in this class. The implicit
constructor and destructor have nontrivial code size, so they're going out of
line. (Note that this has been enforced on non-win for years, this is just doing
the same thing on win)

[commit-bot: I haz the power, 2015-05-01 02:49:24]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-01 02:51:38]

Patchset 2 (id:??) landed as
https://crrev.com/9d2ba75cedbc180aa019b25c05ff39c5df6ca04b
Cr-Commit-Position: refs/heads/master@{#327867}

[Peter Kasting, 2015-05-01 03:48:32]

https://codereview.chromium.org/1121633002/diff/20001/chrome/common/importer/...
File chrome/common/importer/importer_data_types.h (right):

https://codereview.chromium.org/1121633002/diff/20001/chrome/common/importer/...
chrome/common/importer/importer_data_types.h:63: ~ImporterIE7PasswordInfo();
On 2015/05/01 02:46:37, Nico wrote:
> On 2015/05/01 02:42:01, Peter Kasting wrote:
> > Most of these changes make sense, but this one doesn't.  While we should
have
> an
> > explicit destructor if there's an explicit constructor, in this case there
> > wasn't an explicit either.  It doesn't seem reasonable to require explicit
> > (empty) constructors for data-only structs, even when they have non-POD
> members.
> 
> There are three classes with constructors in this class. The implicit
> constructor and destructor have nontrivial code size, so they're going out of
> line. (Note that this has been enforced on non-win for years, this is just
doing
> the same thing on win)

Is there a way to make that border between trivial and nontrivial code size
clear to the reader?  I'm aware of that general rule, but it wasn't clear to me
(and still wouldn't be if I came across this code) that that's the reason for
the out-of-line definition here.