FP / C++ / S5421 : Global variables

Rules and Languages Report False-positive / False-negative...
1

Hello,

Non constant variables declared in an anonymous namespace raise a S5421 issue (“Non-const global variables should not be used”) :

namespace
{
    int a = 5;
}

“Global” means “global to the whole program”, and so this rule shouldn’t cause any trouble for variables declared in anonymous namespaces.

Raised by Sonar for Visual Studio 8.29.0.14599

4

Hi @Oodini ,

Thanks for the question and sorry for the delayed answer.

It’s true that a variable scoped in an anonymous namespace is less an issue than a variable in the global namespace. But they are still not thread-safe, can also create initialization order bugs and make reasoning on the code more complex than passing them as parameters to the functions using them. That is why it was decided to still apply the rule to them.

That being said, we recognize that in some cases, such a variable can still be necessary, so don’t hesitate to “Accept” the issue if you think the alternative would be worst in your specific case.

6

Hi @Fred_Tingaud ,

All the points you mention also apply to private static data members — and it seems that Sonar doesn’t raise any issue in that case.

Let’s go through your points one by one, while keeping in mind that:

  • They are still not thread-safe, just like any other data member.

  • They can still create initialization-order issues, exactly as any private static data member can.

  • They can still make reasoning about the code more complex than passing data explicitly as function parameters — but when a member function calls another private member function, do you pass the data members as parameters?

The Sonar rule refers to a C++ Core Guidelines recommendation.
The main reason given there is that non-const global variables hide dependencies and make those dependencies subject to unpredictable changes.

But these dependencies are not hidden (or at least, no more than dependencies to regular data members) if you follow John Lakos’s architectural model — one .h/.cpp pair per component.
Variables declared in an anonymous namespace are used only within that component.
All dependencies remain in the same translation unit, and are therefore predictable.

Why not prefer a private static member instead?
Because the anonymous namespace actually improves binary encapsulation.
It avoids introducing, through the header, a dependency on types that are used only for the implementation.

I understand your point of view, but there should really be two different rules, because my case is not unreasonable.
If this is an architectural principle applied consistently across the entire codebase, I would rather not add // NOSONAR comments that would misleadingly suggest I’m breaking a good rule — one that I do want to apply to real global variables.
Variables declared in an anonymous namespace are not global variables.
“Global” means “global to the entire application.”
Please don’t conflate the two cases.

7

Hi @Oodini

After discussing with the team, we think there are still better alternatives than the variable in an anonymous namespace:

  • Regarding static members of a class, note that they do not need to be in a class declared in the header. From what you describe, they should be part of a class that is declared in the same anonymous namespace your variable is currently declared in. That would allow you to declare it as private and only access it through dedicated methods.
  • An approach that avoids the serious problem of initialization order is a function that returns a reference to a local static variable:
namespace {
Logger& logger() {
   static Logger myLogger{__FILE__};
   return myLogger;
}
}
  • And when it is reasonable to do so, passing a variable around as parameter is a great solution.
8

Hi @Fred_Tingaud

  • I was comparing to any static member of a class, not as a replacement of my static variable. Generally, do you raise an issue for the static variables, which have the same drawbacks than the variables in an anonymous namespace ?
  • I do know that approach, but that’s not the point.
  • So, is it reasonable to pas around a (static or not) data member between function members ? It is less reasonable than passing around a variable which could be in an anonymous namespace ? If so, why ? I explained than members may have the same drawbacks than variables in anonymous namespace.

OK, but don’t raise an issue talking about global variable. It is not a global variable. You’re confusing the C++ developper. Some arguments against global variable are not valid with variables in an anonymous namespace.

9

Thanks for your feedback, we’ll try to improve the rule description and message!