Fix Bug "28480 - cppcoreguidelines-pro-bounds-array-to-pointer-decay handling __PRETTY_FUNCTION__"

[ericLemanissier]

Summary

Ignore array to pointer decay for predefined macros. It is ok because there is no safer way to pass these macros to functions

[aaron.ballman]

I agree with your assessment that these should be safe, but the C++ Core Guidelines themselves don't call this case out as part of the enforcement (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#bounds3-no-array-to-pointer-decay). Have you also filed a pull request (or a bug report) there to see if the maintainers agree with this approach?

[ericLemanissier]

Well, C++ Core Guidelines advises in this case to correct function receiving PRETTY_FUNCTION: the parameter type should not be const char* ( https://github.com/isocpp/CppCoreGuidelines/issues/640 )
I'm not sure if it should be cstring_span, czstring_span, or czstring.
In case it is not possible to change this function, I guess the only solution is static_cast<const char*>(__PRETTY_FUNCTION)

[aaron.ballman]

In D22196#481567, @ericLemanissier wrote:

Well, C++ Core Guidelines advises in this case to correct function receiving PRETTY_FUNCTION: the parameter type should not be const char* ( https://github.com/isocpp/CppCoreGuidelines/issues/640 )
I'm not sure if it should be cstring_span, czstring_span, or czstring.
In case it is not possible to change this function, I guess the only solution is static_cast<const char*>(__PRETTY_FUNCTION)

It's not that it's not possible to change this function; I actually think the change is pretty sensible. It's mostly that we want to make sure that the check we implement matches the expected behavior of the C++ Core Guidelines. I think a good approach is to propose this patch (and the test cases) to the C++ Core Guidelines folks via their GitHub tracker, and if they agree with the behavior of your patch, then we can go ahead and add the functionality under the assumption the core guidelines will be updated accordingly as well.

[ericLemanissier]

This comment was removed by ericLemanissier.

[ericLemanissier]

having studied cpp core guidelines in more depth, I completely understand that the problem in the case I described is that the function receiving PRETTY_FUNCTION takes a const char* parameter, whereas it should take a czstring or szstring_span. As a consequence this patch is not a good thing.

In case it is not possible to change this function, I guess the only solution is static_cast<const char*>(__PRETTY_FUNCTION)

The function I refer to in this sentence is not ProBoundsArrayToPointerDecayCheck::registerMatchers, it is the function in user code receiving PRETTY_FUNCTION.

[• Zitrax]

Would it be an alternative to add the this patch as an option to the check (disabled by default)? Thus one could allow for example PRETTY_FUNCTION without having to disable this whole check while still letting it fully follow the core guidelines by default.

clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp

void ProBoundsArrayToPointerDecayCheck::registerMatchers(MatchFinder *Finder) {
   // The only allowed array to pointer decay
   // 1) just before array subscription
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
       implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
                        unless(hasParentIgnoringImpCasts(explicitCastExpr())),
                        unless(isInsideOfRangeBeginEndStmt()),
-                       unless(hasSourceExpression(stringLiteral())))
+                       unless(hasSourceExpression(stringLiteral())),
+                       unless(hasDescendant(predefinedExpr())))
           .bind("cast"),
       this);
 }
 
 void ProBoundsArrayToPointerDecayCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *MatchedCast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast");
   if (MatchedCast->getCastKind() != CK_ArrayToPointerDecay)

test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp

const char *g() {
   return "clang"; // OK, decay string literal to pointer
 }
 
 void f2(void *const *);
 void bug25362() {
   void *a[2];
   f2(static_cast<void *const*>(a)); // OK, explicit cast
 }
+
+void cstringfun(const char *s);
+void bug28480() {
+    cstringfun(__PRETTY_FUNCTION__);    //OK, predefined macro
+    cstringfun(__func__);               //OK, predefined macro
+    cstringfun(__FUNCTION__);           //OK, predefined macro
+}