Removed usage of std::map and std::vector from kernel code. Issue #28064.

Removed usage of std::map and std::vector from kernel code. Issue #28064.

Added fix to hopefully fix build error.

BUG=
R=zra@google.com

Review-Url: https://codereview.chromium.org/2647633003 .
Committed: https://github.com/dart-lang/sdk/commit/b9035d949f72edf329c2590f7becca0ec250bc73

[bkonyi, 2017-01-19 20:48:46]

bkonyi@google.com changed reviewers:
+ zra@google.com

[bkonyi, 2017-01-19 20:48:46]

https://codereview.chromium.org/2641243002/diff/1/runtime/vm/growable_array.h
File runtime/vm/growable_array.h (right):

https://codereview.chromium.org/2641243002/diff/1/runtime/vm/growable_array.h...
runtime/vm/growable_array.h:42: ASSERT(static_cast<uintptr_t>(length_) >=
static_cast<uintptr_t>(length));
The compiler was complaining that doing length_ >= length could lead to bad
results in the case of overflows. Checking if length is positive before casting
to uintptr_t should be sufficient.

[zra, 2017-01-19 21:06:25]

https://codereview.chromium.org/2641243002/diff/1/runtime/vm/growable_array.h
File runtime/vm/growable_array.h (right):

https://codereview.chromium.org/2641243002/diff/1/runtime/vm/growable_array.h...
runtime/vm/growable_array.h:42: ASSERT(static_cast<uintptr_t>(length_) >=
static_cast<uintptr_t>(length));
On 2017/01/19 20:48:46, bkonyi wrote:
> The compiler was complaining that doing length_ >= length could lead to bad
> results in the case of overflows. Checking if length is positive before
casting
> to uintptr_t should be sufficient.

I think it's not worried about overflow here, but rather at the call-sites of
TruncateTo in kernel_binary.cc. I think we might need to change instances of int
-> intptr_t in that file.

[bkonyi, 2017-01-20 23:56:28]

https://codereview.chromium.org/2641243002/diff/1/runtime/vm/growable_array.h
File runtime/vm/growable_array.h (right):

https://codereview.chromium.org/2641243002/diff/1/runtime/vm/growable_array.h...
runtime/vm/growable_array.h:42: ASSERT(static_cast<uintptr_t>(length_) >=
static_cast<uintptr_t>(length));
On 2017/01/19 at 21:06:25, zra wrote:
> On 2017/01/19 20:48:46, bkonyi wrote:
> > The compiler was complaining that doing length_ >= length could lead to bad
> > results in the case of overflows. Checking if length is positive before
casting
> > to uintptr_t should be sufficient.
> 
> I think it's not worried about overflow here, but rather at the call-sites of
TruncateTo in kernel_binary.cc. I think we might need to change instances of int
-> intptr_t in that file.

See comment in new patch in kernel_binary.

https://codereview.chromium.org/2641243002/diff/20001/runtime/vm/kernel_binar...
File runtime/vm/kernel_binary.cc (left):

https://codereview.chromium.org/2641243002/diff/20001/runtime/vm/kernel_binar...
runtime/vm/kernel_binary.cc:184: variables_.resize(variables_.size() - 1);
Just changing this line to use RemoveLast seems to fix the compile error.

[zra, 2017-01-21 00:01:46]

I'm still seeing errors like:
../../runtime/vm/kernel_binary.cc:166:41: error: comparison of integers of
different signs: 'unsigned int' and 'intptr_t' (aka 'long')
[-Werror,-Wsign-compare]
    ASSERT(static_cast<unsigned>(index) < variables_.length());

[zra, 2017-01-21 00:02:02]

On 2017/01/21 00:01:46, zra wrote:
> I'm still seeing errors like:
> ../../runtime/vm/kernel_binary.cc:166:41: error: comparison of integers of
> different signs: 'unsigned int' and 'intptr_t' (aka 'long')
> [-Werror,-Wsign-compare]
>     ASSERT(static_cast<unsigned>(index) < variables_.length());

On Mac on DebugIA32

[zra, 2017-01-21 00:03:44]

https://codereview.chromium.org/2641243002/diff/20001/runtime/vm/kernel_binar...
File runtime/vm/kernel_binary.cc (right):

https://codereview.chromium.org/2641243002/diff/20001/runtime/vm/kernel_binar...
runtime/vm/kernel_binary.cc:166: ASSERT(static_cast<unsigned>(index) <
variables_.length());
This cast looks like it is no longer needed. Maybe there are others as well.

[zra, 2017-01-21 07:25:35]

https://codereview.chromium.org/2641243002/diff/20001/runtime/vm/kernel_binar...
File runtime/vm/kernel_binary.cc (right):

https://codereview.chromium.org/2641243002/diff/20001/runtime/vm/kernel_binar...
runtime/vm/kernel_binary.cc:166: ASSERT(static_cast<unsigned>(index) <
variables_.length());
On 2017/01/21 00:03:44, zra wrote:
> This cast looks like it is no longer needed. Maybe there are others as well.

Removing the cast fixes the Mac DebugIA32 build for me.

[bkonyi, 2017-01-24 20:48:27]

https://codereview.chromium.org/2641243002/diff/20001/runtime/vm/kernel_binar...
File runtime/vm/kernel_binary.cc (right):

https://codereview.chromium.org/2641243002/diff/20001/runtime/vm/kernel_binar...
runtime/vm/kernel_binary.cc:166: ASSERT(static_cast<unsigned>(index) <
variables_.length());
On 2017/01/21 07:25:35, zra wrote:
> On 2017/01/21 00:03:44, zra wrote:
> > This cast looks like it is no longer needed. Maybe there are others as well.
> 
> Removing the cast fixes the Mac DebugIA32 build for me.

Yes, it does. Done.

[zra, 2017-01-24 21:09:40]

lgtm

[bkonyi, 2017-01-24 22:26:03]

Description was changed from

==========
Removed usage of std::map and std::vector from kernel code. Issue #28064.

Added fix to hopefully fix build error.

BUG=

Review-Url: https://codereview.chromium.org/2647633003 .
==========

to

==========
Removed usage of std::map and std::vector from kernel code. Issue #28064.

Added fix to hopefully fix build error.

BUG=
R=zra@google.com

Review-Url: https://codereview.chromium.org/2647633003 .
Committed:
https://github.com/dart-lang/sdk/commit/b9035d949f72edf329c2590f7becca0ec250bc73
==========

[bkonyi, 2017-01-24 22:26:04]

Committed patchset #4 (id:60001) manually as
b9035d949f72edf329c2590f7becca0ec250bc73 (presubmit successful).