C++11 usage

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

C++11 usage

Alexander Krotov-2
Some time has passed since NS-3 switched to C++11, but still it is almost
never used.  I have already submitted a patch [1] that uses some C++11
features, but we have no guide on which features are acceptable. Coding
style [2] just mentions that C++11 features will be accepted but without
some guide on what is acceptable and what is not everyone seemingly
avoid using any new features at all.

I propose to explicitly list some accepted features in the coding
style so we can actually benefit from this change.


Some changes that I think should be acceptable:

1. Using `auto' before CreateObject and similar macros.

Just an example from some recent patch [3]:

  Ptr<WaypointMobilityModel> mob = DynamicCast<WaypointMobilityModel> (m_mobilityModel);

Should we start writing

  auto mob = DynamicCast<WaypointMobilityModel> (m_mobilityModel);

instead?

2. Using `auto' for iterators where typedefs are used now.

For example, here CacheI makes no more sense than auto,
but I still used it:

http://code.nsnam.org/ns-3-dev/rev/c10baf379016

Should CacheI typedef just be removed and replaced with auto?

3. Range-based for loops.

IMO those should be used everywhere when possible. Just looks at recent
bugfixes [4] [5].

Could be replaced with
  for (std::list<uint32_t>::iterator it2: it->cgiInfo.plmnIdentityList)
and
  for (std::vector<Ptr<WifiPhy>>::iterator j: phys)

This way it is much easier to avoid such bugs.

4. Variadic templates

This one is an obvious improvement over lots of duplicate code [6].

5. Strongly-typed enums

This one should be used for all state machines at least, really underused.
The only problem is that attribute system can't work with them now.


Some more controversial changes:

1. Use lambdas for small functions that are not going to be reused.

Can't think of good examples except for my patch [1] right now.  Tell me
what you think about it, maybe we can use it as a precedent.

2. Thread support.

Not sure how good gcc support is and if we can replace pthreads with
native C++ threads.

3. Uniform initialization

This one is really great in catching truncation bugs (implicitly
converting uint32_t to uint16_t for examples).  But using it with auto
should be prohibited at least for now. The meaning of some combinations of
auto and initializer lists have changed since g++ 4.8 (minimum version
supported by NS-3). See [7], Question 2 for details. Or just follow
google style guide here [8].

4. In-class member initialization.

Some code can really benefit from this change.  It is impossible to
spot unitialized variable when you have more than 50 initializations
in a row [9] and you will surely get those reorder warnings when trying
to add initialization of a new member.  Of course mixing styles within
one class should not be accepted. Otherwise, I don't see any problems,
maybe there are some hidden problems with this one.


Hopefully other developers can extend this list and provide feedback.

[1] https://www.nsnam.org/bugzilla/show_bug.cgi?id=2589#c2
[2] https://www.nsnam.org/developers/contributing-code/coding-style/#Language_features
[3] http://code.nsnam.org/ns-3-dev/rev/8c22110878fb
[4] http://code.nsnam.org/ns-3-dev/rev/fbfc8b94e287
[5] http://code.nsnam.org/ns-3-dev/rev/f7a4f17f4abb
[6] https://www.nsnam.org/bugzilla/show_bug.cgi?id=2457
[7] https://scottmeyers.blogspot.ru/2015/09/thoughts-on-vagaries-of-c-initialization.html
[8] https://google.github.io/styleguide/cppguide.html#Braced_Initializer_List
[9] http://code.nsnam.org/ns-3-dev/file/f7a4f17f4abb/src/internet/model/tcp-socket-base.cc#l266
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: C++11 usage

Tom Henderson-2
On 12/25/2016 02:31 PM, Alexander Krotov wrote:
> Some time has passed since NS-3 switched to C++11, but still it is almost
> never used.  I have already submitted a patch [1] that uses some C++11
> features, but we have no guide on which features are acceptable. Coding
> style [2] just mentions that C++11 features will be accepted but without
> some guide on what is acceptable and what is not everyone seemingly
> avoid using any new features at all.
>
> I propose to explicitly list some accepted features in the coding
> style so we can actually benefit from this change.

Alexander, thanks for your suggestions.  I agree with the suggestions in
general.  See inline below.

