Use std::tie() for operator< in components/

Use std::tie() for operator< in components/

Simplify the code for operator< when comparing multiple members using
a common std::tie idiom.

BUG=555171
R=thakis@chromium.org,vabr@chromium.org,jochen@chromium.org

Committed: https://crrev.com/4afb256b17b1c9a716a031205f48f4648768c308
Cr-Commit-Position: refs/heads/master@{#360367}

[jsbell, 2015-11-16 21:14:56]

Description was changed from

==========
Use std::tie() for operator< in components/

Simplify the code for operator< when comparing multiple members using
a common std::tie idiom.

BUG=555171
R=thakis@chromium.org
==========

to

==========
Use std::tie() for operator< in components/

Simplify the code for operator< when comparing multiple members using
a common std::tie idiom.

BUG=555171
R=thakis@chromium.org
TBR=jochen@chromium.org
==========

[jsbell, 2015-11-16 21:14:56]

jsbell@chromium.org changed reviewers:
+ jochen@chromium.org

[jsbell, 2015-11-17 23:10:00]

thakis@ - please take a look?

https://codereview.chromium.org/1447153002/diff/1/components/content_settings...
File
components/content_settings/core/browser/content_settings_origin_identifier_value_map.cc
(right):

https://codereview.chromium.org/1447153002/diff/1/components/content_settings...
components/content_settings/core/browser/content_settings_origin_identifier_value_map.cc:82:
return std::tie(primary_pattern, secondary_pattern) >
Note the comment - this one is intentionally inverted.

[Nico, 2015-11-18 01:36:38]

lgtm

[Nico, 2015-11-18 01:36:49]

(not an owner here though)

[vabr (Chromium), 2015-11-18 09:10:00]

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

[vabr (Chromium), 2015-11-18 09:10:01]

components/autofill/ LGTM!

[jochen (gone - plz use gerrit), 2015-11-18 16:12:59]

lgtm

[jsbell, 2015-11-18 17:10:32]

Description was changed from

==========
Use std::tie() for operator< in components/

Simplify the code for operator< when comparing multiple members using
a common std::tie idiom.

BUG=555171
R=thakis@chromium.org
TBR=jochen@chromium.org
==========

to

==========
Use std::tie() for operator< in components/

Simplify the code for operator< when comparing multiple members using
a common std::tie idiom.

BUG=555171
R=thakis@chromium.org,vabr@chromium.org,jochen@chromium.org
==========

[jsbell, 2015-11-18 17:10:47]

The CQ bit was checked by jsbell@chromium.org

[commit-bot: I haz the power, 2015-11-18 17:12:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1447153002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1447153002/1

[commit-bot: I haz the power, 2015-11-18 18:31:08]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-11-18 18:32:09]

Patchset 1 (id:??) landed as
https://crrev.com/4afb256b17b1c9a716a031205f48f4648768c308
Cr-Commit-Position: refs/heads/master@{#360367}

[jsbell, 2015-11-20 01:23:54]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1456263005/ by jsbell@chromium.org.

The reason for reverting is: Perf regression? Shouldn't be, but let's check.
https://crbug.com/558616.

[Alexei Svitkine (slow), 2015-11-26 21:18:18]

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

[Alexei Svitkine (slow), 2015-11-26 21:18:18]

FYI: I think this now causing this (non-fatal) warning on Android build:

../../components/autofill/core/common/form_field_data.cc: In member function
'bool autofill::FormFieldData::operator<(const autofill::FormFieldData&) const':
../../components/autofill/core/common/form_field_data.cc:120:6: note: variable
tracking size limit exceeded with -fvar-tracking-assignments, retrying without
 bool FormFieldData::operator<(const FormFieldData& field) const {
      ^

[jsbell, 2015-11-30 17:44:10]

On 2015/11/26 21:18:18, Alexei Svitkine (slow) wrote:
> FYI: I think this now causing this (non-fatal) warning on Android build:
> 
> ../../components/autofill/core/common/form_field_data.cc: In member function
> 'bool autofill::FormFieldData::operator<(const autofill::FormFieldData&)
const':
> ../../components/autofill/core/common/form_field_data.cc:120:6: note: variable
> tracking size limit exceeded with -fvar-tracking-assignments, retrying without
>  bool FormFieldData::operator<(const FormFieldData& field) const {
>       ^

If that's distracting and/or we can't (or shouldn't) suppress we can revert that
part of the CL. It's purely for readability.

[Alexei Svitkine (slow), 2015-11-30 17:58:10]

I think the goal is to keep builds warning free, so can you see if the
warning can be suppressed?

Also, thakis is cc'd to this discussion, so he might have more insight /
suggestions for best practices w.r.t to this sort of thing since he's been
working on compiler stuff.

On Mon, Nov 30, 2015 at 12:44 PM, <jsbell@chromium.org> wrote:

> On 2015/11/26 21:18:18, Alexei Svitkine (slow) wrote:
>
>> FYI: I think this now causing this (non-fatal) warning on Android build:
>>
>
> ../../components/autofill/core/common/form_field_data.cc: In member
>> function
>> 'bool autofill::FormFieldData::operator<(const autofill::FormFieldData&)
>>
> const':
>
>> ../../components/autofill/core/common/form_field_data.cc:120:6: note:
>> variable
>> tracking size limit exceeded with -fvar-tracking-assignments, retrying
>> without
>>   bool FormFieldData::operator<(const FormFieldData& field) const {
>>        ^
>>
>
> If that's distracting and/or we can't (or shouldn't) suppress we can
> revert that
> part of the CL. It's purely for readability.
>
> https://codereview.chromium.org/1447153002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2015-11-30 17:59:14]

I'd check what we use -fvar-tracking-assignments for, and if that note can
be suppressed. If not, I'd revert this to the old form for this file for
now.

On Mon, Nov 30, 2015 at 12:57 PM, Alexei Svitkine <asvitkine@chromium.org>
wrote:

> I think the goal is to keep builds warning free, so can you see if the
> warning can be suppressed?
>
> Also, thakis is cc'd to this discussion, so he might have more insight /
> suggestions for best practices w.r.t to this sort of thing since he's been
> working on compiler stuff.
>
> On Mon, Nov 30, 2015 at 12:44 PM, <jsbell@chromium.org> wrote:
>
>> On 2015/11/26 21:18:18, Alexei Svitkine (slow) wrote:
>>
>>> FYI: I think this now causing this (non-fatal) warning on Android build:
>>>
>>
>> ../../components/autofill/core/common/form_field_data.cc: In member
>>> function
>>> 'bool autofill::FormFieldData::operator<(const autofill::FormFieldData&)
>>>
>> const':
>>
>>> ../../components/autofill/core/common/form_field_data.cc:120:6: note:
>>> variable
>>> tracking size limit exceeded with -fvar-tracking-assignments, retrying
>>> without
>>>   bool FormFieldData::operator<(const FormFieldData& field) const {
>>>        ^
>>>
>>
>> If that's distracting and/or we can't (or shouldn't) suppress we can
>> revert that
>> part of the CL. It's purely for readability.
>>
>> https://codereview.chromium.org/1447153002/
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jsbell, 2015-12-01 20:13:04]

I put a revert up at https://codereview.chromium.org/1486173003

It looks like we can't suppress, and if we did it would still recompile the
file. We could bump max-vartrack-size but that doesn't seem worthwhile, and
could slow down compile elsewhere.