Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(344)

Issue 336970043: Eliminate Visual Studio compiler warnings

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 months, 2 weeks ago by ammo6818-vandals.uidaho.edu
Modified:
6 days, 17 hours ago
Visibility:
Public.

Description

Eliminate Visual Studio compiler warnings

Patch Set 1 #

Patch Set 2 : Additional updates #

Patch Set 3 : Updates to use C++ style type casts #

Patch Set 4 : Updated patch set to reduce number of casts #

Patch Set 5 : Updates to patch set to eliminate some casts. Add ability to tolerate multiple class registrations… #

Patch Set 6 : Patch updates for x64 build #

Patch Set 7 : Additional changes for x64 build #

Patch Set 8 : Patch updates to resolve review comments #

Patch Set 9 : Updates to patch for review comments #

Total comments: 4

Patch Set 10 : Updated patch for review comments #

Total comments: 25

Patch Set 11 : Updates for latest comments #

Patch Set 12 : Review and update of patch set to make sure all comments are addressed #

Total comments: 34
Unified diffs Side-by-side diffs Delta from patch set Stats (+350 lines, -219 lines) Patch
M src/core/examples/hash-example.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M src/core/helper/random-variable-stream-helper.cc View 1 2 3 4 5 6 7 1 chunk +3 lines, -2 lines 0 comments Download
M src/core/model/attribute-accessor-helper.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +5 lines, -0 lines 2 comments Download
M src/core/model/cairo-wideint.c View 1 2 3 4 5 6 7 8 5 chunks +9 lines, -7 lines 0 comments Download
M src/core/model/calendar-scheduler.h View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download
M src/core/model/calendar-scheduler.cc View 1 2 3 4 5 6 7 8 9 4 chunks +5 lines, -5 lines 0 comments Download
M src/core/model/command-line.cc View 1 2 3 4 5 6 7 8 4 chunks +5 lines, -5 lines 0 comments Download
M src/core/model/config.h View 1 2 3 4 5 6 7 8 9 2 chunks +3 lines, -3 lines 0 comments Download
M src/core/model/config.cc View 1 2 3 4 5 6 7 8 9 8 chunks +15 lines, -15 lines 0 comments Download
M src/core/model/hash-fnv.cc View 1 2 3 4 5 6 7 2 chunks +3 lines, -3 lines 0 comments Download
M src/core/model/hash-murmur3.cc View 1 2 3 4 5 6 7 8 5 chunks +9 lines, -7 lines 6 comments Download
M src/core/model/heap-scheduler.h View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M src/core/model/heap-scheduler.cc View 1 2 3 4 5 6 7 8 9 4 chunks +4 lines, -4 lines 6 comments Download
M src/core/model/int64x64-cairo.h View 1 2 3 4 5 6 7 2 chunks +4 lines, -4 lines 0 comments Download
M src/core/model/int64x64-cairo.cc View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download
M src/core/model/int64x64-double.h View 1 2 3 4 5 6 7 3 chunks +6 lines, -5 lines 0 comments Download
M src/core/model/log.h View 1 2 3 4 1 chunk +18 lines, -0 lines 1 comment Download
M src/core/model/log.cc View 1 2 3 4 5 6 7 8 9 1 chunk +32 lines, -0 lines 2 comments Download
M src/core/model/nstime.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M src/core/model/object-base.cc View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -2 lines 0 comments Download
M src/core/model/object-map.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -2 lines 0 comments Download
M src/core/model/object-ptr-container.h View 1 2 3 4 5 6 7 8 9 7 chunks +9 lines, -9 lines 2 comments Download
M src/core/model/object-ptr-container.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +6 lines, -6 lines 0 comments Download
M src/core/model/object-vector.h View 1 2 3 4 5 6 7 2 chunks +3 lines, -3 lines 0 comments Download
M src/core/model/random-variable-stream.h View 1 2 3 4 5 6 7 8 9 2 chunks +4 lines, -4 lines 0 comments Download
M src/core/model/random-variable-stream.cc View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -2 lines 0 comments Download
M src/core/model/realtime-simulator-impl.cc View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 2 comments Download
M src/core/model/rng-seed-manager.cc View 1 2 3 4 5 6 7 3 chunks +11 lines, -11 lines 0 comments Download
M src/core/model/simple-ref-count.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +5 lines, -1 line 0 comments Download
M src/core/model/simulator.h View 1 2 3 4 5 6 7 8 1 chunk +8 lines, -2 lines 0 comments Download
M src/core/model/synchronizer.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -2 lines 1 comment Download
M src/core/model/system-path.cc View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M src/core/model/test.h View 1 2 3 4 5 6 7 8 3 chunks +7 lines, -7 lines 0 comments Download
M src/core/model/type-id.h View 1 2 3 4 5 6 7 8 4 chunks +9 lines, -9 lines 0 comments Download
M src/core/model/type-id.cc View 1 2 3 4 5 6 7 8 9 10 11 15 chunks +41 lines, -40 lines 2 comments Download
M src/core/model/wall-clock-synchronizer.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M src/core/test/attribute-test-suite.cc View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +23 lines, -5 lines 0 comments Download
M src/core/test/callback-test-suite.cc View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +25 lines, -4 lines 0 comments Download
M src/core/test/config-test-suite.cc View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +11 lines, -2 lines 0 comments Download
M src/core/test/int64x64-test-suite.cc View 1 2 3 4 5 6 7 3 chunks +4 lines, -4 lines 0 comments Download
M src/core/test/random-variable-stream-test-suite.cc View 1 2 3 4 5 6 7 4 chunks +8 lines, -4 lines 0 comments Download
M src/core/test/simulator-test-suite.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -0 lines 0 comments Download
M src/core/test/traced-callback-test-suite.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +5 lines, -0 lines 0 comments Download
M src/core/test/type-id-test-suite.cc View 1 2 3 4 5 6 7 8 9 3 chunks +3 lines, -3 lines 0 comments Download
M src/lte/examples/lena-intercell-interference.cc View 1 2 3 4 5 6 7 10 11 1 chunk +1 line, -1 line 0 comments Download
M src/lte/examples/lena-pathloss-traces.cc View 1 2 3 4 5 6 7 8 10 11 1 chunk +1 line, -1 line 0 comments Download
M src/lte/test/lte-test-cell-selection.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -2 lines 0 comments Download
M src/lte/test/lte-test-cell-selection.cc View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +5 lines, -5 lines 6 comments Download
M src/lte/test/lte-test-phy-error-model.cc View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +6 lines, -6 lines 4 comments Download
M src/lte/test/lte-test-rlc-am-e2e.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M src/lte/test/lte-test-secondary-cell-selection.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -2 lines 0 comments Download
M src/lte/test/lte-test-secondary-cell-selection.cc View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +7 lines, -7 lines 0 comments Download

