Unified usage of vector<unique_ptr<T>>

Unified usage of vector<unique_ptr<T>>

Replace occurrences of vector of ptr and ScopedVector (deprecated) by vector<unique_ptr<T>>

BUG=554289
Committed: https://crrev.com/7d4e8eebaa558b2bdf045a11fd9bcc311be5e36d
Cr-Commit-Position: refs/heads/master@{#393040}

[etiennep, 2016-05-10 19:54:54]

Description was changed from

==========
Unified usage of vector<unique_ptr<T>>

Replace occurrences of vector of ptr and ScopedVector (deprecated) by
vector<unique_ptr<T>>

BUG=
==========

to

==========
Unified usage of vector<unique_ptr<T>>

Replace occurrences of vector of ptr and ScopedVector (deprecated) by
vector<unique_ptr<T>>

BUG=
==========

[etiennep, 2016-05-10 19:54:54]

etiennep@google.com changed reviewers:
+ huangs@chromium.org

[etiennep, 2016-05-10 19:56:35]

PTAL

[etiennep, 2016-05-10 19:56:58]

The CQ bit was checked by etiennep@google.com

[etiennep, 2016-05-10 19:56:58]

The CQ bit was unchecked by etiennep@google.com

[etiennep, 2016-05-10 19:58:52]

The CQ bit was checked by etiennep@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-10 19:59:24]

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

[commit-bot: I haz the power, 2016-05-10 20:25:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-10 20:25:22]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[huangs, 2016-05-11 04:49:24]

