I still think they could be joined, but probably it would complicate a lot the code.
For example, one could use std::function
to handle both the "normal" functions and the member functions - eliminating the need for duplicated code.
BUT this will have a cascade effect on Simulator::Schedule
, so... scratch it.
I don't see how.
The problem is, that the types of the arguments passed to SetArguments()
are template dependent, i.e. with moving SetArguments()
from TimerImplX
to TimerImpl
it would become a template function (like SetArgs
). The function also needs to be virtual
to override it in FnTimerImpl
and MemFnTimerImpl
. But template functions cannot be virtual.
I don't want to block this MR, but I have a (maybe stupid) question.
Since we now have only one TimerImplX
, is it necessary to have it?
I mean, couldn't TimerImplX
and TimerImpl
be collapsed into just one class? A hierarchy was logical when we had TimerImplOne
, TimerImplTwo
and so on, to "hide" them. But now... I'm not that sure.
I'd hesitate to label this a good first issue. Let's be careful that we do not unnecessarily touch performance-sensitive code in the core. Perhaps Peter and/or Gabriel could take this one if they want to make this change and evaluate.
The topic was mentioned in a comment in !1914.
Using "raw" function pointers is questionable (with C++20), as std::function
offers - among other things - more control on the function being pointed, and makes the code easier to read.
A good MR should:
std:.function
Done.
Eduardo Almeida (a0dddf32) at 27 Mar 15:59
Replace variable length arrays by std::vector
Variable length array is a C feature, available in C++ when compiler extensions are enabled. Newer versions of the clang compiler flag these issues as errors, such as the one below:
src/wifi/model/wifi-mac-queue-container.cc:212:20: error: variable length arrays in C++ are a Clang extension [clang-diagnostic-vla-cxx-extension]
212 | uint8_t buffer[size];
| ^~~~
src/wifi/model/wifi-mac-queue-container.cc:212:20: note: initializer of 'size' is not a constant expression
src/wifi/model/wifi-mac-queue-container.cc:210:23: note: declared here
210 | const std::size_t size = tid.has_value() ? 8 : 7;
| ^
This MR replaces variable length C-style arrays by std::vector
. In some cases, C-style arrays can be kept, but its size has to be declared constexpr
.
The default compiler for Ubuntu 20.04 is still gcc-9, and it doesn't have <numbers>
.
Hence, the error reported in https://gitlab.com/nsnam/ns-3-dev/-/pipelines/1225274366
Our master branch does no longer compile on my virtual machine (Ubuntu 22.04, aarch64, gcc 13.1.0):
In file included from /home/parallels/repos/ns-3-dev/src/core/model/simulator.h:25,
from /home/parallels/repos/ns-3-dev/src/core/model/unix-fd-reader.cc:25:
/home/parallels/repos/ns-3-dev/src/core/model/make-event.h: In lambda function:
/home/parallels/repos/ns-3-dev/src/core/model/make-event.h:163:22: error: ‘<anonymous>’ may be used uninitialized [-Werror=maybe-uninitialized]
163 | m_function)(args...);
| ^~~~~~~~~~
/home/parallels/repos/ns-3-dev/src/core/model/make-event.h:163:22: note: ‘<anonymous>’ was declared here
163 | m_function)(args...);
| ^~~~~~~~~~
cc1plus: all warnings being treated as errors
The default compiler for Ubuntu 20.04 is still gcc-9, and it doesn't have <numbers>
.
Hence, the error reported in https://gitlab.com/nsnam/ns-3-dev/-/pipelines/1225274366
Our master branch does no longer compile on my virtual machine (Ubuntu 22.04, aarch64, gcc 13.1.0):
In file included from /home/parallels/repos/ns-3-dev/src/core/model/simulator.h:25,
from /home/parallels/repos/ns-3-dev/src/core/model/unix-fd-reader.cc:25:
/home/parallels/repos/ns-3-dev/src/core/model/make-event.h: In lambda function:
/home/parallels/repos/ns-3-dev/src/core/model/make-event.h:163:22: error: ‘<anonymous>’ may be used uninitialized [-Werror=maybe-uninitialized]
163 | m_function)(args...);
| ^~~~~~~~~~
/home/parallels/repos/ns-3-dev/src/core/model/make-event.h:163:22: note: ‘<anonymous>’ was declared here
163 | m_function)(args...);
| ^~~~~~~~~~
cc1plus: all warnings being treated as errors
Tommaso Pecorella (fa5c41b3) at 27 Mar 01:05
core: (fixes #1052) silence maybe-uninitialized warning in gcc 13.1
... and 3 more commits
@sebastienderonne That's more a point in modernizing the code and avoiding "C-style" constants Vs "C++20-style" numbers.
In my opinion is preferable to use the same approach in all the code, and to not mix-n-match.
Mind that the two numbers might differ in some decimals, with M_PI
being actually slightly more precise than numbers::pi
(at least on my clang).
@edalm I'd rather push the code and check it later on, possibly after "modernizing" the code.
One little issue is that this code is used very often, so any extra operation will cause performance problems. Hence, I'd rather avoid to use complex checks, and rather rely on specializations.
@edalm I'll be more than happy to kill the function pointer in a different MR. In my opinion using std::function
is far more readable.
Sébastien Deronne (8f3053fc) at 26 Mar 21:46
wifi: Extend multiple PHY interfaces test with an EMLSR use case
... and 4 more commits
The main goal of this MR is to fix an issue with multiple PHY interfaces encountered when running the following EMLSR scenario:
A PHY is on an initial band and receives a packet. Then, the PHY switches to another band where it starts receiving another packet. During reception of the PHY header, the PHY switches back to the initial band and starts receiving yet another packet. In this case, first and last packets should be successfully received (no interference), the second packet reception has been interrupted (before the payload reception does start, hence it does not reach the RX state). However, we observed that the last packet was not successfully received, taking into account the very first packet as an interference (which was by long time received by that PHY).