>
>
> Some changes that I think should be acceptable:
>
> 1. Using `auto' before CreateObject and similar macros.
>
> Just an example from some recent patch [3]:
>
>   Ptr<WaypointMobilityModel> mob = DynamicCast<WaypointMobilityModel> (m_mobilityModel);
>
> Should we start writing
>
>   auto mob = DynamicCast<WaypointMobilityModel> (m_mobilityModel);
>
> instead?
>
> 2. Using `auto' for iterators where typedefs are used now.
>
> For example, here CacheI makes no more sense than auto,
> but I still used it:
>
> http://code.nsnam.org/ns-3-dev/rev/c10baf379016
>
> Should CacheI typedef just be removed and replaced with auto?
>
> 3. Range-based for loops.
>
> IMO those should be used everywhere when possible. Just looks at recent
> bugfixes [4] [5].
>
> Could be replaced with
>   for (std::list<uint32_t>::iterator it2: it->cgiInfo.plmnIdentityList)
> and
>   for (std::vector<Ptr<WifiPhy>>::iterator j: phys)
>
> This way it is much easier to avoid such bugs.
>
> 4. Variadic templates
>
> This one is an obvious improvement over lots of duplicate code [6].

Agree on the above 1-4.

>
> 5. Strongly-typed enums
>
> This one should be used for all state machines at least, really underused.
> The only problem is that attribute system can't work with them now.

Do you have any idea whether the attribute value class could be extended
to work with them in a backward-compatible way?

>
>
> Some more controversial changes:
>
> 1. Use lambdas for small functions that are not going to be reused.
>
> Can't think of good examples except for my patch [1] right now.  Tell me
> what you think about it, maybe we can use it as a precedent.

Craig suggested this a while back (before we were supporting C++-11); I
think that something like he proposed would be potentially very useful:

https://www.nsnam.org/bugzilla/show_bug.cgi?id=2203
>
> 2. Thread support.
>
> Not sure how good gcc support is and if we can replace pthreads with
> native C++ threads.

I think we would need someone to test this under realtime and emulation
scenarios.

>
> 3. Uniform initialization
>
> This one is really great in catching truncation bugs (implicitly
> converting uint32_t to uint16_t for examples).  But using it with auto
> should be prohibited at least for now. The meaning of some combinations of
> auto and initializer lists have changed since g++ 4.8 (minimum version
> supported by NS-3). See [7], Question 2 for details. Or just follow
> google style guide here [8].

OK

>
> 4. In-class member initialization.
>
> Some code can really benefit from this change.  It is impossible to
> spot unitialized variable when you have more than 50 initializations
> in a row [9] and you will surely get those reorder warnings when trying
> to add initialization of a new member.  Of course mixing styles within
> one class should not be accepted. Otherwise, I don't see any problems,
> maybe there are some hidden problems with this one.

Agreed.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: C++11 usage

Alex Afanasyev-2
Hi all,

> On Dec 27, 2016, at 7:22 AM, Tom Henderson <[hidden email]> wrote:
>
> On 12/25/2016 02:31 PM, Alexander Krotov wrote:
>> Some time has passed since NS-3 switched to C++11, but still it is almost
>> never used.  I have already submitted a patch [1] that uses some C++11
>> features, but we have no guide on which features are acceptable. Coding
>> style [2] just mentions that C++11 features will be accepted but without
>> some guide on what is acceptable and what is not everyone seemingly
>> avoid using any new features at all.
>>
>> I propose to explicitly list some accepted features in the coding
>> style so we can actually benefit from this change.
>
> Alexander, thanks for your suggestions.  I agree with the suggestions in general.  See inline below.
>>
>>
>> Some changes that I think should be acceptable:
>>
>> 1. Using `auto' before CreateObject and similar macros.
>>
>> Just an example from some recent patch [3]:
>>
>>  Ptr<WaypointMobilityModel> mob = DynamicCast<WaypointMobilityModel> (m_mobilityModel);
>>
>> Should we start writing
>>
>>  auto mob = DynamicCast<WaypointMobilityModel> (m_mobilityModel);
>>
>> instead?
>>
>> 2. Using `auto' for iterators where typedefs are used now.
>>
>> For example, here CacheI makes no more sense than auto,
>> but I still used it:
>>
>> http://code.nsnam.org/ns-3-dev/rev/c10baf379016
>>
>> Should CacheI typedef just be removed and replaced with auto?
>>
>> 3. Range-based for loops.
>>
>> IMO those should be used everywhere when possible. Just looks at recent
>> bugfixes [4] [5].
>>
>> Could be replaced with
>>  for (std::list<uint32_t>::iterator it2: it->cgiInfo.plmnIdentityList)
>> and
>>  for (std::vector<Ptr<WifiPhy>>::iterator j: phys)
>>
>> This way it is much easier to avoid such bugs.

