cjdb created this revision.
cjdb added a reviewer: aaron.ballman.
Herald added a subscriber: jdoerfert.
Herald added a project: All.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Although the default behaviour for C and C++ is to not diagnose ignored function
calls, we can make this the default by using `#pragma clang attribute
push([[nodiscard]], apply_to = function)`. When we have a function that can
have its value discarded, we can use `[[clang::discardable]]` to indicate that
the ``[[nodiscard]]`` attribute should be ignored.
`[[clang::discardable]]` can be placed anywhere `[[nodiscard]]` is allowed,
but the presence of either is prioritised when it's applied to a callable.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D129835
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/Expr.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/lib/AST/Expr.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
clang/test/SemaCXX/discardable.cpp
Index: clang/test/SemaCXX/discardable.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/discardable.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunknown-attributes -verify -std=c++17 %s
+
+[[clang::discardable]] int without_nodiscard();
+
+[[nodiscard, clang::discardable]] int no_warn_fundamental();
+
+struct naked {};
+[[nodiscard, clang::discardable]] naked no_warn_class_type();
+[[nodiscard, clang::discardable]] naked* no_warn_pointer();
+[[nodiscard, clang::discardable]] naked& no_warn_reference();
+
+struct [[nodiscard]] nodiscard_type {};
+[[clang::discardable]] nodiscard_type no_warn_function_override();
+
+struct [[clang::discardable]] discardable_type {};
+discardable_type no_warn_discardable_type();
+[[nodiscard]] discardable_type warns_function_override();
+
+struct [[nodiscard, clang::discardable]] discardable_type2 {};
+discardable_type2 no_warn_discardable_type2();
+
+struct discardable_members {
+ [[nodiscard, clang::discardable]] discardable_members(int);
+ [[nodiscard, clang::discardable]] int f() const;
+};
+
+void test_expression_statements() {
+ without_nodiscard();
+
+ no_warn_fundamental();
+
+ no_warn_class_type();
+ no_warn_pointer();
+ no_warn_reference();
+
+ no_warn_function_override();
+
+ no_warn_discardable_type();
+ warns_function_override(); // expected-warning{{ignoring return value}}
+
+ no_warn_discardable_type2();
+
+ discardable_members(0);
+ discardable_members(0).f();
+}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===================================================================
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -60,6 +60,7 @@
// CHECK-NEXT: DiagnoseAsBuiltin (SubjectMatchRule_function)
// CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
// CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
+// CHECK-NEXT: DisableWarnUnusedResult (SubjectMatchRule_objc_method, SubjectMatchRule_enum, SubjectMatchRule_record, SubjectMatchRule_hasType_functionType, SubjectMatchRule_type_alias)
// CHECK-NEXT: EnableIf (SubjectMatchRule_function)
// CHECK-NEXT: EnforceTCB (SubjectMatchRule_function, SubjectMatchRule_objc_method)
// CHECK-NEXT: EnforceTCBLeaf (SubjectMatchRule_function, SubjectMatchRule_objc_method)
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -282,8 +282,12 @@
if (E->getType()->isVoidType())
return;
- if (DiagnoseNoDiscard(*this, cast_or_null<WarnUnusedResultAttr>(
- CE->getUnusedResultAttr(Context)),
+ if (CE->getDisableUnusedResultAttr(Context))
+ return;
+
+ if (DiagnoseNoDiscard(*this,
+ cast_or_null<WarnUnusedResultAttr>(
+ CE->getUnusedResultAttr(Context)),
Loc, R1, R2, /*isCtor=*/false))
return;
@@ -305,6 +309,11 @@
}
} else if (const auto *CE = dyn_cast<CXXConstructExpr>(E)) {
if (const CXXConstructorDecl *Ctor = CE->getConstructor()) {
+ const auto *D = Ctor->getAttr<DisableWarnUnusedResultAttr>();
+ D = D ? D : Ctor->getParent()->getAttr<DisableWarnUnusedResultAttr>();
+ if (D)
+ return;
+
const auto *A = Ctor->getAttr<WarnUnusedResultAttr>();
A = A ? A : Ctor->getParent()->getAttr<WarnUnusedResultAttr>();
if (DiagnoseNoDiscard(*this, A, Loc, R1, R2, /*isCtor=*/true))
@@ -312,6 +321,8 @@
}
} else if (const auto *ILE = dyn_cast<InitListExpr>(E)) {
if (const TagDecl *TD = ILE->getType()->getAsTagDecl()) {
+ if (TD->getAttr<DisableWarnUnusedResultAttr>())
+ return;
if (DiagnoseNoDiscard(*this, TD->getAttr<WarnUnusedResultAttr>(), Loc, R1,
R2, /*isCtor=*/false))
@@ -328,6 +339,8 @@
}
const ObjCMethodDecl *MD = ME->getMethodDecl();
if (MD) {
+ if (MD->getAttr<DisableWarnUnusedResultAttr>())
+ return;
if (DiagnoseNoDiscard(*this, MD->getAttr<WarnUnusedResultAttr>(), Loc, R1,
R2, /*isCtor=*/false))
return;
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3167,6 +3167,39 @@
D->addAttr(::new (S.Context) WarnUnusedResultAttr(S.Context, AL, Str));
}
+static void handleDisableWarnUnusedResult(Sema &S, Decl *D,
+ const ParsedAttr &AL) {
+ if (D->getFunctionType() &&
+ D->getFunctionType()->getReturnType()->isVoidType() &&
+ !isa<CXXConstructorDecl>(D)) {
+ S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method) << AL << 0;
+ return;
+ }
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(D))
+ if (MD->getReturnType()->isVoidType()) {
+ S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method) << AL << 1;
+ return;
+ }
+
+ if (AL.isStandardAttributeSyntax() && !AL.getScopeName()) {
+ // The standard attribute cannot be applied to variable declarations such
+ // as a function pointer.
+ if (isa<VarDecl>(D))
+ S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type_str)
+ << AL << "functions, classes, or enumerations";
+ }
+
+ 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) DisableWarnUnusedResultAttr(S.Context, AL));
+}
+
static void handleWeakImportAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
// weak_import only applies to variable & function declarations.
bool isDef = false;
@@ -8795,6 +8828,9 @@
case ParsedAttr::AT_WarnUnusedResult:
handleWarnUnusedResult(S, D, AL);
break;
+ case ParsedAttr::AT_DisableWarnUnusedResult:
+ handleDisableWarnUnusedResult(S, D, AL);
+ break;
case ParsedAttr::AT_WeakRef:
handleWeakRefAttr(S, D, AL);
break;
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -14,6 +14,7 @@
#include "clang/AST/APValue.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
+#include "clang/AST/Attrs.inc"
#include "clang/AST/ComputeDependence.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
@@ -1534,6 +1535,35 @@
return D ? D->getAttr<WarnUnusedResultAttr>() : nullptr;
}
+const Attr *CallExpr::getDisableUnusedResultAttr(const ASTContext &Ctx) const {
+ // Callees have the final say on whether or not their call is discardable, so
+ // we exit early if the callee has an opinion.
+
+ const Decl *D = getCalleeDecl();
+ const auto *CallResult =
+ D ? D->getAttr<DisableWarnUnusedResultAttr>() : nullptr;
+ if (CallResult != nullptr)
+ return CallResult;
+
+ bool CallIsNodiscard =
+ D ? D->getAttr<WarnUnusedResultAttr>() != nullptr : false;
+ if (CallIsNodiscard && CallResult == nullptr)
+ return nullptr;
+
+ // If the return type is a class or enum type that is marked discardable and
+ // the call isn't marked nodiscard, return the type attribute.
+ if (const TagDecl *TD = getCallReturnType(Ctx)->getAsTagDecl())
+ if (const auto *A = TD->getAttr<DisableWarnUnusedResultAttr>())
+ return A;
+
+ for (const auto *TD = getCallReturnType(Ctx)->getAs<TypedefType>(); TD;
+ TD = TD->desugar()->getAs<TypedefType>())
+ if (const auto *A = TD->getDecl()->getAttr<DisableWarnUnusedResultAttr>())
+ return A;
+
+ return nullptr;
+}
+
SourceLocation CallExpr::getBeginLoc() const {
if (isa<CXXOperatorCallExpr>(this))
return cast<CXXOperatorCallExpr>(this)->getBeginLoc();
@@ -2711,7 +2741,8 @@
}
if (const ObjCMethodDecl *MD = ME->getMethodDecl())
- if (MD->hasAttr<WarnUnusedResultAttr>()) {
+ if (MD->hasAttr<WarnUnusedResultAttr>() &&
+ !MD->hasAttr<DisableWarnUnusedResultAttr>()) {
WarnE = this;
Loc = getExprLoc();
return true;
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -513,16 +513,16 @@
let Category = DocCatStmt;
let Content = [{
If a statement is marked ``nomerge`` and contains call expressions, those call
-expressions inside the statement will not be merged during optimization. This
+expressions inside the statement will not be merged during optimization. This
attribute can be used to prevent the optimizer from obscuring the source
location of certain calls. For example, it will prevent tail merging otherwise
identical code sequences that raise an exception or terminate the program. Tail
merging normally reduces the precision of source location information, making
stack traces less useful for debugging. This attribute gives the user control
-over the tradeoff between code size and debug information precision.
+over the tradeoff between code size and debug information precision.
-``nomerge`` attribute can also be used as function attribute to prevent all
-calls to the specified function from merging. It has no effect on indirect
+``nomerge`` attribute can also be used as function attribute to prevent all
+calls to the specified function from merging. It has no effect on indirect
calls.
}];
}
@@ -1555,7 +1555,7 @@
``watchos``
Apple's watchOS operating system. The minimum deployment target is specified by
the ``-mwatchos-version-min=*version*`` command-line argument.
-
+
``driverkit``
Apple's DriverKit userspace kernel extensions. The minimum deployment target
is specified as part of the triple.
@@ -1862,6 +1862,37 @@
}];
}
+def DisableWarnUnusedResultDocs : Documentation {
+ let Category = DocCatFunction;
+ let Heading = "clang::discardable";
+ let Content = [{
+Although the default behaviour for C and C++ is to not diagnose ignored function
+calls, we can make this the default by using ``#pragma clang attribute
+push([[nodiscard]], apply_to = function)``. When we have a function that can
+have its value discarded, we can use ``[[clang::discardable]]`` to indicate that
+the ``[[nodiscard]]`` attribute should be ignored.
+
+``[[clang::discardable]]`` can be placed anywhere ``[[nodiscard]]`` is allowed,
+but the presence of either is prioritised when it's applied to a callable.
+
+.. code-block c++
+ struct S1 {};
+ [[nodiscard, clang::discardable]] S1 f1();
+
+ struct [[nodiscard]] S2 {};
+ [[clang::discardable]] int f2();
+
+ struct [[clang::discardable]] S3 {};
+ [[nodiscard]] int f3();
+
+ int main() {
+ f1(); // no warning (cancels out)
+ f2(); // no warning (function attribute overrides type attribute)
+ f3(); // warno (function attribute overrides type attribute)
+ }
+ }];
+}
+
def FallthroughDocs : Documentation {
let Category = DocCatStmt;
let Heading = "fallthrough";
@@ -3816,7 +3847,7 @@
with pointers in the C family of languages. The various nullability attributes
indicate whether a particular pointer can be null or not, which makes APIs more
expressive and can help static analysis tools identify bugs involving null
-pointers. Clang supports several kinds of nullability attributes: the
+pointers. Clang supports several kinds of nullability attributes: the
``nonnull`` and ``returns_nonnull`` attributes indicate which function or
method parameters and result types can never be null, while nullability type
qualifiers indicate which pointer types can be null (``_Nullable``) or cannot
@@ -3992,7 +4023,7 @@
The ``returns_nonnull`` attribute implies that returning a null pointer is
undefined behavior, which the optimizer may take advantage of. The ``_Nonnull``
type qualifier indicates that a pointer cannot be null in a more general manner
-(because it is part of the type system) and does not imply undefined behavior,
+(because it is part of the type system) and does not imply undefined behavior,
making it more widely applicable
}];
}
@@ -5887,15 +5918,15 @@
let Content = [{
Code can indicate CFG checks are not wanted with the ``__declspec(guard(nocf))``
attribute. This directs the compiler to not insert any CFG checks for the entire
-function. This approach is typically used only sparingly in specific situations
-where the programmer has manually inserted "CFG-equivalent" protection. The
-programmer knows that they are calling through some read-only function table
-whose address is obtained through read-only memory references and for which the
-index is masked to the function table limit. This approach may also be applied
-to small wrapper functions that are not inlined and that do nothing more than
-make a call through a function pointer. Since incorrect usage of this directive
-can compromise the security of CFG, the programmer must be very careful using
-the directive. Typically, this usage is limited to very small functions that
+function. This approach is typically used only sparingly in specific situations
+where the programmer has manually inserted "CFG-equivalent" protection. The
+programmer knows that they are calling through some read-only function table
+whose address is obtained through read-only memory references and for which the
+index is masked to the function table limit. This approach may also be applied
+to small wrapper functions that are not inlined and that do nothing more than
+make a call through a function pointer. Since incorrect usage of this directive
+can compromise the security of CFG, the programmer must be very careful using
+the directive. Typically, this usage is limited to very small functions that
only call one function.
`Control Flow Guard documentation <https://docs.microsoft.com/en-us/windows/win32/secbp/pe-metadata>`
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2981,6 +2981,12 @@
}];
}
+def DisableWarnUnusedResult : InheritableAttr {
+ let Spellings = [Clang<"discardable", 0>];
+ let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike, TypedefName]>;
+ let Documentation = [DisableWarnUnusedResultDocs];
+}
+
def Weak : InheritableAttr {
let Spellings = [GCC<"weak">];
let Subjects = SubjectList<[Var, Function, CXXRecord]>;
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -3107,6 +3107,10 @@
/// function, or its return type declaration.
const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+ /// Returns the DisableWarnUnusedResultAttr that is either declared on the
+ /// called function, or its return type declaration.
+ const Attr *getDisableUnusedResultAttr(const ASTContext &Ctx) const;
+
/// Returns true if this call expression should warn on unused results.
bool hasUnusedResultAttr(const ASTContext &Ctx) const {
return getUnusedResultAttr(Ctx) != nullptr;
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -474,6 +474,9 @@
unsigned character literals. This fixes `Issue 54886 <https://github.com/llvm/llvm-project/issues/54886>`_.
- Stopped allowing constraints on non-template functions to be compliant with
dcl.decl.general p4.
+- Added the attribute ``[[clang::discardable]]``, which cancels out ``[[nodiscard]]``.
+ This makes it possible for libraries to liberally use ``[[nodiscard]]`` inside
+ ``#pragma clang attribute``.
C++20 Feature Support
^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
