[clang-tidy] Do not warn on pointer decays in system macros

[estan]

Summary

As system headers are in general out of reach, it makes no sense to warn on pointer decays happening in them.

This is a reboot of https://reviews.llvm.org/D31130
It is essentially unchanged, only updated to the new repository as well as minor API changes.

[njames93]

Can you please upload this diff with full context -U999999

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp

50

Is this matcher necessary, there is already a matcher hasCastKind that'll do this functionality

56

Elide braces on single line if branches

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

13

Unrelated formatting changes

36

Unrelated formatting change

46

Unrelated formatting change

52

Unrelated formatting change

[fiesh]

Applied changes to address code review

[fiesh]

@njames93 Thank you for your review, I hopefully addressed all your points.

[aaron.ballman]

Despite my earlier happiness with the patch, it's now a bit unclear to me from the C++ Core Guideline wording what the right behavior here actually is. The bounds profile is trying to ensure you don't access out-of-range elements of a container and the decay part specifically seems to be more about how it impacts pointer arithmetic.

My intuition is that sys_fn_decay_str(__PRETTY_FUNCTION__) and sys_fn_decay_str(SYS_STRING) outside of a system header should diagnose the same way, as should the user_fn_decay_str variants of it. Both are decays of a string expanded from something outside of the user's control and requiring a cast for one and not the other seems inexplicable. However, it's not clear to me whether the C++ core guidelines would expect a diagnostic for these cases or not, but we currently diagnose one way and not the other, which seems wrong.

WDYT?

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp

56–58

if (const ValueDecl *VD = SymDeclRef->getDecl())
  return SM.isInSystemHeader(VD->getLocation());

[fiesh]

@aaron.ballman valid point. It would seem natural to diagnose both since the user is voluntarily causing the decay?

[aaron.ballman]

In D88833#2319118, @fiesh wrote:

@aaron.ballman valid point. It would seem natural to diagnose both since the user is voluntarily causing the decay?

I'm on the fence. If the argument to the function is one that's defined by a system header, you could argue that the user isn't voluntarily causing the decay -- they're using the API they were given by the system. If you're calling a user-defined function with that argument, then you could say that the user could change their function signature to remove the decay. If you're calling a system-defined function with that argument, then there's really nothing the user can do except add a cast that they'll likely view as useless. So at the very least, I think if the function declaration and the argument to the function are in a system header, then we shouldn't warn.

I think for a user-declared function, my intuition is that using an argument from a system header which causes decay is pretty uncommon but that we should follow the same behavior for the argument as we do for predefined expressions like __PRETTY_FUNCTION__. Because the argument is outside of the user's control, asking them to jump through hoops with their function declaration seems like it wouldn't likely add a lot of value. However, I'm not firmly tied to that. For user-declared functions, I just want to make sure we treat __PRETTY_FUNCTION__ and SYS_STRING consistently.

[lebedev.ri]

I think the current behavior should instead be configurable, with a way to opt-in into weaker guarantees.

[aaron.ballman]

In D88833#2319799, @lebedev.ri wrote:

I think the current behavior should instead be configurable, with a way to opt-in into weaker guarantees.

I think that's a good idea, thank you!

[arysef]

Here are also some bugs that seem like they could be closed once this is merged:
https://bugs.llvm.org/show_bug.cgi?id=28480
https://bugs.llvm.org/show_bug.cgi?id=32239

Also about the current behavior, it seems like this behavior happens even when it's a system header macro that is called directly (for example any time you use assert) rather than being one that is passed as a parameter. In those cases, the behavior seems undesired.

[estan]

@lebedev.ri @aaron.ballman I think the opt-in sounds good too. Are we waiting for @fiesh to implement this, or is the patch abandoned? Would be sad if it was stranded the way https://reviews.llvm.org/D31130 was, since I think many people are skipping this check due to diagnostics that are for all intents and purposes "out of their control".

[aaron.ballman]

In D88833#3222331, @estan wrote:

@lebedev.ri @aaron.ballman I think the opt-in sounds good too. Are we waiting for @fiesh to implement this, or is the patch abandoned? Would be sad if it was stranded the way https://reviews.llvm.org/D31130 was, since I think many people are skipping this check due to diagnostics that are for all intents and purposes "out of their control".

My expectation was that @fiesh would be updating the review if they wanted this to land. If they indicate they're no longer interested in working on the patch, then I think it's fine for you to commandeer the patch. But you should give them a chance to speak up in case they're still intending to finish this. @fiesh, are you expecting to work on this further?

[fiesh]

