`M0-1-10`: Generates lot of false positives for used functions

[rak3-sh]

Affected rules

• M0-1-10

Description

The query M0-1-10 generates a significant amount of false positives while analyzing "unused functions". These are noticed especially in the following scenarios.

• constexpr functions
• template instantiations
• special member functions (constructors, destructors, operators, conversion operators)

This results from the fact that CodeQL cannot track the usages of such functions in compile time constructs like constexpr, static_asserts, templates etc.

Example

test.hpp

template <class T>
constexpr T to_underlying(T value) noexcept { // M0-1-3 violation reported here
  return static_cast<T>(value);
}

test.cpp

#include <type_traits>
#include "test.hpp"

template <typename T1, typename T2>
constexpr bool StaticAssertTypeEq() noexcept {
  static_assert(std::is_same<T1, T2>::value, "T1 and T2 are not the same type");
  return true;
}

template <typename T, int val>
class X
{
  T arr[val];
};

void foo()
{
  struct dummy {
    dummy() noexcept(false) { static_cast<void>(0); } // M0-1-3 violation reported here
  };
  // usage of dummy
  static_assert(!std::is_nothrow_default_constructible<X<dummy, 5>>::value,
                "Must not be nothrow default constructible");
}

int main()
{
  int a;
  StaticAssertTypeEq<decltype(to_underlying(a)), int>(); // usage of to_underlying
  foo();
  return 0;
}

[rak3-sh]

Proposed Fix(es)

Hi @lcartey, kindly let me know your valuable insights whether the following can be acceptable or not.

The MISRA2008 standard doesn't highlight any specific exception for this rule. However, considering CodeQL's limitation and to make it more practical to check the results of this query, can we propose to restrict this query in the following way? I would like to seek opinions about the same.

• Avoid analyzing functions that are constexpr because CodeQL has known limitations about tracking the usages of constexpr functions/expressions.
• Avoid analyzing explicitly defaulted functions. Since the user has marked it as "default", should we consider these as candidates for unused functions?
• Avoid analyzing functions which result from template instantiations. Since the function has been instantiated, there should be an explicit call.
• Avoid analyzing special member functions (constructors, destructors, operators, conversion operators). This is proposed mainly because their usages in compile time constructs are not tracked properly by CodeQL.
• Treating test stubs from GoogleTest framework as a likely entry point. E.g. a function named "TestBody" can be treated as a GoogleTest function and considered as an entry point. We can think of making it more specific by checking GoogleTest's stub function's characteristics.

[lcartey]

Hi Rakesh. Thanks again for reporting this. To take each suggestion in turn:

Avoid analyzing functions that are constexpr because CodeQL has known limitations about tracking the usages of constexpr functions/expressions.

Yes, I think that would be reasonable.

Avoid analyzing explicitly defaulted functions. Since the user has marked it as "default", should we consider these as candidates for unused functions?

default was added to the language after MISRA C++ 2008, so wasn't considered when the rule was written. While you could argue that a default function that is never called is unused in the strict sense of the word, I think

Avoid analyzing functions which result from template instantiations. Since the function has been instantiated, there should be an explicit call.

Functions from template instantiations should already be excluded (but contribute to whether the uninstantiated version of the function is considered used or not). Do you have a specific example where they are not?

Avoid analyzing special member functions (constructors, destructors, operators, conversion operators). This is proposed mainly because their usages in compile time constructs are not tracked properly by CodeQL.

I think we can exclude special member functions which are marked constexpr - because those I would expect we may not be able to report accurately, due to the automatic folding of constants. In other cases, I would expect to have enough information in the database to judge whether the function is or isn't used.

I note that the equivalent rule in MISRA C++ 2023 (Rule 0.2.4) does exclude special member functions, so another option would be to split the rule into two queries (one for special member functions, one without) so as to enable deviations to be applied to this case.

Treating test stubs from GoogleTest framework as a likely entry point. E.g. a function named "TestBody" can be treated as a GoogleTest function and considered as an entry point. We can think of making it more specific by checking GoogleTest's stub function's characteristics

Yes, the query already has a concept of an EntryPoint abstract class that can be extended to support other types of entry points.

[rak3-sh]

Thanks a lot @lcartey for your valuable inputs! I will incorporate suggestions (1), (4) and (5) above. Any thoughts on finding out the call to the dummy constructor in the example above? I can see that there is a TypeMention for dummy at the static_assert statement location, so I was thinking maybe I can check if an unused constructor's TypeMention resides in a used function, I can mark that constructor call as "used". But, I am not able to get this information from the AST.

[lcartey]

Closing as completed by #725.