sberg created this revision.
sberg added a reviewer: erichkeane.
Herald added a reviewer: aaron.ballman.
Herald added a subscriber: jdoerfert.
Herald added a project: All.
sberg requested review of this revision.
Herald added a project: clang.
This takes inspiration from the standard `nodiscard` attribute, which can also
be declared on individual constructors. (I have used this change locally for
about a year on the LibreOffice code base, marking various compilers as
`warn_unused`, without any positive or negative effects. Until I happened to
notice an additional constructor that would benefit from `warn_unused`, and
which found about 20 unused variables across the LibreOffice code base, see
e.g.,
https://git.libreoffice.org/core/+/7cdbe504ff3b59d3aec1b1e86caf7f24223dce72%5E!
"Fix what looks like copy/paste typos".)
(The changes in `clang/test/Misc/pragma-attribute-strict-subjects.c`,
`clang/test/Parser/pragma-attribute.cpp`, and
`clang/test/Sema/pragma-attribute-strict-subjects.c` are necessary to make
those tests not fail after adding `SubRuleForCXXConstructor` to
`SubjectMatcherForFunction` in `clang/include/clang/Basic/Attr.td`. It appears
that the exact diagnostic output that is generated is somewhat brittle,
depending on implementation details.)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D148505
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/DeclNodes.td
clang/lib/AST/Expr.cpp
clang/lib/Sema/SemaDecl.cpp
clang/test/Misc/pragma-attribute-strict-subjects.c
clang/test/Misc/pragma-attribute-supported-attributes-list.test
clang/test/Parser/pragma-attribute.cpp
clang/test/Sema/pragma-attribute-strict-subjects.c
clang/test/SemaCXX/warn-unused-attribute.cpp
Index: clang/test/SemaCXX/warn-unused-attribute.cpp
===================================================================
--- clang/test/SemaCXX/warn-unused-attribute.cpp
+++ clang/test/SemaCXX/warn-unused-attribute.cpp
@@ -9,12 +9,21 @@
TestNormal();
};
+struct TestCtor {
+ __attribute__((warn_unused)) TestCtor();
+ TestCtor(int);
+ void __attribute__((warn_unused)) f(); // expected-warning {{'warn_unused' attribute only applies to structs, unions, classes, and constructors}}
+};
+
int main(void) {
Test unused; // expected-warning {{unused variable 'unused'}}
Test used;
TestNormal normal;
used.use();
- int i __attribute__((warn_unused)) = 12; // expected-warning {{'warn_unused' attribute only applies to structs, unions, and classes}}
+ TestCtor ctorUnused; // expected-warning {{unused variable 'ctorUnused'}}
+ TestCtor ctorUsed(0);
+
+ int i __attribute__((warn_unused)) = 12; // expected-warning {{'warn_unused' attribute only applies to structs, unions, classes, and constructors}}
return i;
}
Index: clang/test/Sema/pragma-attribute-strict-subjects.c
===================================================================
--- clang/test/Sema/pragma-attribute-strict-subjects.c
+++ clang/test/Sema/pragma-attribute-strict-subjects.c
@@ -28,7 +28,7 @@
#pragma clang attribute pop
#pragma clang attribute push (__attribute__((annotate("negatedSubRuleContradictions2"))), apply_to = any(variable(unless(is_parameter)), variable(is_thread_local), function, variable(is_global)))
-// expected-error@-1 {{negated attribute subject matcher sub-rule 'variable(unless(is_parameter))' contradicts sub-rule 'variable(is_global)'}}
+// expected-error@-1 {{negated attribute subject matcher sub-rule 'variable(unless(is_parameter))' contradicts sub-rule 'variable(is_thread_local)'}}
// We have just one error, don't error on 'variable(is_global)'
#pragma clang attribute pop
Index: clang/test/Parser/pragma-attribute.cpp
===================================================================
--- clang/test/Parser/pragma-attribute.cpp
+++ clang/test/Parser/pragma-attribute.cpp
@@ -190,7 +190,7 @@
#pragma clang attribute pop
#pragma clang attribute push([[clang::uninitialized]], apply_to = variable(is_global)) // expected-error {{attribute 'uninitialized' can't be applied to 'variable(is_global)'}}
#pragma clang attribute pop
-#pragma clang attribute push([[clang::uninitialized]], apply_to = any(variable(is_parameter), variable(unless(is_parameter)))) // expected-error {{attribute 'uninitialized' can't be applied to 'variable(is_parameter)', and 'variable(unless(is_parameter))'}}
+#pragma clang attribute push([[clang::uninitialized]], apply_to = any(variable(is_parameter), variable(unless(is_parameter)))) // expected-error {{attribute 'uninitialized' can't be applied to 'variable(unless(is_parameter))', and 'variable(is_parameter)'}}
#pragma clang attribute pop
// We're allowed to apply attributes to subsets of allowed subjects.
#pragma clang attribute push([[clang::no_destroy]], apply_to = variable)
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
@@ -192,7 +192,7 @@
// CHECK-NEXT: UseHandle (SubjectMatchRule_variable_is_parameter)
// CHECK-NEXT: VecReturn (SubjectMatchRule_record)
// CHECK-NEXT: VecTypeHint (SubjectMatchRule_function)
-// CHECK-NEXT: WarnUnused (SubjectMatchRule_record)
+// CHECK-NEXT: WarnUnused (SubjectMatchRule_record, SubjectMatchRule_function_constructor)
// 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)
Index: clang/test/Misc/pragma-attribute-strict-subjects.c
===================================================================
--- clang/test/Misc/pragma-attribute-strict-subjects.c
+++ clang/test/Misc/pragma-attribute-strict-subjects.c
@@ -16,7 +16,7 @@
#pragma clang attribute pop
#pragma clang attribute push (__attribute__((annotate("negatedSubRuleContradictions2"))), apply_to = any(variable(unless(is_parameter)), variable(is_thread_local), function, variable(is_global)))
-// expected-error@-1 {{negated attribute subject matcher sub-rule 'variable(unless(is_parameter))' contradicts sub-rule 'variable(is_global)'}}
+// expected-error@-1 {{negated attribute subject matcher sub-rule 'variable(unless(is_parameter))' contradicts sub-rule 'variable(is_thread_local)'}}
// We have just one error, don't error on 'variable(is_global)'
// Ensure that we've recovered from the error:
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2051,15 +2051,15 @@
return false;
if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(Tag)) {
- if (!RD->hasTrivialDestructor() && !RD->hasAttr<WarnUnusedAttr>())
- return false;
-
+ bool ConstructorWarnUnused = false;
if (Init) {
const CXXConstructExpr *Construct =
dyn_cast<CXXConstructExpr>(Init);
- if (Construct && !Construct->isElidable()) {
+ if (Construct) {
CXXConstructorDecl *CD = Construct->getConstructor();
- if (!CD->isTrivial() && !RD->hasAttr<WarnUnusedAttr>() &&
+ ConstructorWarnUnused = CD->hasAttr<WarnUnusedAttr>();
+ if (!Construct->isElidable() && !CD->isTrivial() &&
+ !RD->hasAttr<WarnUnusedAttr>() && !ConstructorWarnUnused &&
(VD->getInit()->isValueDependent() || !VD->evaluateValue()))
return false;
}
@@ -2077,6 +2077,10 @@
if (isa<CXXUnresolvedConstructExpr>(Init))
return false;
}
+
+ if (!RD->hasTrivialDestructor() && !RD->hasAttr<WarnUnusedAttr>()
+ && !ConstructorWarnUnused)
+ return false;
}
}
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2769,7 +2769,8 @@
if (const CXXRecordDecl *Type = getType()->getAsCXXRecordDecl()) {
const auto *WarnURAttr = Type->getAttr<WarnUnusedResultAttr>();
if (Type->hasAttr<WarnUnusedAttr>() ||
- (WarnURAttr && WarnURAttr->IsCXX11NoDiscard())) {
+ (WarnURAttr && WarnURAttr->IsCXX11NoDiscard()) ||
+ cast<CXXConstructExpr>(this)->getConstructor()->hasAttr<WarnUnusedAttr>()) {
WarnE = this;
Loc = getBeginLoc();
R1 = getSourceRange();
Index: clang/include/clang/Basic/DeclNodes.td
===================================================================
--- clang/include/clang/Basic/DeclNodes.td
+++ clang/include/clang/Basic/DeclNodes.td
@@ -51,7 +51,7 @@
def Function : DeclNode<Declarator, "functions">, DeclContext;
def CXXDeductionGuide : DeclNode<Function>;
def CXXMethod : DeclNode<Function>;
- def CXXConstructor : DeclNode<CXXMethod>;
+ def CXXConstructor : DeclNode<CXXMethod, "constructors">;
def CXXDestructor : DeclNode<CXXMethod>;
def CXXConversion : DeclNode<CXXMethod>;
def Var : DeclNode<Declarator, "variables">;
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -471,8 +471,12 @@
def SubRuleForCXXMethod : AttrSubjectMatcherSubRule<"is_member", [CXXMethod]> {
let LangOpts = [CPlusPlus];
}
+def SubRuleForCXXConstructor :
+ AttrSubjectMatcherSubRule<"constructor", [CXXConstructor]> {
+ let LangOpts = [CPlusPlus];
+}
def SubjectMatcherForFunction : AttrSubjectMatcherRule<"function", [Function], [
- SubRuleForCXXMethod
+ SubRuleForCXXMethod, SubRuleForCXXConstructor
]>;
// hasType is abstract, it should be used with one of the sub-rules.
def SubjectMatcherForType : AttrSubjectMatcherRule<"hasType", [], [
@@ -2989,7 +2993,7 @@
def WarnUnused : InheritableAttr {
let Spellings = [GCC<"warn_unused">];
- let Subjects = SubjectList<[Record]>;
+ let Subjects = SubjectList<[Record, CXXConstructor]>;
let Documentation = [Undocumented];
let SimpleHandler = 1;
}
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -186,6 +186,8 @@
the compilation of the foreign language sources (e.g. Swift).
- The ``__has_attribute``, ``__has_c_attribute`` and ``__has_cpp_attribute``
preprocessor operators now return 1 also for attributes defined by plugins.
+- In C++, the ``__attribute__((warn_unused))`` can now also be used on individual
+ constructors,
Improvements to Clang's diagnostics
-----------------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