> My expectation was that @fiesh would be updating the review if they wanted this to land. If they indicate they're no longer interested in working on the patch, then I think it's fine for you to commandeer the patch. But you should give them a chance to speak up in case they're still intending to finish this. @fiesh, are you expecting to work on this further?

Oh sorry, this has somehow completely escaped my attention. I'm perfectly fine with somebody else finalizing this patch as I will not be able to do so in the foreseeable future.

Happy new year!

[estan]

Sounds good @aaron.ballman, let's wait for @fiesh.

Though I realize now that the scope of this patch is probably not enough to solve a problem we have in our code base. The check will warn about (for example) things like this:

In a third party lib outside our control:

void f(double out[3]);

In our code:

double out[3];
f(out);

Include paths for the third party lib are added with -isystem.

Am I right that we're still going to get warnings for this with this patch?

Full disclosure. The third party lib is VTK, which is littered with APIs like these.

[estan]

And there he is :) I've never worked on LLVM / clang-tidy but could have a look at making it opt-in. Now we know the status at least.

[aaron.ballman]

In D88833#3222828, @fiesh wrote:

> My expectation was that @fiesh would be updating the review if they wanted this to land. If they indicate they're no longer interested in working on the patch, then I think it's fine for you to commandeer the patch. But you should give them a chance to speak up in case they're still intending to finish this. @fiesh, are you expecting to work on this further?

Oh sorry, this has somehow completely escaped my attention. I'm perfectly fine with somebody else finalizing this patch as I will not be able to do so in the foreseeable future.

Happy new year!

Thank you for getting back to us and getting a start on this patch!

In D88833#3222843, @estan wrote:

And there he is :) I've never worked on LLVM / clang-tidy but could have a look at making it opt-in. Now we know the status at least.

I'd say you're clear to commandeer (the Add Action pulldown has this option). As for making it an option, a lot of other checks have config options, so you can use one of them to model your changes after, like: https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp#L33

[estan]

@aaron.ballman A bit off topic, but I followed https://github.com/llvm/llvm-project/blob/main/README.md#getting-the-source-code-and-building-llvm to build and install llvm-project main branch as follows:

git clone https://github.com/llvm/llvm-project.git
cd llvm-project
cmake -S llvm -B build -G Ninja -DLLVM_ENABLE_PROJECTS=clang-tools-extra -DCMAKE_INSTALL_PREFIX=/home/estan/orexplore/llvm-project-inst
cmake --build build
cmake --install build

But I did not get any clang-tidy in the build or installation directory. The CMake output said "clang-tools-extra project is enabled". Any tips on where I went wrong?

[estan]

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp

65

[aaron.ballman]

In D88833#3223055, @estan wrote:

@aaron.ballman A bit off topic, but I followed https://github.com/llvm/llvm-project/blob/main/README.md#getting-the-source-code-and-building-llvm to build and install llvm-project main branch as follows:

git clone https://github.com/llvm/llvm-project.git
cd llvm-project
cmake -S llvm -B build -G Ninja -DLLVM_ENABLE_PROJECTS=clang-tools-extra -DCMAKE_INSTALL_PREFIX=/home/estan/orexplore/llvm-project-inst
cmake --build build
cmake --install build

But I did not get any clang-tidy in the build or installation directory. The CMake output said "clang-tools-extra project is enabled". Any tips on where I went wrong?

