[clang-tidy] modernize-use-nullptr matches "NULL" in templates

[tkoeppe]

Make modernize-use-nullptr matcher also match "NULL", but not "0", when it appears on a substituted type of a template specialization.

Previously, any matches on a substituted type were excluded, but this meant that a situation like the following is not diagnosed:

template <typename T>
struct X {
  T val;
  X() { val = NULL; }  // should diagnose
};

When the user says NULL, we expect that the destination type is always meant to be a pointer type, so this should be converted to nullptr. By contrast, we do not propose changing a literal 0 in that case, which appears as initializers of both pointer and integer specializations in reasonable real code. (If NULL is used erroneously in such a situation, it should be changed to 0 or {}.)

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Thomas Köppe (tkoeppe)

Changes

Make modernize-use-nullptr matcher also match "NULL", but not "0", when it appears on a substituted type of a template specialization.

Previously, any matches on a substituted type were excluded, but this meant that a situation like the following is not diagnosed:

template <typename T>
struct X {
  T val;
  X() { val = NULL; }  // should diagnose
};

When the user says NULL, we expect that the destination type is always meant to be a pointer type, so this should be converted to nullptr. By contrast, we do not propose changing a literal 0 in that case, which appears as initializers of both pointer and integer specializations in reasonable real code. (If NULL is used erroneously in such a situation, it should be changed to 0 or {}.)

---

Full diff: https://github.com/llvm/llvm-project/pull/109169.diff

2 Files Affected:

• (modified) clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp (+3-1)
• (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-nullptr.cpp (+8)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
index 6a003a347badac..b2921690863b84 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
@@ -38,7 +38,9 @@ AST_MATCHER(Type, sugaredNullptrType) {
 StatementMatcher makeCastSequenceMatcher(llvm::ArrayRef<StringRef> NameList) {
   auto ImplicitCastToNull = implicitCastExpr(
       anyOf(hasCastKind(CK_NullToPointer), hasCastKind(CK_NullToMemberPointer)),
-      unless(hasImplicitDestinationType(qualType(substTemplateTypeParmType()))),
+      anyOf(hasSourceExpression(gnuNullExpr()),
+            unless(hasImplicitDestinationType(
+                qualType(substTemplateTypeParmType())))),
       unless(hasSourceExpression(hasType(sugaredNullptrType()))),
       unless(hasImplicitDestinationType(
           qualType(matchers::matchesAnyListedTypeName(NameList)))));
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-nullptr.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-nullptr.cpp
index 7bc0925136aa86..1807d6bd56125b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-nullptr.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-nullptr.cpp
@@ -84,6 +84,14 @@ void test_macro_expansion4() {
 #undef MY_NULL
 }
 
+template <typename T> struct pear {
+  T x;
+};
+void test_templated() {
+  pear<int*> p = { NULL };
+  dummy(p.x);
+}
+
 #define IS_EQ(x, y) if (x != y) return;
 void test_macro_args() {
   int i = 0;

[EugeneZelenko]

Please mention changes in Release Notes.

[tkoeppe]

Done!

[tkoeppe]

@EugeneZelenko Could you please have another look?

[EugeneZelenko]

I mostly check documentation and minor code issues. Please wait for at least one of active developers whom I added to reviewers list.

[HerrCai0907]

LGTM

[tkoeppe]

What happens next, do we need more approvals? @EugeneZelenko?

[EugeneZelenko]

What happens next, do we need more approvals? @EugeneZelenko?

I think two approvals from active developers are enough.