Messages

Total messages: 14
ammo6818-vandals.uidaho.edu
Please let me know if there are any other review comments or merge into ns-3-dev
3 months ago (2018-02-18 05:44:41 UTC) #1
Tom Henderson
Once https://codereview.appspot.com/336970043/diff/220001/src/core/examples/sample-log-time-format.cc File src/core/examples/sample-log-time-format.cc (right): https://codereview.appspot.com/336970043/diff/220001/src/core/examples/sample-log-time-format.cc#newcode127 src/core/examples/sample-log-time-format.cc:127: #if 0 The point of this example is ...
3 months ago (2018-02-20 02:19:28 UTC) #2
ammo6818-vandals.uidaho.edu
Update posted to address latest review comments https://codereview.appspot.com/336970043/diff/220001/src/core/examples/sample-log-time-format.cc File src/core/examples/sample-log-time-format.cc (right): https://codereview.appspot.com/336970043/diff/220001/src/core/examples/sample-log-time-format.cc#newcode127 src/core/examples/sample-log-time-format.cc:127: #if 0 ...
3 months ago (2018-02-20 04:40:39 UTC) #3
Tom Henderson
> > I matched the implementation found in ParameterLogger::operator<< > <std::string>(const std::string param) which doesnt ...
3 months ago (2018-02-20 05:14:23 UTC) #4
Peter Barnes
On 2018/02/18 05:44:41, ammo6818-vandals.uidaho.edu wrote: > Please let me know if there are any other ...
3 months ago (2018-02-21 23:16:33 UTC) #5
ammo6818-vandals.uidaho.edu
On 2018/02/21 23:16:33, Peter Barnes wrote: > On 2018/02/18 05:44:41, http://ammo6818-vandals.uidaho.edu wrote: > > Please ...
2 months, 3 weeks ago (2018-02-25 12:12:16 UTC) #6
Tom Henderson
Just some minor things; would like to include this by Tuesday (as two patches-- doxygen ...
2 months, 3 weeks ago (2018-02-25 18:14:57 UTC) #7
ammo6818-vandals.uidaho.edu
Added explanations to the review comments. https://codereview.appspot.com/336970043/diff/260001/src/core/examples/hash-example.cc File src/core/examples/hash-example.cc (right): https://codereview.appspot.com/336970043/diff/260001/src/core/examples/hash-example.cc#newcode362 src/core/examples/hash-example.cc:362: long double k64 ...
2 months, 3 weeks ago (2018-02-26 13:03:20 UTC) #8
Peter Barnes
I thought Robert revised patch 10, but it appears to be the same as last ...
2 months, 3 weeks ago (2018-02-26 18:27:02 UTC) #9
ammo6818-vandals.uidaho.edu
Updates posted for latest comments https://codereview.appspot.com/336970043/diff/260001/src/core/examples/command-line-example.cc File src/core/examples/command-line-example.cc (right): https://codereview.appspot.com/336970043/diff/260001/src/core/examples/command-line-example.cc#newcode86 src/core/examples/command-line-example.cc:86: CommandLine cmd; On 2018/02/25 ...
2 months, 3 weeks ago (2018-02-27 13:11:05 UTC) #10
ammo6818-vandals.uidaho.edu
One of the things that I am struggling with on this issue is who is ...
2 months, 3 weeks ago (2018-03-02 23:22:44 UTC) #11
ammo6818-vandals.uidaho.edu
I have reverified the patch includes off the changes provided in the two attachments and ...
2 months ago (2018-03-22 14:47:01 UTC) #12
Peter Barnes
https://codereview.appspot.com/336970043/diff/300001/src/core/model/attribute-accessor-helper.h File src/core/model/attribute-accessor-helper.h (right): https://codereview.appspot.com/336970043/diff/300001/src/core/model/attribute-accessor-helper.h#newcode392 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-murmur3.cc ...
1 week, 4 days ago (2018-05-11 21:58:13 UTC) #13
ammo6818-vandals.uidaho.edu
6 days, 17 hours ago (2018-05-16 15:06:29 UTC) #14
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld e4aa27b