That looks generally correct, though I think you may need to add clang to the list of LLVM projects to enable (clang-tidy relies on clang and I've never tried building clang-tools-extra without clang before). Also, you shouldn't need to do the install step for local development (so my own setup doesn't set the install prefix). FWIW, I do my development on Windows using Visual Studio and its integrated CMake support, so my CMake settings are going to be a bit different than yours anyway. FWIW, I use -DLLVM_TARGETS_TO_BUILD="X86" -DLLVM_ENABLE_IDE=ON -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" -DLLVM_PARALLEL_COMPILE_JOBS=112 -DLLVM_PARALLEL_LINK_JOBS=16

[estan]

Ah yes I had a hunch it had to be "clang;clang-tools-extra" so running a build with that now. Almost done. It crashed at the very end due to OOM while linking, but reduced parallelism now and running again. Thanks.

[estan]

That did the trick, thanks @aaron.ballman.

[aaron.ballman]

In D88833#3223077, @estan wrote:

That did the trick, thanks @aaron.ballman.

Glad that worked!

[carlosgalvezp]

Please note: I have a patch that disables warnings from system macros for all clang-tidy warnings, it would be good to review it so we don't need to implement the same mechanism in all 400+ checks :)
https://reviews.llvm.org/D116378

[estan]

@carlosgalvezp Oh yes, that would be even better.

[estan]

In D88833#3222829, @estan wrote:

Sounds good @aaron.ballman, let's wait for @fiesh.

Though I realize now that the scope of this patch is probably not enough to solve a problem we have in our code base. The check will warn about (for example) things like this:

In a third party lib outside our control:

void f(double out[3]);

In our code:

double out[3];
f(out);

Include paths for the third party lib are added with -isystem.

Am I right that we're still going to get warnings for this with this patch?

Full disclosure. The third party lib is VTK, which is littered with APIs like these.

@aaron.ballman BTW, would you guys be open to a patch that makes the check not warn in cases like the above, or at least an option to make it not warn?

We like the spirit of this check, to make sure we use gsl::array_view/std::span instead of (T *data, int size)/(T*) style interfaces in our own APIs. But we cannot change the (T*) APIs of our third party libraries. Having to litter our code with hundreds of casts is not very nice. It would be great if there was an option to tell the check to ignore all decays that occur in calls to functions in system headers.

[aaron.ballman]

In D88833#3224389, @carlosgalvezp wrote:

Please note: I have a patch that disables warnings from system macros for all clang-tidy warnings, it would be good to review it so we don't need to implement the same mechanism in all 400+ checks :)
https://reviews.llvm.org/D116378

Thank you for pointing that out; that's a far better approach.

In D88833#3224799, @estan wrote:

In D88833#3222829, @estan wrote:

Sounds good @aaron.ballman, let's wait for @fiesh.

Though I realize now that the scope of this patch is probably not enough to solve a problem we have in our code base. The check will warn about (for example) things like this:

In a third party lib outside our control:

void f(double out[3]);

In our code:

double out[3];
f(out);

Include paths for the third party lib are added with -isystem.

Am I right that we're still going to get warnings for this with this patch?

Full disclosure. The third party lib is VTK, which is littered with APIs like these.

Yes, you will still get warnings with that code.

@aaron.ballman BTW, would you guys be open to a patch that makes the check not warn in cases like the above, or at least an option to make it not warn?

We like the spirit of this check, to make sure we use gsl::array_view/std::span instead of (T *data, int size)/(T*) style interfaces in our own APIs. But we cannot change the (T*) APIs of our third party libraries. Having to litter our code with hundreds of casts is not very nice. It would be great if there was an option to tell the check to ignore all decays that occur in calls to functions in system headers.

You can't change the 3rd party APIs because they're system headers, but you can add a cast to the call site to disable the diagnostic. Does that suffice to meet your needs, or do you still think an option is a better approach? https://godbolt.org/z/q41Wdn3xa

[estan]

In D88833#3225842, @aaron.ballman wrote:

In D88833#3224389, @carlosgalvezp wrote:

Please note: I have a patch that disables warnings from system macros for all clang-tidy warnings, it would be good to review it so we don't need to implement the same mechanism in all 400+ checks :)
https://reviews.llvm.org/D116378

Thank you for pointing that out; that's a far better approach.

In D88833#3224799, @estan wrote:

In D88833#3222829, @estan wrote:

Sounds good @aaron.ballman, let's wait for @fiesh.

Though I realize now that the scope of this patch is probably not enough to solve a problem we have in our code base. The check will warn about (for example) things like this:

In a third party lib outside our control:

void f(double out[3]);

In our code:

double out[3];
f(out);

Include paths for the third party lib are added with -isystem.

Am I right that we're still going to get warnings for this with this patch?

Full disclosure. The third party lib is VTK, which is littered with APIs like these.

Yes, you will still get warnings with that code.

@aaron.ballman BTW, would you guys be open to a patch that makes the check not warn in cases like the above, or at least an option to make it not warn?

We like the spirit of this check, to make sure we use gsl::array_view/std::span instead of (T *data, int size)/(T*) style interfaces in our own APIs. But we cannot change the (T*) APIs of our third party libraries. Having to litter our code with hundreds of casts is not very nice. It would be great if there was an option to tell the check to ignore all decays that occur in calls to functions in system headers.

You can't change the 3rd party APIs because they're system headers, but you can add a cast to the call site to disable the diagnostic. Does that suffice to meet your needs, or do you still think an option is a better approach? https://godbolt.org/z/q41Wdn3xa

Well it's not just one call site, but a whole bunch of them, and having to add casts at all these places is a bit cumbersome. I think an option to only warn for cases where you can meaningfully improve the code (by changing the API to use std::span / gsl::array_view) would be good.

[carlosgalvezp]

My patch has now been merged, hope it helps a bit! Reading through the comments I'm not sure it will be enough though. In the cases you mention we've typically wrapped the third-party function into a helper function that adds a NOLINT suppression/cast there, to avoid littering the code at the call site. Would that help? I guess it will still be a pain if you have many different such functions.

[estan]

Yep thanks a lot for that @carlosgalvezp. Getting the system macros sorted out will get rid of many warnings we were getting in our code base (seems the qDebug / qWarning macros in the Qt version we're using violate this check on their own).

Regarding decay in calls to functions in system headers, it's unfortunately many different functions involved, and often they are member functions of some class. VTK is an old slow-moving library and has many API functions like:

void vtkInformation::Get(vtkInformationDoubleVectorKey* key, double* value);
void vtkPiecewiseFunction::GetNodeValue(int index, double val[4]);
void vtkCamera::GetPosition(double[3]);
void vtkCamera::GetFocalPoint(double[3]);
void vtkInteractorObserver::ComputeDisplayToWorld(double x, double y, double z, double worldPt[4]);
...

These were just some examples of functions we use in our code base, but VTK is full of APIs like these.

We also use the Qwt and ed25519 libraries, which have functions like:

void QwtPlotCurve::setSamples(const float* xData, const float* yData, int size);
int ed25519_verify(const unsigned char *signature, const unsigned char *message, size_t message_len, const unsigned char *public_key);
...

I don't think it's uncommon when working with libraries that are not modern C++ to run into a lot of APIs like these, so would be nice to be able to avoid the decay warning when calling into them, without sprinkling casts and wrappers.

[carlosgalvezp]

I see, thanks for the examples! Since you mention modern C++, I can guess you are using std::array in your code. Maybe just pass "buffer.data()" to the third-party functions? I think it's a bit cleaner than casts. If you use C arrays, consider switching to std::array or pass "std::begin(buffer)". That's actually what we do in our code more than wrappers.

My gut feeling is that it feels strange to add this exception, it's still user code after all. If you enable the warning it's because you care about the problems of pointer decay. Warnings typically add some level of "pain" that need to be traded off with the benefit they provide. I'm also afraid this opens the door for more such exceptions, making the logic more complicated and harder to maintain.

That's just my 2 cents, I don't intend to block this patch in any way :) just came in as I saw it was related to my patch.

[estan]

Alright, yea I'll see what using std::array would mean for our code. In many cases we are just using plain arrays currently, and often they are just a way to get data in/out of these APIs (and for the out direction, we quickly go on to saving the data we got out into something else, like a class of our own). I'll think some about it, perhaps you are right and the exception (would make it an option of course) is not worth it.

[estan]

I think we can close this one now that https://reviews.llvm.org/D116378 has landed though, since this revision was originally only about warnings in system macros case which is now solved (?)

I'll commandeer and close, but feel free to re-open if that's the wrong move here.

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp

if (Parents.size() != 1)
       return false;
     E = Parents[0].get<Expr>();
     if (!E)
       return false;
   } while (isa<ImplicitCastExpr>(E));
 
   return InnerMatcher.matches(*E, Finder, Builder);
 }
