Eliminate Visual Studio compiler warnings

Eliminate Visual Studio compiler warnings

[ammo6818-vandals.uidaho.edu, 2018-02-18 05:44:41]

Please let me know if there are any other review comments or merge into ns-3-dev

[Tom Henderson, 2018-02-20 02:19:28]

Once

https://codereview.appspot.com/336970043/diff/220001/src/core/examples/sample...
File src/core/examples/sample-log-time-format.cc (right):

https://codereview.appspot.com/336970043/diff/220001/src/core/examples/sample...
src/core/examples/sample-log-time-format.cc:127: #if 0
The point of this example is to display the log output.  If we need to suppress
it, add a command line argument 'quiet' (default boolean false) and if set to
true, suppress the log component enabling.

https://codereview.appspot.com/336970043/diff/220001/src/core/model/log.cc
File src/core/model/log.cc (right):

https://codereview.appspot.com/336970043/diff/220001/src/core/model/log.cc#ne...
src/core/model/log.cc:689: m_os << static_cast<int16_t>(param);
I would echo Peter's question here:  why is the ", " missing (here and below)?

[ammo6818-vandals.uidaho.edu, 2018-02-20 04:40:39]

Update posted to address latest review comments

https://codereview.appspot.com/336970043/diff/220001/src/core/examples/sample...
File src/core/examples/sample-log-time-format.cc (right):

https://codereview.appspot.com/336970043/diff/220001/src/core/examples/sample...
src/core/examples/sample-log-time-format.cc:127: #if 0
On 2018/02/20 02:19:28, Tom Henderson wrote:
> The point of this example is to display the log output.  If we need to
suppress
> it, add a command line argument 'quiet' (default boolean false) and if set to
> true, suppress the log component enabling.

restored original implementation

https://codereview.appspot.com/336970043/diff/220001/src/core/model/log.cc
File src/core/model/log.cc (right):

https://codereview.appspot.com/336970043/diff/220001/src/core/model/log.cc#ne...
src/core/model/log.cc:689: m_os << static_cast<int16_t>(param);
On 2018/02/20 02:19:28, Tom Henderson wrote:
> I would echo Peter's question here:  why is the ", " missing (here and below)?

I matched the implementation found in ParameterLogger::operator<<
<std::string>(const std::string param) which doesnt output a comma if this is
the first output.

[Tom Henderson, 2018-02-20 05:14:23]

> 
> I matched the implementation found in ParameterLogger::operator<<
> <std::string>(const std::string param) which doesnt output a comma if this is
> the first output.

I see now, thanks.

[Peter Barnes, 2018-02-21 23:16:33]

On 2018/02/18 05:44:41, ammo6818-vandals.uidaho.edu wrote:
> Please let me know if there are any other review comments or merge into
ns-3-dev

My and Tom's suggested cleanups aren't present in patch set 10 (doxygen, test
case logging, various others).  Please post an accurate patch set which passes
your VS warnings.

Thanks,
Peter

[ammo6818-vandals.uidaho.edu, 2018-02-25 12:12:16]

On 2018/02/21 23:16:33, Peter Barnes wrote:
> On 2018/02/18 05:44:41, http://ammo6818-vandals.uidaho.edu wrote:
> > Please let me know if there are any other review comments or merge into
> ns-3-dev
> 
> My and Tom's suggested cleanups aren't present in patch set 10 (doxygen, test
> case logging, various others).  Please post an accurate patch set which passes
> your VS warnings.
> 
> Thanks,
> Peter

My apologizes, the wrong files were uploaded.  A corrected patch has been
uploaded.

[Tom Henderson, 2018-02-25 18:14:57]

