Support warn_unused_result on typedefs

While it's not as robust as using the attribute on enums/classes (the type information may be lost through a function pointer, a declaration or use of the underlying type without using the typedef, etc) but I think there's still value in being able to attribute a typedef and have all return types written with that typedef pick up the warn_unused_result behavior. Specifically I'd like to be able to annotate LLVMErrorRef (a wrapper for llvm::Error used in the C API - the underlying type is a raw pointer, so it can't be attributed itself) to reduce the chance of unhandled errors. Differential Revision: https://reviews.llvm.org/D102122
llvm · Jun 2, 2022 · cb08f4a · cb08f4a
1 parent dbead23
commit cb08f4a
Show file tree

Hide file tree

Showing 11 changed files with 61 additions and 5 deletions.
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
@@ -2944,7 +2944,7 @@ def WarnUnusedResult : InheritableAttr {
                    C2x<"", "nodiscard", 201904>,
                    CXX11<"clang", "warn_unused_result">,
                    GCC<"warn_unused_result">];
-  let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike]>;
+  let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike, TypedefName]>;
   let Args = [StringArgument<"Message", 1>];
   let Documentation = [WarnUnusedResultsDocs];
   let AdditionalMembers = [{

diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -146,6 +146,7 @@ class AttributeCommonInfo {
   bool isMicrosoftAttribute() const { return SyntaxUsed == AS_Microsoft; }
 
   bool isGNUScope() const;
+  bool isClangScope() const;
 
   bool isAlignasAttribute() const {
     // FIXME: Use a better mechanism to determine this.
@@ -164,6 +165,8 @@ class AttributeCommonInfo {
     return isCXX11Attribute() || isC2xAttribute();
   }
 
+  bool isGNUAttribute() const { return SyntaxUsed == AS_GNU; }
+
   bool isKeywordAttribute() const {
     return SyntaxUsed == AS_Keyword || SyntaxUsed == AS_ContextSensitiveKeyword;
   }

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8734,6 +8734,10 @@ def warn_unused_result : Warning<
 def warn_unused_result_msg : Warning<
   "ignoring return value of function declared with %0 attribute: %1">,
   InGroup<UnusedResult>;
+def warn_unused_result_typedef_unsupported_spelling : Warning<
+  "'[[%select{nodiscard|gnu::warn_unused_result}0]]' attribute ignored when "
+  "applied to a typedef; consider using '__attribute__((warn_unused_result))' "
+  "or '[[clang::warn_unused_result]]' instead">, InGroup<IgnoredAttributes>;
 def warn_unused_volatile : Warning<
   "expression result unused; assign into a variable to force a volatile load">,
   InGroup<DiagGroup<"unused-volatile-lvalue">>;

diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
@@ -1522,6 +1522,11 @@ const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
     if (const auto *A = TD->getAttr<WarnUnusedResultAttr>())
       return A;
 
+  for (const auto *TD = getCallReturnType(Ctx)->getAs<TypedefType>(); TD;
+       TD = TD->desugar()->getAs<TypedefType>())
+    if (const auto *A = TD->getDecl()->getAttr<WarnUnusedResultAttr>())
+      return A;
+
   // Otherwise, see if the callee is marked nodiscard and return that attribute
   // instead.
   const Decl *D = getCalleeDecl();

diff --git a/clang/lib/Basic/Attributes.cpp b/clang/lib/Basic/Attributes.cpp
@@ -85,6 +85,10 @@ bool AttributeCommonInfo::isGNUScope() const {
   return ScopeName && (ScopeName->isStr("gnu") || ScopeName->isStr("__gnu__"));
 }
 
+bool AttributeCommonInfo::isClangScope() const {
+  return ScopeName && (ScopeName->isStr("clang") || ScopeName->isStr("_Clang"));
+}
+
 #include "clang/Sema/AttrParsedAttrKinds.inc"
 
 static SmallString<64> normalizeName(const IdentifierInfo *Name,

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3156,6 +3156,14 @@ static void handleWarnUnusedResult(Sema &S, Decl *D, const ParsedAttr &AL) {
       S.Diag(AL.getLoc(), diag::ext_cxx17_attr) << AL;
   }
 
+  if ((!AL.isGNUAttribute() &&
+       !(AL.isStandardAttributeSyntax() && AL.isClangScope())) &&
+      isa<TypedefNameDecl>(D)) {
+    S.Diag(AL.getLoc(), diag::warn_unused_result_typedef_unsupported_spelling)
+        << AL.isGNUScope();
+    return;
+  }
+
   D->addAttr(::new (S.Context) WarnUnusedResultAttr(S.Context, AL, Str));
 }
 

diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp
@@ -7,4 +7,4 @@ struct [[nodiscard("Wrong")]] S3 {};
 [[nodiscard]] int f();
 enum [[nodiscard]] E {};
 
-namespace [[nodiscard]] N {} // expected-warning {{'nodiscard' attribute only applies to Objective-C methods, enums, structs, unions, classes, functions, and function pointers}}
+namespace [[nodiscard]] N {} // expected-warning {{'nodiscard' attribute only applies to Objective-C methods, enums, structs, unions, classes, functions, function pointers, and typedefs}}
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -185,7 +185,7 @@
 // CHECK-NEXT: VecReturn (SubjectMatchRule_record)
 // CHECK-NEXT: VecTypeHint (SubjectMatchRule_function)
 // CHECK-NEXT: WarnUnused (SubjectMatchRule_record)
-// CHECK-NEXT: WarnUnusedResult (SubjectMatchRule_objc_method, SubjectMatchRule_enum, SubjectMatchRule_record, SubjectMatchRule_hasType_functionType)
+// CHECK-NEXT: WarnUnusedResult (SubjectMatchRule_objc_method, SubjectMatchRule_enum, SubjectMatchRule_record, SubjectMatchRule_hasType_functionType, SubjectMatchRule_type_alias)
 // CHECK-NEXT: Weak (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)
 // CHECK-NEXT: WeakRef (SubjectMatchRule_variable, SubjectMatchRule_function)
 // CHECK-NEXT: WebAssemblyExportName (SubjectMatchRule_function)

diff --git a/clang/test/Sema/c2x-nodiscard.c b/clang/test/Sema/c2x-nodiscard.c
@@ -15,7 +15,7 @@ struct S3 get_s3(void);
 [[nodiscard]] int f1(void);
 enum [[nodiscard]] E1 { One };
 
-[[nodiscard]] int i; // expected-warning {{'nodiscard' attribute only applies to Objective-C methods, enums, structs, unions, classes, functions, and function pointers}}
+[[nodiscard]] int i; // expected-warning {{'nodiscard' attribute only applies to Objective-C methods, enums, structs, unions, classes, functions, function pointers, and typedefs}}
 
 struct [[nodiscard]] S4 {
   int i;

diff --git a/clang/test/Sema/unused-expr.c b/clang/test/Sema/unused-expr.c
@@ -96,7 +96,7 @@ int t6(void) {
   return 0;
 }
 
-int t7 __attribute__ ((warn_unused_result)); // expected-warning {{'warn_unused_result' attribute only applies to Objective-C methods, enums, structs, unions, classes, functions, and function pointers}}
+int t7 __attribute__ ((warn_unused_result)); // expected-warning {{'warn_unused_result' attribute only applies to Objective-C methods, enums, structs, unions, classes, functions, function pointers, and typedefs}}
 
 // PR4010
 int (*fn4)(void) __attribute__ ((warn_unused_result));

diff --git a/clang/test/SemaCXX/warn-unused-result.cpp b/clang/test/SemaCXX/warn-unused-result.cpp
@@ -254,3 +254,35 @@ __attribute__((warn_unused_result)) bool (*h)();
 
 void i([[nodiscard]] bool (*fp)()); // expected-warning {{'nodiscard' attribute only applies to functions, classes, or enumerations}}
 }
+
+namespace unused_typedef_result {
+[[clang::warn_unused_result]] typedef void *a;
+typedef a indirect;
+a af1();
+indirect indirectf1();
+void af2() {
+  af1(); // expected-warning {{ignoring return value}}
+  void *(*a1)();
+  a1(); // no warning
+  a (*a2)();
+  a2(); // expected-warning {{ignoring return value}}
+  indirectf1(); // expected-warning {{ignoring return value}}
+}
+[[nodiscard]] typedef void *b1; // expected-warning {{'[[nodiscard]]' attribute ignored when applied to a typedef; consider using '__attribute__((warn_unused_result))' or '[[clang::warn_unused_result]]' instead}}
+[[gnu::warn_unused_result]] typedef void *b2; // expected-warning {{'[[gnu::warn_unused_result]]' attribute ignored when applied to a typedef; consider using '__attribute__((warn_unused_result))' or '[[clang::warn_unused_result]]' instead}}
+b1 b1f1();
+b2 b2f1();
+void bf2() {
+  b1f1(); // no warning
+  b2f1(); // no warning
+}
+__attribute__((warn_unused_result)) typedef void *c;
+c cf1();
+void cf2() {
+  cf1(); // expected-warning {{ignoring return value}}
+  void *(*c1)();
+  c1();
+  c (*c2)();
+  c2(); // expected-warning {{ignoring return value}}
+}
+}