+
+AST_MATCHER(ImplicitCastExpr, sysSymbolDecayInSysHeader) {
+  auto &SM = Finder->getASTContext().getSourceManager();
+  if (!SM.isInSystemMacro(Node.getBeginLoc()))
+    return false;
+
+  if (const auto *SymbolDeclRef = dyn_cast<DeclRefExpr>(Node.getSubExpr())) {
+    const ValueDecl *SymbolDecl = SymbolDeclRef->getDecl();
+    if (SymbolDecl && SM.isInSystemHeader(SymbolDecl->getLocation()))
+      return true;
+  }
+  return false;
+}
 } // namespace
 
 void ProBoundsArrayToPointerDecayCheck::registerMatchers(MatchFinder *Finder) {
-  // The only allowed array to pointer decay
+  // We only allowed array to pointer decays
   // 1) just before array subscription
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
+  // 4) if it is caused by a system macro
   Finder->addMatcher(
-      traverse(ast_type_traits::TK_AsIs,
-               implicitCastExpr(
-                   unless(hasParent(arraySubscriptExpr())),
-                   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
-                   unless(isInsideOfRangeBeginEndStmt()),
-                   unless(hasSourceExpression(ignoringParens(stringLiteral()))))
-                   .bind("cast")),
+      traverse(
+          ast_type_traits::TK_AsIs,
+              hasCastKind(CK_ArrayToPointerDecay),
+              unless(hasSourceExpression(ignoringParens(stringLiteral()))),
+              unless(hasSourceExpression(ignoringParens(predefinedExpr()))),
+              unless(sysSymbolDecayInSysHeader()))
       this);
 }
 
 void ProBoundsArrayToPointerDecayCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *MatchedCast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast");