Looks good!  There's a compiler error.  You can find the cause bu clicking on
the failed try jobs (red rectangles) to see try details, scroll down to 
"compile (with patch) compile (with patch)", click "stdio" and search for
compiler error messages.

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/disassembl...
File courgette/disassembler_elf_32.cc (right):

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/disassembl...
courgette/disassembler_elf_32.cc:308: auto current_rel =
rel32_locations_.begin();
More consistent to keep at
std::vector<std::uniqe_ptr<TypedRVA>>::iterator
(even if it wraps to next line).

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/disassembl...
File courgette/disassembler_elf_32.h (right):

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/disassembl...
courgette/disassembler_elf_32.h:65: const std::unique_ptr<TypedRVA>& b) {
NIT: Align to "(" like
function_name(int param1,
              int param2) {

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/disassembl...
courgette/disassembler_elf_32.h:71: const std::unique_ptr<TypedRVA>& b) {
Same as above.

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/disassembl...
courgette/disassembler_elf_32.h:97: std::vector<RVA> &Abs32Locations() { return
abs32_locations_; }
Please group '&' with type (fixes earlier mistake), i.e., 

std::vector<RVA>& Abs32Locations()

same below.

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/disassembl...
File courgette/disassembler_elf_32_x86.cc (right):

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/disassembl...
courgette/disassembler_elf_32_x86.cc:8: #include <vector>
#include <utility>

for std::move().

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/ensemble_a...
File courgette/ensemble_apply.cc (right):

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/ensemble_a...
courgette/ensemble_apply.cc:11: #include <memory>
NIT: new line to separate C-style and C++-style.

Add

#include <utility>

if you adopt the std::move() suggestion below.

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/ensemble_a...
courgette/ensemble_apply.cc:136: TransformationPatcher* patcher = NULL;
std::unique_ptr<TransformationPatcher> patcher;

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/ensemble_a...
courgette/ensemble_apply.cc:138: switch (kind)
Unwrap.

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/ensemble_a...
courgette/ensemble_apply.cc:141: patcher = new PatcherX86_32(base_region_);
patcher.reset(...);
...

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/ensemble_a...
courgette/ensemble_apply.cc:152: }
    default:
      return C_BAD_ENSEMBLE_HEADER;

https://chromiumcodereview.appspot.com/1969543002/diff/1/courgette/ensemble_a...
courgette/ensemble_apply.cc:154: if (patcher)
Can do:

  DCHECK(patcher);
  patchers_.push_back(std::move(patcher));

remove the else.

[etiennep, 2016-05-11 18:17:50]

The CQ bit was checked by etiennep@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-11 18:18:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1969543002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1969543002/60001

[etiennep, 2016-05-11 18:19:16]

PTAnL

https://codereview.chromium.org/1969543002/diff/1/courgette/disassembler_elf_...
File courgette/disassembler_elf_32.cc (right):

https://codereview.chromium.org/1969543002/diff/1/courgette/disassembler_elf_...
courgette/disassembler_elf_32.cc:308: auto current_rel =
rel32_locations_.begin();
On 2016/05/11 04:49:23, huangs wrote:
> More consistent to keep at
> std::vector<std::uniqe_ptr<TypedRVA>>::iterator
> (even if it wraps to next line).

Done.

https://codereview.chromium.org/1969543002/diff/1/courgette/disassembler_elf_...
File courgette/disassembler_elf_32.h (right):

https://codereview.chromium.org/1969543002/diff/1/courgette/disassembler_elf_...
courgette/disassembler_elf_32.h:65: const std::unique_ptr<TypedRVA>& b) {
On 2016/05/11 04:49:23, huangs wrote:
> NIT: Align to "(" like
> function_name(int param1,
>               int param2) {

Done.

https://codereview.chromium.org/1969543002/diff/1/courgette/disassembler_elf_...
courgette/disassembler_elf_32.h:71: const std::unique_ptr<TypedRVA>& b) {
On 2016/05/11 04:49:23, huangs wrote:
> Same as above.

Done.

https://codereview.chromium.org/1969543002/diff/1/courgette/disassembler_elf_...
courgette/disassembler_elf_32.h:97: std::vector<RVA> &Abs32Locations() { return
abs32_locations_; }
On 2016/05/11 04:49:23, huangs wrote:
> Please group '&' with type (fixes earlier mistake), i.e., 
> 
> std::vector<RVA>& Abs32Locations()
> 
> same below.

Done.

https://codereview.chromium.org/1969543002/diff/1/courgette/disassembler_elf_...
File courgette/disassembler_elf_32_x86.cc (right):

https://codereview.chromium.org/1969543002/diff/1/courgette/disassembler_elf_...
courgette/disassembler_elf_32_x86.cc:8: #include <vector>
On 2016/05/11 04:49:23, huangs wrote:
> #include <utility>
> 
> for std::move().

Done.

https://codereview.chromium.org/1969543002/diff/1/courgette/ensemble_apply.cc
File courgette/ensemble_apply.cc (right):

https://codereview.chromium.org/1969543002/diff/1/courgette/ensemble_apply.cc...
courgette/ensemble_apply.cc:11: #include <memory>
On 2016/05/11 04:49:23, huangs wrote:
> NIT: new line to separate C-style and C++-style.
> 
> Add
> 
> #include <utility>
> 
> if you adopt the std::move() suggestion below.

Done.

https://codereview.chromium.org/1969543002/diff/1/courgette/ensemble_apply.cc...
courgette/ensemble_apply.cc:136: TransformationPatcher* patcher = NULL;
On 2016/05/11 04:49:24, huangs wrote:
> std::unique_ptr<TransformationPatcher> patcher;

Done.

https://codereview.chromium.org/1969543002/diff/1/courgette/ensemble_apply.cc...
courgette/ensemble_apply.cc:138: switch (kind)
On 2016/05/11 04:49:24, huangs wrote:
> Unwrap.

Done.

https://codereview.chromium.org/1969543002/diff/1/courgette/ensemble_apply.cc...
courgette/ensemble_apply.cc:141: patcher = new PatcherX86_32(base_region_);
On 2016/05/11 04:49:23, huangs wrote:
> patcher.reset(...);
> ...

Done.

https://codereview.chromium.org/1969543002/diff/1/courgette/ensemble_apply.cc...
courgette/ensemble_apply.cc:152: }
On 2016/05/11 04:49:23, huangs wrote:
>     default:
>       return C_BAD_ENSEMBLE_HEADER;

Done.

https://codereview.chromium.org/1969543002/diff/1/courgette/ensemble_apply.cc...
courgette/ensemble_apply.cc:154: if (patcher)
On 2016/05/11 04:49:23, huangs wrote:
> Can do:
> 
>   DCHECK(patcher);
>   patchers_.push_back(std::move(patcher));
> 
> remove the else.

Done.

[Avi (use Gerrit), 2016-05-11 18:32:33]

drive by

This is bug 554289 ; please tag it as such.

[etiennep, 2016-05-11 18:46:34]

Description was changed from

==========
Unified usage of vector<unique_ptr<T>>

Replace occurrences of vector of ptr and ScopedVector (deprecated) by
vector<unique_ptr<T>>

BUG=
==========

to

==========
Unified usage of vector<unique_ptr<T>>

Replace occurrences of vector of ptr and ScopedVector (deprecated) by
vector<unique_ptr<T>>

BUG=554289
==========

[etiennep, 2016-05-11 18:47:21]

On 2016/05/11 18:32:33, Avi wrote:
> drive by
> 
> This is bug 554289 ; please tag it as such.

Thanks for the tip!

[huangs, 2016-05-11 18:51:21]

Thanks! LGTM.

[etiennep, 2016-05-11 18:52:32]

The CQ bit was checked by etiennep@google.com

[commit-bot: I haz the power, 2016-05-11 18:53:01]

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

[commit-bot: I haz the power, 2016-05-11 20:14:58]

Description was changed from

==========
Unified usage of vector<unique_ptr<T>>

Replace occurrences of vector of ptr and ScopedVector (deprecated) by
vector<unique_ptr<T>>

BUG=554289
==========

to

==========
Unified usage of vector<unique_ptr<T>>

Replace occurrences of vector of ptr and ScopedVector (deprecated) by
vector<unique_ptr<T>>

BUG=554289
==========

[commit-bot: I haz the power, 2016-05-11 20:14:59]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-11 20:16:38]

Description was changed from

==========
Unified usage of vector<unique_ptr<T>>

Replace occurrences of vector of ptr and ScopedVector (deprecated) by
vector<unique_ptr<T>>

BUG=554289
==========

to

==========
Unified usage of vector<unique_ptr<T>>

Replace occurrences of vector of ptr and ScopedVector (deprecated) by
vector<unique_ptr<T>>

BUG=554289

Committed: https://crrev.com/7d4e8eebaa558b2bdf045a11fd9bcc311be5e36d
Cr-Commit-Position: refs/heads/master@{#393040}
==========

[commit-bot: I haz the power, 2016-05-11 20:16:39]

Patchset 5 (id:??) landed as
https://crrev.com/7d4e8eebaa558b2bdf045a11fd9bcc311be5e36d
Cr-Commit-Position: refs/heads/master@{#393040}