Agree with this.  There is one potential issue with NS-3 naming convention that uses .Begin(), .End() for iterators (e.g., NameList class), though I think there could be workarounds for this.

>>
>> 4. Variadic templates
>>
>> This one is an obvious improvement over lots of duplicate code [6].
>
> Agree on the above 1-4.
>
>>
>> 5. Strongly-typed enums
>>
>> This one should be used for all state machines at least, really underused.
>> The only problem is that attribute system can't work with them now.
>
> Do you have any idea whether the attribute value class could be extended to work with them in a backward-compatible way?

Do you mean getting a number from enum value?  It is possible, but needs to be done explicitly.

>> Some more controversial changes:
>>
>> 1. Use lambdas for small functions that are not going to be reused.
>>
>> Can't think of good examples except for my patch [1] right now.  Tell me
>> what you think about it, maybe we can use it as a precedent.
>
> Craig suggested this a while back (before we were supporting C++-11); I think that something like he proposed would be potentially very useful:
>
> https://www.nsnam.org/bugzilla/show_bug.cgi?id=2203 <https://www.nsnam.org/bugzilla/show_bug.cgi?id=2203>
>>
>> 2. Thread support.
>>
>> Not sure how good gcc support is and if we can replace pthreads with
>> native C++ threads.
>
> I think we would need someone to test this under realtime and emulation scenarios.
>
>>
>> 3. Uniform initialization
>>
>> This one is really great in catching truncation bugs (implicitly
>> converting uint32_t to uint16_t for examples).  But using it with auto
>> should be prohibited at least for now. The meaning of some combinations of
>> auto and initializer lists have changed since g++ 4.8 (minimum version
>> supported by NS-3). See [7], Question 2 for details. Or just follow
>> google style guide here [8].
>
> OK
>
>>
>> 4. In-class member initialization.
>>
>> Some code can really benefit from this change.  It is impossible to
>> spot unitialized variable when you have more than 50 initializations
>> in a row [9] and you will surely get those reorder warnings when trying
>> to add initialization of a new member.  Of course mixing styles within
>> one class should not be accepted. Otherwise, I don't see any problems,
>> maybe there are some hidden problems with this one.
>
> Agreed.

I like this, but if gcc 4.8 is the supported compiler, you can forget about it for now :(  At least this was my experience.

---
Alex

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: C++11 usage

Tom Henderson-2
On 12/27/2016 03:25 PM, Alex Afanasyev wrote:

>>> 5. Strongly-typed enums
>>>
>>> This one should be used for all state machines at least, really
>>> underused.
>>> The only problem is that attribute system can't work with them now.
>>
>> Do you have any idea whether the attribute value class could be
>> extended to work with them in a backward-compatible way?
>
> Do you mean getting a number from enum value?  It is possible, but
> needs to be done explicitly.

I was thinking of existing EnumValue class being used for both
traditional and strongly typed enums, but it may be easier to simply
define a new EnumClassValue side-by-side with EnumValue, and start to
replace the old usage over time.

>
>>> 4. In-class member initialization.
>>>
>>> Some code can really benefit from this change.  It is impossible to
>>> spot unitialized variable when you have more than 50 initializations
>>> in a row [9] and you will surely get those reorder warnings when trying
>>> to add initialization of a new member.  Of course mixing styles within
>>> one class should not be accepted. Otherwise, I don't see any problems,
>>> maybe there are some hidden problems with this one.
>>
>> Agreed.
>
> I like this, but if gcc 4.8 is the supported compiler, you can forget
> about it for now :(  At least this was my experience.

gcc-4.8 and clang-3.3 are currently documented as our minimum supported
compiler versions, unless someone proposes to raise the bar.  I am not
sure there are supported systems still using clang-3.3, however; 3.6 or
3.7 seems to be the minimal versions in use on Macs (Xcode 6 or greater).

- Tom
Loading...