-  if (MatchedCast->getCastKind() != CK_ArrayToPointerDecay)
-    return;
 
   diag(MatchedCast->getExprLoc(), "do not implicitly decay an array into a "
                                   "pointer; consider using gsl::array_view or "
                                   "an explicit cast instead");
 }
 
 } // namespace cppcoreguidelines
 } // namespace tidy
 } // namespace clang

clang-tools-extra/test/clang-tidy/checkers/Inputs/cppcoreguidelines-pro-bounds-array-to-pointer-decay/macro.h

+const char SYS_STRING[2]    = "s";
+const int  SYS_INT_ARRAY[2] = {0, 0};
+
+void sys_fn_decay_str(const char *);
+void sys_fn_decay_int_array(const int *);
+
+#define sys_macro_with_pretty_function_string_decay sys_fn_decay_str(__PRETTY_FUNCTION__)
+#define sys_macro_with_sys_string_decay             sys_fn_decay_str(SYS_STRING)
+#define sys_macro_with_sys_int_array_decay          sys_fn_decay_int_array(SYS_INT_ARRAY)
+
+#define user_code_in_sys_macro(expr) expr;
+
+#define decay_to_char_pointer_in_macro(expr) sys_fn_decay_str(expr)
+#define decay_to_int_pointer_in_macro(expr)  sys_fn_decay_int_array(expr)

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

-// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-array-to-pointer-decay %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-array-to-pointer-decay %t -- -- -isystem%S/Inputs/cppcoreguidelines-pro-bounds-array-to-pointer-decay
+
+#include <macro.h>
 #include <stddef.h>
 
 namespace gsl {
 template <class T>
 class array_view {
 public:
   template <class U, size_t N>
   array_view(U (&arr)[N]);
 };
 }
 
 void pointerfun(int *p);
 void arrayfun(int p[]);
 void arrayviewfun(gsl::array_view<int> &p);
 size_t s();
 
 void f() {
   int a[5];
   pointerfun(a);
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
   pointerfun((int *)a); // OK, explicit cast
   arrayfun(a);
   // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not implicitly decay an array into a pointer
 
   pointerfun(a + s() - 10); // Convert to &a[g() - 10];
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not implicitly decay an array into a pointer
 
   gsl::array_view<int> av(a);
   arrayviewfun(av); // OK
 
   int i = a[0];      // OK
   int j = a[(1 + 2)];// OK
   pointerfun(&a[0]); // OK
 
   for (auto &e : a) // OK, iteration internally decays array to pointer
     e = 1;
 }
 
 const char *g() {
   return "clang"; // OK, decay string literal to pointer
 }
 const char *g2() {
     return ("clang"); // OK, ParenExpr hides the literal-pointer decay
 }
 
 void f2(void *const *);
 void bug25362() {
   void *a[2];
   f2(static_cast<void *const*>(a)); // OK, explicit cast
 }
+
+void user_fn_decay_str(const char *);
+void user_fn_decay_int_array(const int *);
+void bug32239() {
+  sys_macro_with_pretty_function_string_decay; // Ok
+  sys_macro_with_sys_string_decay;             // Ok
+  sys_macro_with_sys_int_array_decay;          // Ok
+
+  sys_fn_decay_str(__PRETTY_FUNCTION__); // Ok
+
+  sys_fn_decay_str(SYS_STRING); // Not Ok - should it be ok?
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  sys_fn_decay_int_array(SYS_INT_ARRAY); // Not Ok
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  user_code_in_sys_macro(sys_fn_decay_str(__PRETTY_FUNCTION__)); // Ok
+
+  user_code_in_sys_macro(sys_fn_decay_str(SYS_STRING)); // Not Ok - should it be ok?
+  // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  user_code_in_sys_macro(sys_fn_decay_int_array(SYS_INT_ARRAY)); // Not Ok
+  // CHECK-MESSAGES: :[[@LINE-1]]:49: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  const char UserString[1] = "";
+  decay_to_char_pointer_in_macro(UserString); // Not Ok
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  decay_to_char_pointer_in_macro(__PRETTY_FUNCTION__); // Ok
+
+  int a[5];
+  decay_to_int_pointer_in_macro(a); // Not Ok
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  user_fn_decay_str(__PRETTY_FUNCTION__); // Ok
+
+  user_fn_decay_str(SYS_STRING); // Not Ok - should it be ok?
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  user_fn_decay_int_array(SYS_INT_ARRAY); // Not ok
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+}