Just some minor things; would like to include this by Tuesday (as two patches--
doxygen and compiler issues)

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/comman...
File src/core/examples/command-line-example.cc (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/comman...
src/core/examples/command-line-example.cc:86: CommandLine cmd;
can be stripped from final patch (whitespace change)

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/hash-e...
File src/core/examples/hash-example.cc (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/hash-e...
src/core/examples/hash-example.cc:362: long double k64 = static_cast<long
double> (0xFFFFFFFFFFFFFFFFULL);
I am not sure about these; isn't long double an 8-byte quantity?  why is a cast
needed on k64 and not k32?  

Shouldn't it be something like std::numeric_limits<long double>::max()?

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/main-t...
File src/core/examples/main-test-sync.cc (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/main-t...
src/core/examples/main-test-sync.cc:45: NS_LOG_COMPONENT_DEFINE ("TestSync");
can be stripped from final patch (whitespace change)

https://codereview.appspot.com/336970043/diff/260001/src/core/model/assert.h
File src/core/model/assert.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/assert.h#...
src/core/model/assert.h:75: }                                                   
   \
is this just a whitespace change?

https://codereview.appspot.com/336970043/diff/260001/src/core/model/object-ba...
File src/core/model/object-base.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/object-ba...
src/core/model/object-base.h:82: namespace ns3 {
whitespace change

https://codereview.appspot.com/336970043/diff/260001/src/core/model/object.h
File src/core/model/object.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/object.h#...
src/core/model/object.h:668: #endif /* OBJECT_H */
whitespace change

https://codereview.appspot.com/336970043/diff/260001/src/core/model/vector.h
File src/core/model/vector.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/vector.h#...
src/core/model/vector.h:252: std::istream &operator >> (std::istream &is,
Vector2D &vector);
this perhaps can be pulled out of this patch and committed separately as doxygen
fix.

https://codereview.appspot.com/336970043/diff/260001/src/core/model/vector.h#...
src/core/model/vector.h:300: Ptr<const AttributeChecker> MakeVectorChecker
(void);
whitespace

https://codereview.appspot.com/336970043/diff/260001/src/core/model/wall-cloc...
File src/core/model/wall-clock-synchronizer.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/wall-cloc...
src/core/model/wall-clock-synchronizer.h:209: virtual int64_t DoGetDrift
(uint64_t ns);
similarly, this is a Doxygen update (can be separated)

[ammo6818-vandals.uidaho.edu, 2018-02-26 13:03:20]

Added explanations to the review comments.

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/hash-e...
File src/core/examples/hash-example.cc (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/hash-e...
src/core/examples/hash-example.cc:362: long double k64 = static_cast<long
double> (0xFFFFFFFFFFFFFFFFULL);
On 2018/02/25 18:14:57, Tom Henderson wrote:
> I am not sure about these; isn't long double an 8-byte quantity?  why is a
cast
> needed on k64 and not k32?  
> 
> Shouldn't it be something like std::numeric_limits<long double>::max()?

ULL is unsigned long long which different precision than long double hence the
required downcast.

https://codereview.appspot.com/336970043/diff/260001/src/core/model/assert.h
File src/core/model/assert.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/assert.h#...
src/core/model/assert.h:75: }                                                   
   \
On 2018/02/25 18:14:57, Tom Henderson wrote:
> is this just a whitespace change?

alignment of continuation character

https://codereview.appspot.com/336970043/diff/260001/src/core/model/vector.h
File src/core/model/vector.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/vector.h#...
src/core/model/vector.h:252: std::istream &operator >> (std::istream &is,
Vector2D &vector);
On 2018/02/25 18:14:57, Tom Henderson wrote:
> this perhaps can be pulled out of this patch and committed separately as
doxygen
> fix.

this was provided by Peter in a previous comment.

https://codereview.appspot.com/336970043/diff/260001/src/core/model/wall-cloc...
File src/core/model/wall-clock-synchronizer.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/wall-cloc...
src/core/model/wall-clock-synchronizer.h:209: virtual int64_t DoGetDrift
(uint64_t ns);
On 2018/02/25 18:14:57, Tom Henderson wrote:
> similarly, this is a Doxygen update (can be separated)

this was provided by Peter in a previous comment

[Peter Barnes, 2018-02-26 18:27:02]

I thought Robert revised patch 10, but it appears to be the same as last week,
still containing doxy and whitespace changes I've been trying to remove.  My
most up to date suggestion is in the bug report, patch 3039, to apply on top of
patch 8 here.  As far as I can tell this still hasn't appeared here for review.

I won't be able to do anything more on this until Friday at the earliest.  I
don't think this should hold up ns-3.28 release.

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/hash-e...
File src/core/examples/hash-example.cc (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/hash-e...
src/core/examples/hash-example.cc:362: long double k64 = static_cast<long
double> (0xFFFFFFFFFFFFFFFFULL);
On 2018/02/25 18:14:57, Tom Henderson wrote:
> I am not sure about these; isn't long double an 8-byte quantity?  why is a
cast
> needed on k64 and not k32?  
> 
> Shouldn't it be something like std::numeric_limits<long double>::max()?

I too am puzzled why casting is needed here, and not previous line.
This is the one patch I'm most unclear about.  I need some time to stare at the
code and remember why I did this value originally.  I won't have time to do that
until Friday.

https://codereview.appspot.com/336970043/diff/260001/src/core/model/assert.h
File src/core/model/assert.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/assert.h#...
src/core/model/assert.h:75: }                                                   
   \
On 2018/02/25 18:14:57, Tom Henderson wrote:
> is this just a whitespace change?

Yes both diffs are just whitespace to align '\'.  Could be discarded from final
patch.

https://codereview.appspot.com/336970043/diff/260001/src/core/model/vector.h
File src/core/model/vector.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/vector.h#...
src/core/model/vector.h:252: std::istream &operator >> (std::istream &is,
Vector2D &vector);
On 2018/02/26 13:03:20, ammo6818-vandals.uidaho.edu wrote:
> On 2018/02/25 18:14:57, Tom Henderson wrote:
> > this perhaps can be pulled out of this patch and committed separately as
> doxygen
> > fix.
> 
> this was provided by Peter in a previous comment.

Actually I've been trying to remove doxy from the VS patch.  This is a holdover
from Robert's very first patch.

https://codereview.appspot.com/336970043/diff/260001/src/core/model/wall-cloc...
File src/core/model/wall-clock-synchronizer.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/wall-cloc...
src/core/model/wall-clock-synchronizer.h:209: virtual int64_t DoGetDrift
(uint64_t ns);
On 2018/02/26 13:03:20, ammo6818-vandals.uidaho.edu wrote:
> On 2018/02/25 18:14:57, Tom Henderson wrote:
> > similarly, this is a Doxygen update (can be separated)
> 
> this was provided by Peter in a previous comment

Ditto:  part of Robert's original patch which I've been trying to remove.

[ammo6818-vandals.uidaho.edu, 2018-02-27 13:11:05]

Updates posted for latest comments

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/comman...
File src/core/examples/command-line-example.cc (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/comman...
src/core/examples/command-line-example.cc:86: CommandLine cmd;
On 2018/02/25 18:14:56, Tom Henderson wrote:
> can be stripped from final patch (whitespace change)

change reverted

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/hash-e...
File src/core/examples/hash-example.cc (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/hash-e...
src/core/examples/hash-example.cc:362: long double k64 = static_cast<long
double> (0xFFFFFFFFFFFFFFFFULL);
On 2018/02/26 18:27:03, Peter Barnes wrote:
> On 2018/02/25 18:14:57, Tom Henderson wrote:
> > I am not sure about these; isn't long double an 8-byte quantity?  why is a
> cast
> > needed on k64 and not k32?  
> > 
> > Shouldn't it be something like std::numeric_limits<long double>::max()?
> 
> I too am puzzled why casting is needed here, and not previous line.
> This is the one patch I'm most unclear about.  I need some time to stare at
the
> code and remember why I did this value originally.  I won't have time to do
that
> until Friday.

The previous line does not require casting because it is a long to a long double
which does not cause truncation of the numeric value.

The second line does because it is a long long to a long double which can cause
a truncation of the numeric value.

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/main-t...
File src/core/examples/main-test-sync.cc (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/examples/main-t...
src/core/examples/main-test-sync.cc:45: NS_LOG_COMPONENT_DEFINE ("TestSync");
On 2018/02/25 18:14:57, Tom Henderson wrote:
> can be stripped from final patch (whitespace change)

change reverted

https://codereview.appspot.com/336970043/diff/260001/src/core/model/object-ba...
File src/core/model/object-base.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/object-ba...
src/core/model/object-base.h:82: namespace ns3 {
On 2018/02/25 18:14:57, Tom Henderson wrote:
> whitespace change

change reverted

https://codereview.appspot.com/336970043/diff/260001/src/core/model/object.h
File src/core/model/object.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/object.h#...
src/core/model/object.h:668: #endif /* OBJECT_H */
On 2018/02/25 18:14:57, Tom Henderson wrote:
> whitespace change

change reverted

https://codereview.appspot.com/336970043/diff/260001/src/core/model/vector.h
File src/core/model/vector.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/vector.h#...
src/core/model/vector.h:252: std::istream &operator >> (std::istream &is,
Vector2D &vector);
On 2018/02/26 18:27:03, Peter Barnes wrote:
> On 2018/02/26 13:03:20, http://ammo6818-vandals.uidaho.edu wrote:
> > On 2018/02/25 18:14:57, Tom Henderson wrote:
> > > this perhaps can be pulled out of this patch and committed separately as
> > doxygen
> > > fix.
> > 
> > this was provided by Peter in a previous comment.
> 
> Actually I've been trying to remove doxy from the VS patch.  This is a
holdover
> from Robert's very first patch.

doxygen updates removed and submitted as a separate patch

https://codereview.appspot.com/336970043/diff/260001/src/core/model/vector.h#...
src/core/model/vector.h:300: Ptr<const AttributeChecker> MakeVectorChecker
(void);
On 2018/02/25 18:14:57, Tom Henderson wrote:
> whitespace

change reverted

https://codereview.appspot.com/336970043/diff/260001/src/core/model/wall-cloc...
File src/core/model/wall-clock-synchronizer.h (right):

https://codereview.appspot.com/336970043/diff/260001/src/core/model/wall-cloc...
src/core/model/wall-clock-synchronizer.h:209: virtual int64_t DoGetDrift
(uint64_t ns);
On 2018/02/26 18:27:03, Peter Barnes wrote:
> On 2018/02/26 13:03:20, http://ammo6818-vandals.uidaho.edu wrote:
> > On 2018/02/25 18:14:57, Tom Henderson wrote:
> > > similarly, this is a Doxygen update (can be separated)
> > 
> > this was provided by Peter in a previous comment
> 
> Ditto:  part of Robert's original patch which I've been trying to remove.

doxygen updates removed and submitted as separate patch

[ammo6818-vandals.uidaho.edu, 2018-03-02 23:22:44]

One of the things that I am struggling with on this issue is who is on first and
whats on second.  I have lost track of what I need to be doing to bring this to
completion.

A suggestion that I have is that you merge the files that are acceptable so that
what is left is only the files that still have comments that need to be
addressed which should make the patch set smaller and more manageable.

[ammo6818-vandals.uidaho.edu, 2018-03-22 14:47:01]

I have reverified the patch includes off the changes provided in the two
attachments and uploaded updates where I missed corrections from the
attachments.

[Peter Barnes, 2018-05-11 21:58:13]

https://codereview.appspot.com/336970043/diff/300001/src/core/model/attribute...
File src/core/model/attribute-accessor-helper.h (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/attribute...
src/core/model/attribute-accessor-helper.h:392: NS_UNUSED(v);
Previous two lines need space before paren.

https://codereview.appspot.com/336970043/diff/300001/src/core/model/hash-murm...
File src/core/model/hash-murmur3.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/hash-murm...
src/core/model/hash-murmur3.cc:525: m_hash32, (void *)& m_hash32);
Change size formal argument to size_t

https://codereview.appspot.com/336970043/diff/300001/src/core/model/hash-murm...
src/core/model/hash-murmur3.cc:526: m_size32 += static_cast<uint32_t> (size);
Change m_size32 type to size_t

https://codereview.appspot.com/336970043/diff/300001/src/core/model/hash-murm...
src/core/model/hash-murmur3.cc:556: MurmurHash3_x86_128_fin (static_cast<int>
(m_size64),
Change size formal argument to size_t

https://codereview.appspot.com/336970043/diff/300001/src/core/model/heap-sche...
File src/core/model/heap-scheduler.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/heap-sche...
src/core/model/heap-scheduler.cc:156: uint32_t index = static_cast<uint32_t>
(Last ());
This has already been fixed, differently! in ns-3-dev.
std::size_t index = Last ();

https://codereview.appspot.com/336970043/diff/300001/src/core/model/heap-sche...
src/core/model/heap-scheduler.cc:220: Exch (Root (), static_cast<uint32_t> (Last
()));
This has already been fixed, differently! in ns-3-dev.

https://codereview.appspot.com/336970043/diff/300001/src/core/model/heap-sche...
src/core/model/heap-scheduler.cc:237: Exch (i, static_cast<uint32_t> (Last ()));
This has already been fixed, differently! in ns-3-dev.

https://codereview.appspot.com/336970043/diff/300001/src/core/model/log.cc
File src/core/model/log.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/log.cc#ne...
src/core/model/log.cc:714: 
This is already in upstream, and unrelated to this issue.

https://codereview.appspot.com/336970043/diff/300001/src/core/model/log.h
File src/core/model/log.h (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/log.h#new...
src/core/model/log.h:531: 
This is already in upstream, and unrelated to this issue.

https://codereview.appspot.com/336970043/diff/300001/src/core/model/object-pt...
File src/core/model/object-ptr-container.h (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/object-pt...
src/core/model/object-ptr-container.h:262: return
(obj->*m_get)(static_cast<uint32_t> (i));
Unnecessary in ns-3-dev.  If it is necessary, the fix is outside core, to update
Get(), GetN() to size_t

https://codereview.appspot.com/336970043/diff/300001/src/core/model/realtime-...
File src/core/model/realtime-simulator-impl.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/realtime-...
src/core/model/realtime-simulator-impl.cc:376: if (tsJitter >
static_cast<uint64_t> (m_hardLimit.GetTimeStep ()))
Time::GetTimeStep returns int64_t.  Can we change the declaration of tsJitter
instead of casting? By pushing the cast into Synchronizer::GetCurrentRealtime

https://codereview.appspot.com/336970043/diff/300001/src/core/model/synchroni...
File src/core/model/synchronizer.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/synchroni...
src/core/model/synchronizer.cc:93: return -static_cast<int64_t>
(NanosecondToTimeStep (-tDrift));
Need to eliminate this cast as well, by chasing down the uint64_t declaration.

https://codereview.appspot.com/336970043/diff/300001/src/core/model/type-id.cc
File src/core/model/type-id.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/type-id.c...
src/core/model/type-id.cc:451: return static_cast<uint16_t> (uid);
Unnecessary cast; uid is already uint16_t

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ce...
File src/lte/test/lte-test-cell-selection.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ce...
src/lte/test/lte-test-cell-selection.cc:189: uint16_t nUe =
static_cast<uint16_t> (m_ueSetupList.size ());
Can nUe be size_t, so no cast would be necessary?

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ce...
src/lte/test/lte-test-cell-selection.cc:387: NS_TEST_ASSERT_MSG_EQ
(m_lastState.at (static_cast<unsigned int>(ueDev->GetImsi () - 1)),
Cast should be to size_t.  What's the guarantee the -1 doesn't go negative?

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ce...
src/lte/test/lte-test-cell-selection.cc:401: m_lastState.at
(static_cast<unsigned int>(imsi - 1)) = newState;
Cast should be to size_t.  What's the guarantee the -1 doesn't go negative?

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ph...
File src/lte/test/lte-test-phy-error-model.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ph...
src/lte/test/lte-test-phy-error-model.cc:273: double expectedDlTxPackets =
static_cast<double> (statsDuration.GetMilliSeconds ());
Use Time::ToDouble (Time::MS) instead of casting

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ph...
src/lte/test/lte-test-phy-error-model.cc:429: double expectedDlTxPackets =
static_cast<double> (statsDuration.GetMilliSeconds ());
Use Time::ToDouble (Time::MS) instead of casting

[ammo6818-vandals.uidaho.edu, 2018-05-16 15:06:29]

Updated patch submitted as https://codereview.appspot.com/342120043/ as
requested.

I have responded to some of the questions in this patch about why a certain
implementation could not be used.

https://codereview.appspot.com/336970043/diff/300001/src/core/model/attribute...
File src/core/model/attribute-accessor-helper.h (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/attribute...
src/core/model/attribute-accessor-helper.h:392: NS_UNUSED(v);
On 2018/05/11 21:58:11, Peter Barnes wrote:
> Previous two lines need space before paren. 

Fixed

https://codereview.appspot.com/336970043/diff/300001/src/core/model/hash-murm...
File src/core/model/hash-murmur3.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/hash-murm...
src/core/model/hash-murmur3.cc:525: m_hash32, (void *)& m_hash32);
On 2018/05/11 21:58:11, Peter Barnes wrote:
> Change size formal argument to size_t

Done

https://codereview.appspot.com/336970043/diff/300001/src/core/model/hash-murm...
src/core/model/hash-murmur3.cc:526: m_size32 += static_cast<uint32_t> (size);
On 2018/05/11 21:58:11, Peter Barnes wrote:
> Change m_size32 type to size_t

Done

https://codereview.appspot.com/336970043/diff/300001/src/core/model/hash-murm...
src/core/model/hash-murmur3.cc:556: MurmurHash3_x86_128_fin (static_cast<int>
(m_size64),
On 2018/05/11 21:58:11, Peter Barnes wrote:
> Change size formal argument to size_t

Done

https://codereview.appspot.com/336970043/diff/300001/src/core/model/heap-sche...
File src/core/model/heap-scheduler.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/heap-sche...
src/core/model/heap-scheduler.cc:156: uint32_t index = static_cast<uint32_t>
(Last ());
On 2018/05/11 21:58:11, Peter Barnes wrote:
> This has already been fixed, differently! in ns-3-dev.
> std::size_t index = Last ();

removed

https://codereview.appspot.com/336970043/diff/300001/src/core/model/heap-sche...
src/core/model/heap-scheduler.cc:220: Exch (Root (), static_cast<uint32_t> (Last
()));
On 2018/05/11 21:58:12, Peter Barnes wrote:
> This has already been fixed, differently! in ns-3-dev.

removed

https://codereview.appspot.com/336970043/diff/300001/src/core/model/heap-sche...
src/core/model/heap-scheduler.cc:237: Exch (i, static_cast<uint32_t> (Last ()));
On 2018/05/11 21:58:12, Peter Barnes wrote:
> This has already been fixed, differently! in ns-3-dev.

removed

https://codereview.appspot.com/336970043/diff/300001/src/core/model/log.cc
File src/core/model/log.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/log.cc#ne...
src/core/model/log.cc:714: 
On 2018/05/11 21:58:12, Peter Barnes wrote:
> This is already in upstream, and unrelated to this issue.

One of the previous review comments was support for uint8_t needed to be added
so this change in response to the other changes for this phase.

https://codereview.appspot.com/336970043/diff/300001/src/core/model/object-pt...
File src/core/model/object-ptr-container.h (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/object-pt...
src/core/model/object-ptr-container.h:262: return
(obj->*m_get)(static_cast<uint32_t> (i));
On 2018/05/11 21:58:12, Peter Barnes wrote:
> Unnecessary in ns-3-dev.  If it is necessary, the fix is outside core, to
update
> Get(), GetN() to size_t

Removed.

https://codereview.appspot.com/336970043/diff/300001/src/core/model/realtime-...
File src/core/model/realtime-simulator-impl.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/realtime-...
src/core/model/realtime-simulator-impl.cc:376: if (tsJitter >
static_cast<uint64_t> (m_hardLimit.GetTimeStep ()))
On 2018/05/11 21:58:12, Peter Barnes wrote:
> Time::GetTimeStep returns int64_t.  Can we change the declaration of tsJitter
> instead of casting? By pushing the cast into Synchronizer::GetCurrentRealtime 

Making the suggested change causes a number of other casts to be created (the
total number of casts required is larger).

https://codereview.appspot.com/336970043/diff/300001/src/core/model/type-id.cc
File src/core/model/type-id.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/core/model/type-id.c...
src/core/model/type-id.cc:451: return static_cast<uint16_t> (uid);
On 2018/05/11 21:58:12, Peter Barnes wrote:
> Unnecessary cast; uid is already uint16_t

Removed

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ce...
File src/lte/test/lte-test-cell-selection.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ce...
src/lte/test/lte-test-cell-selection.cc:189: uint16_t nUe =
static_cast<uint16_t> (m_ueSetupList.size ());
On 2018/05/11 21:58:12, Peter Barnes wrote:
> Can nUe be size_t, so no cast would be necessary?

No. ueNodes.Create does not take a size_t

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ce...
src/lte/test/lte-test-cell-selection.cc:387: NS_TEST_ASSERT_MSG_EQ
(m_lastState.at (static_cast<unsigned int>(ueDev->GetImsi () - 1)),
On 2018/05/11 21:58:13, Peter Barnes wrote:
> Cast should be to size_t.  What's the guarantee the -1 doesn't go negative?

changed to a size_t

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ce...
src/lte/test/lte-test-cell-selection.cc:401: m_lastState.at
(static_cast<unsigned int>(imsi - 1)) = newState;
On 2018/05/11 21:58:13, Peter Barnes wrote:
> Cast should be to size_t.  What's the guarantee the -1 doesn't go negative?

changed to a size_t

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ph...
File src/lte/test/lte-test-phy-error-model.cc (right):

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ph...
src/lte/test/lte-test-phy-error-model.cc:273: double expectedDlTxPackets =
static_cast<double> (statsDuration.GetMilliSeconds ());
On 2018/05/11 21:58:13, Peter Barnes wrote:
> Use Time::ToDouble (Time::MS) instead of casting

Changed as noted.

https://codereview.appspot.com/336970043/diff/300001/src/lte/test/lte-test-ph...
src/lte/test/lte-test-phy-error-model.cc:429: double expectedDlTxPackets =
static_cast<double> (statsDuration.GetMilliSeconds ());
On 2018/05/11 21:58:13, Peter Barnes wrote:
> Use Time::ToDouble (Time::MS) instead of casting

Changed as noted.