[Clang] [Sema] Enable nodiscard warnings for function pointers (#154250)

A call through a function pointer has no associated FunctionDecl, but it
still might have a nodiscard return type. Ensure there is a codepath to
emit the nodiscard warning in this case.

Fixes #142453

clang/docs/ReleaseNotes.rst

@@ -216,8 +216,8 @@ Bug Fixes to Compiler Builtins
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-- ``[[nodiscard]]`` is now respected on Objective-C and Objective-C++ methods.
+- ``[[nodiscard]]`` is now respected on Objective-C and Objective-C++ methods
-  (#GH141504)
+  (#GH141504) and on types returned from indirect calls (#GH142453).
 - Fixes some late parsed attributes, when applied to function definitions, not being parsed
   in function try blocks, and some situations where parsing of the function body
   is skipped, such as error recovery and code completion. (#GH153551)

clang/lib/AST/Expr.cpp

@@ -2776,23 +2776,22 @@ bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc,
   case UserDefinedLiteralClass: {
     // If this is a direct call, get the callee.
     const CallExpr *CE = cast<CallExpr>(this);
-    if (const Decl *FD = CE->getCalleeDecl()) {
+    // If the callee has attribute pure, const, or warn_unused_result, warn
-      // If the callee has attribute pure, const, or warn_unused_result, warn
+    // about it. void foo() { strlen("bar"); } should warn.
-      // about it. void foo() { strlen("bar"); } should warn.
+    // Note: If new cases are added here, DiagnoseUnusedExprResult should be
-      //
+    // updated to match for QoI.
-      // Note: If new cases are added here, DiagnoseUnusedExprResult should be
+    const Decl *FD = CE->getCalleeDecl();
-      // updated to match for QoI.
+    bool PureOrConst =
-      if (CE->hasUnusedResultAttr(Ctx) ||
+        FD && (FD->hasAttr<PureAttr>() || FD->hasAttr<ConstAttr>());
-          FD->hasAttr<PureAttr>() || FD->hasAttr<ConstAttr>()) {
+    if (CE->hasUnusedResultAttr(Ctx) || PureOrConst) {
-        WarnE = this;
+      WarnE = this;
-        Loc = CE->getCallee()->getBeginLoc();
+      Loc = getBeginLoc();
-        R1 = CE->getCallee()->getSourceRange();
+      R1 = getSourceRange();
 
-        if (unsigned NumArgs = CE->getNumArgs())
+      if (unsigned NumArgs = CE->getNumArgs())
-          R2 = SourceRange(CE->getArg(0)->getBeginLoc(),
+        R2 = SourceRange(CE->getArg(0)->getBeginLoc(),
-                           CE->getArg(NumArgs - 1)->getEndLoc());
+                         CE->getArg(NumArgs - 1)->getEndLoc());
-        return true;
+      return true;
-      }
     }
     return false;
   }

clang/test/Sema/c2x-nodiscard.c

@@ -41,6 +41,10 @@ void f2(void) {
   (void)get_s3();
   (void)get_i();
   (void)get_e();
+
+  One; // expected-warning {{expression result unused}}
+  (enum E2)(0); // expected-warning {{expression result unused}}
+  (struct S4){1}; // expected-warning {{expression result unused}}
 }
 
 struct [[nodiscard]] error_info{
@@ -60,3 +64,16 @@ void GH104391() {
 #define M (unsigned int) f3()
   M; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
 }
+
+[[nodiscard]] typedef int NoDInt; // expected-warning {{'[[nodiscard]]' attribute ignored when applied to a typedef}}
+typedef __attribute__((warn_unused)) int WUInt; // expected-warning {{'warn_unused' attribute only applies to structs, unions, and classes}}
+typedef __attribute__((warn_unused_result)) int WURInt;
+NoDInt get_nodint();
+WUInt get_wuint();
+WURInt get_wurint();
+
+void f4(void) {
+  get_nodint(); // no warning because attribute is ignored
+  get_wuint();  // no warning because attribute is ignored
+  get_wurint(); // expected-warning {{ignoring return value of type 'WURInt' declared with 'warn_unused_result' attribute}}
+}

clang/test/SemaCXX/warn-unused-result.cpp

@@ -407,3 +407,88 @@ void doGccThings() {
 }
 
 } // namespace BuildStringOnClangScope
+
+namespace candiscard {
+
+struct [[nodiscard]] NoDiscard {
+  [[nodiscard]] NoDiscard(int);
+  NoDiscard(const char *);
+};
+
+struct [[gnu::warn_unused]] WarnUnused {
+  [[gnu::warn_unused]] WarnUnused(int); // expected-warning {{'gnu::warn_unused' attribute only applies to structs, unions, and classes}}
+  WarnUnused(const char*);
+};
+
+struct [[gnu::warn_unused_result]] WarnUnusedResult {
+  [[gnu::warn_unused_result]] WarnUnusedResult(int);
+  WarnUnusedResult(const char*);
+};
+
+NoDiscard return_nodiscard();
+WarnUnused return_warnunused();
+WarnUnusedResult return_warnunusedresult();
+
+NoDiscard (*p_return_nodiscard)();
+WarnUnused (*p_return_warnunused)();
+WarnUnusedResult (*p_return_warnunusedresult)();
+
+NoDiscard (*(*pp_return_nodiscard)())();
+WarnUnused (*(*pp_return_warnunused)())();
+WarnUnusedResult (*(*pp_return_warnunusedresult)())();
+
+template <class T> T from_a_template();
+
+void test() {
+  // Unused but named variables
+  NoDiscard unused_variable1(1);         // no warning
+  NoDiscard unused_variable2("");        // no warning
+  WarnUnused unused_variable3(1);        // no warning
+  WarnUnused unused_variable4("");       // no warning
+  WarnUnusedResult unused_variable5(1);  // no warning
+  WarnUnusedResult unused_variable6(""); // no warning
+
+  // Constructor return values
+  NoDiscard(1);         // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+  NoDiscard("");        // expected-warning {{ignoring temporary of type 'NoDiscard' declared with 'nodiscard' attribute}}
+  WarnUnused(1);        // expected-warning {{expression result unused}}
+  WarnUnused("");       // expected-warning {{expression result unused}}
+  WarnUnusedResult(1);  // expected-warning {{ignoring temporary created by a constructor declared with 'gnu::warn_unused_result' attribute}}
+  WarnUnusedResult(""); // expected-warning {{ignoring temporary of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
+
+  NoDiscard{1};         // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+  NoDiscard{""};        // expected-warning {{ignoring temporary of type 'NoDiscard' declared with 'nodiscard' attribute}}
+  WarnUnused{1};        // expected-warning {{expression result unused}}
+  WarnUnused{""};       // expected-warning {{expression result unused}}
+  WarnUnusedResult{1};  // expected-warning {{ignoring temporary created by a constructor declared with 'gnu::warn_unused_result' attribute}}
+  WarnUnusedResult{""}; // expected-warning {{ignoring temporary of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
+
+  static_cast<NoDiscard>(1);         // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+  static_cast<NoDiscard>("");        // expected-warning {{ignoring temporary of type 'NoDiscard' declared with 'nodiscard' attribute}}
+  static_cast<WarnUnused>(1);        // expected-warning {{expression result unused}}
+  static_cast<WarnUnused>("");       // expected-warning {{expression result unused}}
+  static_cast<WarnUnusedResult>(1);  // expected-warning {{ignoring temporary created by a constructor declared with 'gnu::warn_unused_result' attribute}}
+  static_cast<WarnUnusedResult>(""); // expected-warning {{ignoring temporary of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
+
+  // Function return values
+  return_nodiscard(); // expected-warning {{ignoring return value of type 'NoDiscard' declared with 'nodiscard' attribute}}
+  return_warnunused(); // no warning
+  return_warnunusedresult(); // expected-warning {{ignoring return value of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
+
+  // Function pointer return values
+  p_return_nodiscard(); // expected-warning {{ignoring return value of type 'NoDiscard' declared with 'nodiscard' attribute}}
+  p_return_warnunused(); // no warning
+  p_return_warnunusedresult(); // expected-warning {{ignoring return value of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
+
+  // Function pointer expression return values
+  pp_return_nodiscard()(); // expected-warning {{ignoring return value of type 'NoDiscard' declared with 'nodiscard' attribute}}
+  pp_return_warnunused()(); // no warning
+  pp_return_warnunusedresult()(); // expected-warning {{ignoring return value of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
+
+  // From a template
+  from_a_template<NoDiscard>(); // expected-warning {{ignoring return value of type 'NoDiscard' declared with 'nodiscard' attribute}}
+  from_a_template<WarnUnused>(); // no warning
+  from_a_template<WarnUnusedResult>(); // expected-warning {{ignoring return value of type 'WarnUnusedResult' declared with 'gnu::warn_unused_result' attribute}}
+}
+
+} // namespace candiscard

clang/test/SemaObjC/attr-nodiscard.m

@@ -4,6 +4,9 @@ struct [[nodiscard]] expected {};
 
 typedef struct expected E;
 
+[[nodiscard]] typedef int NI; // expected-warning {{'[[nodiscard]]' attribute ignored when applied to a typedef}}
+typedef __attribute__((warn_unused_result)) int WUR;
+
 @interface INTF
 - (int) a [[nodiscard]];
 + (int) b [[nodiscard]];
@@ -12,6 +15,8 @@ typedef struct expected E;
 - (E) e;
 + (E) f;
 - (void) g [[nodiscard]]; // expected-warning {{attribute 'nodiscard' cannot be applied to Objective-C method without return value}}
+- (NI) h;
+- (WUR) i;
 @end
 
 void foo(INTF *a) {
@@ -21,5 +26,7 @@ void foo(INTF *a) {
   [INTF d]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}}
   [a e]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}}
   [INTF f]; // expected-warning {{ignoring return value of type 'expected' declared with 'nodiscard' attribute}}
-  [a g];
+  [a g]; // no warning because g returns void
+  [a h]; // no warning because attribute is ignored when applied to a typedef
+  [a i]; // expected-warning {{ignoring return value of type 'WUR' declared with 'warn_unused_result' attribute}}
 }

clang/test/SemaObjCXX/attr-nodiscard.mm

@@ -5,6 +5,9 @@ struct [[nodiscard]] expected {};
 
 using E = expected<int>;
 
+using NI [[nodiscard]] = int; // expected-warning {{'[[nodiscard]]' attribute ignored when applied to a typedef}}
+using WURI [[clang::warn_unused_result]] = int;
+
 @interface INTF
 - (int) a [[nodiscard]];
 + (int) b [[nodiscard]];
@@ -13,6 +16,8 @@ using E = expected<int>;
 - (E) e;
 + (E) f;
 - (void) g [[nodiscard]]; // expected-warning {{attribute 'nodiscard' cannot be applied to Objective-C method without return value}}
+- (NI) h;
+- (WURI) i;
 @end
 
 void foo(INTF *a) {
@@ -22,5 +27,7 @@ void foo(INTF *a) {
   [INTF d]; // expected-warning {{ignoring return value of type 'expected<int>' declared with 'nodiscard' attribute}}
   [a e]; // expected-warning {{ignoring return value of type 'expected<int>' declared with 'nodiscard' attribute}}
   [INTF f]; // expected-warning {{ignoring return value of type 'expected<int>' declared with 'nodiscard' attribute}}
-  [a g];
+  [a g]; // no warning because g returns void
+  [a h]; // no warning because attribute is ignored
+  [a i]; // expected-warning {{ignoring return value of type 'WURI' declared with 'clang::warn_unused_result' attribute}}
 }