I have a private static vector in my class that keeps a pointer to all objects created from it. It's necessary as each object needs access to information from all the other objects to perform some calculations:
// Header file:
class Example {
public:
Example();
private:
static std::vector<const Example*> examples_;
};
// Cpp file:
std::vector<const Example *> Example::examples_ = {};
Example::Example() {
// intialization
examples_.emplace_back(this);
}
void Example::DoCalc() {
for (auto example : examples_) {
// do stuff
}
}
clang-tidy
points out that I'm violating C++ Core Guidelines, namely : "Variable 'examples_' is non-const and globally accessible, consider making it const (cppcoreguidelines-avoid-non-const-global-variables)".
Personally, I don't see the resemblance between my code and the sample code in the core guidelines, especially since the variable is inside a class and private. What would be the 'correct' way of implementing this functionality? I don't want to disable this check from clang-tidy if it can be avoided.
A possible solution is to force each client that accesses Example::examples_
to go through a function. Then put examples
as a static variable into that function. That way the object will be created the first time the function is called - independent of any global object construction order.
// Header file:
class Example {
public:
Example();
private:
std::vector<const Example*>& examples();
};
// Cpp file:
std::vector<Example *>& Example::examples()
{
static std::vector<Example *> examples_;
return examples_;
};
Example::Example() {
// intialization
examples().emplace_back(this);
}
void Example::DoCalc() {
for (auto example : examples()) {
// do stuff
}
}
Of course if you are sure that you have no problem with global objects and are sure that no other global object is accessing Examples::examples_
during its construction, you can ignore the warning. It is just a guideline, you don't need to follow it strictly.
As Asteroids With Wings noted the guideline I.2 does not apply to your code. But please note that the CoreGuidelines intend to ban static members as well, see To-do: Unclassified proto-rules:
avoid static class members variables (race conditions, almost-global variables)
What you've done is fine. This is literally the purpose of class-static
. Some people would recommend alternatives, for unrelated reasons, which may be worth considering… but not because of anything clang-tidy
is telling you here.
You've run into clang-tidy
bug #48040. You can see this because it's wrong in its messaging: the vector is not "globally accessible", at least not in the sense of access rules, since it's marked private
(though it's globally present across translation units, which is fine).
Your code doesn't relate to the cited core guideline.
Personally, I don't see the resemblance between my code and the sample code in the core guidelines
You have a single variable that is accessible to every thread, hidden from users of Example
. The only difference to an ordinary global variable is that it is private
, i.e. you can't use the name Example::examples_
to refer to it outside of Example
.
Note
The rule is "avoid", not "don't use."
The "correct" way of implementing this functionality might be how you have it, but I strongly suggest you rework "each object needs access to information from all the other objects to perform some calculations" so that you pass a std::vector<const Example*>
to where it is needed, having kept track of all the relevant (and especially alive) Example
s where they are used.
Alternative: [...] Another solution is to define the data as the state of some object and the operations as member functions.
Warning: Beware of data races: If one thread can access non-local data (or data passed by reference) while another thread executes the callee, we can have a data race. Every pointer or reference to mutable data is a potential data race.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With