Rule "cpp:S6018 - use inline variables" should not apply to function local variables

Regarding rule “cpp:S6018 - use inline variables to declare this global variable”.

I think the scope of this rule should be altered so that it does not apply to function local variables.
Consider the following code example:

const std::string& ProcessStatus::getStatusFileName()
   static const std::string statFile = "/proc/self/status";
   return statFile;

Rule cpp:S6018 flags statFile as being more appropriate as a C++17 inline variable belonging to the ProcessStatus class.
However, this is in fact a constant used in a single function. Therefore it makes sense to limit the constants scope to that function. Changing it to class scope would pollute the class with unnecessary information.

I also note the rule has a tag of “clumsy”. Does that refer to the rule itself or the cases of code where it applies?

Hi @KantarBruceAdams,

In your example, the reference to the variable is returned from the function, so it is not only used in that function, but also by the code that is calling this function. Note that the callers need to be aware that this function returns a reference to static, to know when it is safe to use returned reference. Furthermore, the inline static function variables are initialized during the first call, and as consequence, they require a synchronization (mutex) to be held during initialization, as the function may be called from multiple threads concurrently.

In contrast defining this variable as a static member conveys all of the above information, and eliminates the required synchronization case. This is why the rule is raised in this case. Note, that if the function would not reference to that variable (return statFile) the issue would not be raised.

The tags clumsy refers to the code patterns detected by the rule. Per [Built-in Rule Tags | SonarQube Docs]:

clumsy - extra steps are used to accomplish something that could be done more clearly and concisely. (E.G. calling .toString() on a String).

In this case, this refers to use of inline function (and synchronization) to achieve something that could be done just by declaring variables.

I believe this is thread safe. See c++ - Is Meyers' implementation of the Singleton pattern thread safe? - Stack Overflow where the accepted and highly upvoted answer quotes the C++11 standard as follows:

In C++11, it is thread safe. According to the standard, §6.7 [stmt.dcl] p4 :

If control enters the declaration concurrently while the variable is being initialized, the concurrent execution shall wait for completion of the initialization.

This was not guaranteed to true in C++03 when the standard was not thread-aware. Though g++ added code to make it safe. Post C++11 at least some versions of the Microsoft C++ compiler were broken.

And that is for non-constant variables so constants should be doubly safe.
In fact if I look in the assembly compiled I see:

.string "/proc/self/status"

Regarding visibility and lifetime guarantees. Callers need to be aware of the lifetime but they do not need to know how or where it is implemented. Lifetime guarantees are outside of the scope of this snippet but I would assume given a const& return that it would live at least as long as the object does unless otherwise specified.

Ah sorry for being unclear. The function local variables are initialized in a thread-safe manner, and that requires check and synchronization to be inserted for each call of the function by the compiler. Compared to global/static variables that are initialized before the main; this causes additional runtime overhead. This is the case for all static variables that require dynamic initialization (as is the case for the string), regardless if they are declared const.

Detecting such situations and replacing them with the inline global variables is the whole point of this rule, so excluding it would be equivalent to disabling the rule itself (which is an option).

As a side note, the assembly line you quoted, is referring to "/proc/self/status" literal, not statFile string.