[clang][Sema] Provide source range to several Wunused warnings

[hazohelet]

Summary

When the diagnosed function/variable is a template specialization, the source range covers the specialization arguments.
e.g.

warning: unused function 'func<int>' [-Wunused-function]
template <> int func<int> () {}
                ^~~~~~~~~

This comes in line with the printed text in the warning message. In the above case, func<int>

[tbaeder]

clang/test/Misc/diag-unused-source-ranges.cpp

2

Should be %clang_cc1 I think.

[tbaeder]

Precommit-CI also found some problems on Windows

[tbaeder]

clang/test/Misc/diag-unused-source-ranges.h

1 ↗

(On Diff #530481)

Moving this file to Inputs/ would make sense I think.

[hazohelet]

In D152707#4413899, @tbaeder wrote:

Precommit-CI also found some problems on Windows

It's a littile surprising to me that clang outputs different diagnostics on windows. I suspect it might be related to the clang driver, so I'll try %clang_cc1 as you suggested, and if it still fails on windows, I'll split the test into separate files to have completely same inputs as already-working tests in test/Sema

[hazohelet]

• Change %clang to %clang_cc1 -fcxx-exceptions in the test

[aaron.ballman]

clang/lib/Sema/Sema.cpp

1350

Does DiagD->getSourceRange() not give you the same results?

1384

[hazohelet]

clang/lib/Sema/Sema.cpp

1350

FunctionDecl::getSourceRange also covers the return type and the function body if it exists, so we cannot use it here.

[aaron.ballman]

LG aside from the few suggested changes and moving the header to the Inputs directory.

clang/lib/Sema/Sema.cpp

1350

Ah, okay, that makes sense. Thank you!

<not your problem>
I think we should consider adding SourceRange accessors for various interesting ranges like this so that we don't need to use ad hoc solutions in various places.
</not your problem>

clang/test/Misc/diag-unused-source-ranges.h

1 ↗

(On Diff #530481)

+1

[hazohelet]

Address review comments

• Move the test header to Inputs folder
• Remove explicit constructor call of SourceRange

[hazohelet]

Thanks for the review.
I'll check the pre-commit CI result before landing this just in case.

[hazohelet]

clang/lib/Sema/Sema.cpp

1386

It looks like VarTemplateSpecializationDecl::getTemplateArgsInfo() could be null, so I'll add another check for that to avoid regression.

(This seems weird to me because IIRC there's no template type deduction for variable templates and thus every specialization for variable templates has explicit template arguments provided)

[hazohelet]

Added an additional null check that I mentioned earlier

[aaron.ballman]

LGTM!

clang/lib/Sema/Sema.cpp

for (UnusedFileScopedDeclsType::iterator
         continue;
 
       if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(*I)) {
         const FunctionDecl *DiagD;
         if (!FD->hasBody(DiagD))
           DiagD = FD;
         if (DiagD->isDeleted())
           continue; // Deleted functions are supposed to be unused.
+        SourceRange DiagRange = DiagD->getLocation();
+        if (const ASTTemplateArgumentListInfo *ASTTAL =
+                DiagD->getTemplateSpecializationArgsAsWritten())
+          DiagRange.setEnd(ASTTAL->RAngleLoc);
         if (DiagD->isReferenced()) {
           if (isa<CXXMethodDecl>(DiagD))
             Diag(DiagD->getLocation(), diag::warn_unneeded_member_function)
-                << DiagD;
+                << DiagD << DiagRange;
           else {
             if (FD->getStorageClass() == SC_Static &&
                 !FD->isInlineSpecified() &&
                 !SourceMgr.isInMainFile(
                    SourceMgr.getExpansionLoc(FD->getLocation())))
               Diag(DiagD->getLocation(),
                    diag::warn_unneeded_static_internal_decl)
-                  << DiagD;
+                  << DiagD << DiagRange;
             else
               Diag(DiagD->getLocation(), diag::warn_unneeded_internal_decl)
-                  << /*function*/ 0 << DiagD;
+                  << /*function*/ 0 << DiagD << DiagRange;
           }
         } else {
           if (FD->getDescribedFunctionTemplate())
             Diag(DiagD->getLocation(), diag::warn_unused_template)
-                << /*function*/ 0 << DiagD;
+                << /*function*/ 0 << DiagD << DiagRange;
           else
             Diag(DiagD->getLocation(), isa<CXXMethodDecl>(DiagD)
                                            ? diag::warn_unused_member_function
                                            : diag::warn_unused_function)
-                << DiagD;
+                << DiagD << DiagRange;
         }
       } else {
         const VarDecl *DiagD = cast<VarDecl>(*I)->getDefinition();
         if (!DiagD)
           DiagD = cast<VarDecl>(*I);
+        SourceRange DiagRange = DiagD->getLocation();
+        if (const auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(DiagD)) {
+          if (const ASTTemplateArgumentListInfo *ASTTAL =
+                  VTSD->getTemplateArgsInfo())
+            DiagRange.setEnd(ASTTAL->RAngleLoc);
+        }
         if (DiagD->isReferenced()) {
           Diag(DiagD->getLocation(), diag::warn_unneeded_internal_decl)
-              << /*variable*/ 1 << DiagD;
+              << /*variable*/ 1 << DiagD << DiagRange;
         } else if (DiagD->getDescribedVarTemplate()) {
           Diag(DiagD->getLocation(), diag::warn_unused_template)
-              << /*variable*/ 1 << DiagD;
+              << /*variable*/ 1 << DiagD << DiagRange;
         } else if (DiagD->getType().isConstQualified()) {
           const SourceManager &SM = SourceMgr;
           if (SM.getMainFileID() != SM.getFileID(DiagD->getLocation()) ||
               !PP.getLangOpts().IsHeaderFile)
             Diag(DiagD->getLocation(), diag::warn_unused_const_variable)
-                << DiagD;
+                << DiagD << DiagRange;
         } else {
-          Diag(DiagD->getLocation(), diag::warn_unused_variable) << DiagD;
+          Diag(DiagD->getLocation(), diag::warn_unused_variable)
+              << DiagD << DiagRange;
         }
       }
     }
 
     emitAndClearUnusedLocalTypedefWarnings();
   }
 
   if (!Diags.isIgnored(diag::warn_unused_private_field, SourceLocation())) {

clang/lib/Sema/SemaDecl.cpp

void Sema::DiagnoseUnusedDecl(const NamedDecl *D, DiagReceiverTy DiagReceiver) {
   unsigned DiagID;
   if (isa<VarDecl>(D) && cast<VarDecl>(D)->isExceptionVariable())
     DiagID = diag::warn_unused_exception_param;
   else if (isa<LabelDecl>(D))
     DiagID = diag::warn_unused_label;
   else
     DiagID = diag::warn_unused_variable;
 
-  DiagReceiver(D->getLocation(), PDiag(DiagID) << D << Hint);
+  SourceLocation DiagLoc = D->getLocation();
+  DiagReceiver(DiagLoc, PDiag(DiagID) << D << Hint << SourceRange(DiagLoc));
 }
 
 void Sema::DiagnoseUnusedButSetDecl(const VarDecl *VD,
                                     DiagReceiverTy DiagReceiver) {
   // If it's not referenced, it can't be set. If it has the Cleanup attribute,
   // it's not really unused.
   if (!VD->isReferenced() || !VD->getDeclName() || VD->hasAttr<UnusedAttr>() ||
       VD->hasAttr<CleanupAttr>())

clang/test/Misc/Inputs/diag-unused-source-ranges.h

+static void thing(void) {}

clang/test/Misc/diag-unused-source-ranges.cpp

+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -Wunused -Wunused-template -Wunused-exception-parameter -Wunused-member-function -fdiagnostics-print-source-range-info %s 2>&1 | FileCheck %s --strict-whitespace
+#include "Inputs/diag-unused-source-ranges.h"
+
+#define CAT(a, b) a ## b
+
+// CHECK:      :{55:15-55:20}: warning: unused exception parameter 'param'
+// CHECK-NEXT:   catch (int &param) {}
+// CHECK-NEXT:               ^~~~~{{$}}
+
+// CHECK:      :{53:7-53:12}: warning: unused variable 'local'
+// CHECK-NEXT:   int local = 0;
+// CHECK-NEXT:       ^~~~~{{$}}
+
+// CHECK:      In file included from
+// CHECK-NEXT: :{1:13-1:18}: warning: 'static' function 'thing' declared in header file should be declared 'static inline'
+// CHECK-NEXT:   static void thing(void) {}
+// CHECK-NEXT:               ^~~~~{{$}}
+
+namespace {
+class A {
+  // CHECK:      :{[[@LINE+3]]:10-[[@LINE+3]]:14}: warning: member function 'func' is not needed
+  // CHECK-NEXT:   void func() {}
+  // CHECK-NEXT:        ^~~~{{$}}
+    void func() {}
+  // CHECK:      :{[[@LINE+3]]:32-[[@LINE+3]]:37}: warning: unused function template
+  // CHECK-NEXT:   void templ(T) {}
+  // CHECK-NEXT:        ^~~~~{{$}}
+    template <typename T> void templ(T) {}
+  // CHECK:      :{[[@LINE+3]]:22-[[@LINE+3]]:32}: warning: member function 'templ<int>' is not needed
+  // CHECK-NEXT:   void templ<int>(int) {}
+  // CHECK-NEXT:        ^~~~~~~~~~{{$}}
+    template <> void templ<int>(int) {}
+  // CHECK:      :{[[@LINE+3]]:22-[[@LINE+3]]:27}: warning: member function 'templ<float>' is not needed
+  // CHECK-NEXT:   void templ(float) {}
+  // CHECK-NEXT:        ^~~~~{{$}}
+    template <> void templ(float) {}
+
+  // CHECK:      :{[[@LINE+4]]:10-[[@LINE+4]]:13}: warning: unused function template
+  // CHECK-NEXT:   void foo() {
+  // CHECK-NEXT:        ^~~{{$}}
+    template <typename T>
+    void foo() {
+      func();
+      templ(0);
+      templ(0.0f);
+      templ(0.0);
+    }
+};
+// CHECK:      :{[[@LINE+3]]:12-[[@LINE+3]]:23}: warning: unused function 'unused_func'
+// CHECK-NEXT:   static int unused_func(int aaa, char bbb) {
+// CHECK-NEXT:              ^~~~~~~~~~~{{$}}
+static int unused_func(int aaa, char bbb) {
+  int local = 0;
+  try{}
+  catch (int &param) {}
+  return 0;
+}
+
+// CHECK:      :{[[@LINE+4]]:6-[[@LINE+4]]:16}: warning: unused function template
+// CHECK-NEXT:   auto arrow_decl(T a, T b) ->
+// CHECK-NEXT:        ^~~~~~~~~~{{$}}
+template <typename T>
+auto arrow_decl(T a, T b) -> decltype(a + b) { thing(); return a + b; }
+
+// CHECK:      :{[[@LINE+4]]:6-[[@LINE+4]]:21}: warning: unused function 'arrow_decl<int>'
+// CHECK-NEXT:   auto arrow_decl<int>(int a, int b) ->
+// CHECK-NEXT:        ^~~~~~~~~~~~~~~{{$}}
+template <>
+auto arrow_decl<int>(int a, int b) -> int { return a + b; }
+
+
+// CHECK:      :{[[@LINE+4]]:10-[[@LINE+4]]:20}: warning: unused function template
+// CHECK-NEXT:   static T func_templ(int bbb, T ccc) {
+// CHECK-NEXT:            ^~~~~~~~~~{{$}}
+template <typename T>
+static T func_templ(int bbb, T ccc) {
+  return ccc;
+}
+
+// CHECK:      :{[[@LINE+3]]:17-[[@LINE+3]]:32}: warning: function 'func_templ<int>' is not needed
+// CHECK-NEXT:   int func_templ<int>(int bbb, int ccc) {
+// CHECK-NEXT:       ^~~~~~~~~~~~~~~{{$}}
+template <> int func_templ<int>(int bbb, int ccc) {
+  return bbb;
+}
+
+// CHECK:      :{[[@LINE+3]]:35-[[@LINE+3]]:47}: warning: unused function template
+// CHECK-NEXT:   static void never_called() {
+// CHECK-NEXT:               ^~~~~~~~~~~~{{$}}
+template <typename T> static void never_called() {
+  func_templ<int>(0, 0);
+}
+
+// CHECK:      :{[[@LINE+3]]:22-[[@LINE+3]]:31}: warning: unused variable template
+// CHECK-NEXT:   int var_templ =
+// CHECK-NEXT:       ^~~~~~~~~{{$}}
+template <int n> int var_templ = n * var_templ<n-1>;
+// CHECK:      :{[[@LINE+3]]:17-[[@LINE+3]]:29}: warning: variable 'var_templ<0>' is not needed
+// CHECK-NEXT:   int var_templ<0> =
+// CHECK-NEXT:       ^~~~~~~~~~~~{{$}}
+template <> int var_templ<0> = 1;
+struct {
+// CHECK:      :{[[@LINE+3]]:8-[[@LINE+3]]:11}: warning: unused member function 'fun'
+// CHECK-NEXT:   void fun() {}
+// CHECK-NEXT:        ^~~{{$}}
+  void fun() {}
+// CHECK:      :{[[@LINE+3]]:3-[[@LINE+3]]:8}: warning: unused variable 'var_x'
+// CHECK-NEXT:   } var_x;
+// CHECK-NEXT:     ^~~~~{{$}}
+} var_x;
+
+// CHECK:      :{[[@LINE+5]]:12-[[@LINE+6]]:12}: warning: unused variable 'new_line'
+// CHECK-NEXT:   static int CAT(new_,
+// CHECK-NEXT:              ^~~~~~~~~{{$}}
+// CHECK-NEXT:         line) =
+// CHECK-NEXT:         ~~~~~{{$}}
+static int CAT(new_,
+      line) = sizeof(var_templ<0>);
+}
+
+// CHECK:      :{[[@LINE+3]]:15-[[@LINE+3]]:27}: warning: unused variable 'const_unused'
+// CHECK-NEXT:   constexpr int const_unused = 1
+// CHECK-NEXT:                 ^~~~~~~~~~~~{{$}}
+constexpr int const_unused = 1;