comex created this revision.
comex added a reviewer: dblaikie.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
As suggested by FIXME comments, fix commented-out diagnostic in Sema and remove
the equivalent check within the consumed analysis.
The diagnostic in question is the warning for using `param_typestate` and
`return_typestate` with a type that is not consumable.
There were several FIXME comments about the same issue; the most detailed was
before the commented-out check:
// FIXME: This check is currently being done in the analysis. It can be
// enabled here only after the parser propagates attributes at
// template specialization definition, not declaration.
I was confused what this meant. After investigating, I think it actually
refers to the fact that attributes are parsed only once for a template, and are
not re-parsed when the template is instantiated. If I'm right, the issue
actually has nothing to do with template specializations or with definitions
versus declarations. I could be missing something, though, so please let me
know if there's a case I'm not thinking of. I did add some template
specializations as test cases (for both class and function templates).
This patch addresses the issue by moving the diagnostic to a function which is
called from both parsing and template instantiation, similar to what's already
done with some other attributes.
The analysis version of the check didn't always work, and only applied to
`return_typestate` rather than `param_typestate` (even though both had
commented-out Sema checks); therefore, the fixed code may produce warnings that
didn't appear before.
Also, add a new diagnostic for when the `set_typestate` or `test_typestate`
attribute is applied to a constructor or static method; previously Clang would
ignore it or crash, respectively.
Repository:
rC Clang
https://reviews.llvm.org/D67740
Files:
include/clang/Analysis/Analyses/Consumed.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Analysis/Consumed.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
test/SemaCXX/warn-consumed-parsing.cpp
Index: test/SemaCXX/warn-consumed-parsing.cpp
===================================================================
--- test/SemaCXX/warn-consumed-parsing.cpp
+++ test/SemaCXX/warn-consumed-parsing.cpp
@@ -6,16 +6,6 @@
#define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))
#define TEST_TYPESTATE(state) __attribute__ ((test_typestate(state)))
-// FIXME: This test is here because the warning is issued by the Consumed
-// analysis, not SemaDeclAttr. The analysis won't run after an error
-// has been issued. Once the attribute propagation for template
-// instantiation has been fixed, this can be moved somewhere else and the
-// definition can be removed.
-int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{return state set for an unconsumable type 'int'}}
-int returnTypestateForUnconsumable() {
- return 0;
-}
-
class AttrTester0 {
void consumes() __attribute__ ((set_typestate())); // expected-error {{'set_typestate' attribute takes one argument}}
bool testUnconsumed() __attribute__ ((test_typestate())); // expected-error {{'test_typestate' attribute takes one argument}}
@@ -62,5 +52,37 @@
Status {
};
+int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{attribute 'return_typestate' invalid for return value of type 'int': expected a consumable class type as a value, reference, or rvalue reference}}
+int returnTypestateForUnconsumable() {
+ return 0;
+}
+
+struct CONSUMABLE(unknown) UselessAttrs {
+ static void x() SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a static method of class 'UselessAttrs'}}
+ UselessAttrs() SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a constructor of class 'UselessAttrs'}}
+ UselessAttrs(int) CALLABLE_WHEN(consumed); // expected-warning {{consumed analysis attribute is attached to a constructor of class 'UselessAttrs'}}
+ void operator+(UselessAttrs) SET_TYPESTATE(consumed); // OK
+ static void *operator new(unsigned long) SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a static method of class 'UselessAttrs'}}
+ template <typename T>
+ void a([[clang::param_typestate(consumed)]] int) {} // expected-warning {{attribute 'param_typestate' invalid for parameter of type 'int': expected a consumable class type as a value, reference, or rvalue reference}}
+ void b([[clang::return_typestate(consumed)]] UselessAttrs *) {} // expected-warning {{attribute 'return_typestate' invalid for parameter of type 'UselessAttrs *': expected a consumable class type as a value, reference, or rvalue reference}}
+ template <typename T>
+ void c([[clang::return_typestate(consumed)]] T) {} // expected-warning {{attribute 'return_typestate' invalid for parameter of type 'int': expected a consumable class type as a value, reference, or rvalue reference}}
+ template <> // test a template specialization
+ void c<long>([[clang::return_typestate(consumed)]] long); // expected-warning {{attribute 'return_typestate' invalid for parameter of type 'long': expected a consumable class type as a value, reference, or rvalue reference}}
+ void instantiate() {
+ a<int>(42);
+ c(42);
+ }
+};
+void operator-(UselessAttrs, UselessAttrs) SET_TYPESTATE(consumed); // expected-warning {{'set_typestate' attribute only applies to functions}}
+template <typename T, typename U>
+struct CONSUMABLE(unknown) ClassTemplateSpecialization;
+template <typename T>
+struct ClassTemplateSpecialization<T, int> {
+ [[clang::return_typestate(consumed)]] ClassTemplateSpecialization(); // expected-warning {{attribute 'return_typestate' invalid for return value of type 'ClassTemplateSpecialization<long, int>': expected a consumable class type as a value, reference, or rvalue reference}}
+ void a([[clang::return_typestate(consumed)]] T) {} // expected-warning {{attribute 'return_typestate' invalid for parameter of type 'long': expected a consumable class type as a value, reference, or rvalue reference}}
+};
+void testTS() { ClassTemplateSpecialization<long, int>().a(1); } // expected-note {{in instantiation of template class 'ClassTemplateSpecialization<long, int>' requested here}}
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -549,6 +549,11 @@
continue;
}
+ if (isa<ParamTypestateAttr>(TmplAttr) || isa<ReturnTypestateAttr>(TmplAttr)) {
+ if (!CheckParamOrReturnTypestateAttr(TmplAttr, New, Tmpl))
+ continue; // don't add
+ }
+
assert(!TmplAttr->isPackExpansion());
if (TmplAttr->isLateParsed() && LateAttrs) {
// Late parsed attributes must be instantiated and attached after the
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1122,15 +1122,20 @@
static bool checkForConsumableClass(Sema &S, const CXXMethodDecl *MD,
const ParsedAttr &AL) {
- QualType ThisType = MD->getThisType()->getPointeeType();
+ const CXXRecordDecl *RD = MD->getParent();
- if (const CXXRecordDecl *RD = ThisType->getAsCXXRecordDecl()) {
- if (!RD->hasAttr<ConsumableAttr>()) {
- S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class) <<
- RD->getNameAsString();
+ if (!RD->hasAttr<ConsumableAttr>()) {
+ S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class) <<
+ RD->getNameAsString();
- return false;
- }
+ return false;
+ }
+
+ if (MD->isStatic() || isa<CXXConstructorDecl>(MD)) {
+ S.Diag(AL.getLoc(), diag::warn_consumable_attr_on_static) <<
+ (MD->isStatic() ? 0 : 1) <<
+ RD->getNameAsString();
+ return false;
}
return true;
@@ -1190,19 +1195,9 @@
return;
}
- // FIXME: This check is currently being done in the analysis. It can be
- // enabled here only after the parser propagates attributes at
- // template specialization definition, not declaration.
- //QualType ReturnType = cast<ParmVarDecl>(D)->getType();
- //const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
- //
- //if (!RD || !RD->hasAttr<ConsumableAttr>()) {
- // S.Diag(AL.getLoc(), diag::warn_return_state_for_unconsumable_type) <<
- // ReturnType.getAsString();
- // return;
- //}
-
- D->addAttr(::new (S.Context) ParamTypestateAttr(S.Context, AL, ParamState));
+ Attr *A = ::new (S.Context) ParamTypestateAttr(S.Context, AL, ParamState);
+ if (S.CheckParamOrReturnTypestateAttr(A, D, nullptr))
+ D->addAttr(A);
}
static void handleReturnTypestateAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
@@ -1222,32 +1217,42 @@
return;
}
- // FIXME: This check is currently being done in the analysis. It can be
- // enabled here only after the parser propagates attributes at
- // template specialization definition, not declaration.
- //QualType ReturnType;
- //
- //if (const ParmVarDecl *Param = dyn_cast<ParmVarDecl>(D)) {
- // ReturnType = Param->getType();
- //
- //} else if (const CXXConstructorDecl *Constructor =
- // dyn_cast<CXXConstructorDecl>(D)) {
- // ReturnType = Constructor->getThisType()->getPointeeType();
- //
- //} else {
- //
- // ReturnType = cast<FunctionDecl>(D)->getCallResultType();
- //}
- //
- //const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
- //
- //if (!RD || !RD->hasAttr<ConsumableAttr>()) {
- // S.Diag(Attr.getLoc(), diag::warn_return_state_for_unconsumable_type) <<
- // ReturnType.getAsString();
- // return;
- //}
+ Attr *A = ::new (S.Context) ReturnTypestateAttr(S.Context, AL, ReturnState);
+ if (S.CheckParamOrReturnTypestateAttr(A, D, nullptr))
+ D->addAttr(A);
+}
+
+// Check a `param_typestate` or `return_typestate` attribute: emit a warning
+// and return false if the parameter or return type is not consumable.
- D->addAttr(::new (S.Context) ReturnTypestateAttr(S.Context, AL, ReturnState));
+bool Sema::CheckParamOrReturnTypestateAttr(const Attr *A, const Decl *D,
+ const Decl *Old) {
+ auto GetType = [&](const Decl *X) -> QualType {
+ // `return_typestate` can be applied to parameters (referring to the
+ // parameter), constructors (referring to the new object), or functions
+ // (referring to the return value). `param_typestate` can only be applied
+ // to parameters.
+ if (auto *PX = dyn_cast<ParmVarDecl>(X))
+ return PX->getType();
+ else if (auto *CX = dyn_cast<CXXConstructorDecl>(X))
+ return CX->getThisType()->getPointeeType();
+ else
+ return cast<FunctionDecl>(X)->getCallResultType();
+ };
+ QualType Type = GetType(D);
+ if (Type->isDependentType())
+ return true; // will re-check on instantiation
+ if (Old && !GetType(Old)->isDependentType())
+ return true; // we already checked this
+ if (Type->isReferenceType())
+ Type = Type->getPointeeType();
+ if (const CXXRecordDecl *RD = Type->getAsCXXRecordDecl())
+ if (RD->hasAttr<ConsumableAttr>())
+ return true; // OK
+
+ Diag(A->getLoc(), diag::warn_consumable_attr_for_unconsumable_type) <<
+ A << (isa<ParmVarDecl>(D) ? 0 : 1) << Type;
+ return false;
}
static void handleSetTypestateAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
Index: lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -1914,14 +1914,6 @@
Warnings.emplace_back(std::move(Warning), OptionalNotes());
}
- void warnReturnTypestateForUnconsumableType(SourceLocation Loc,
- StringRef TypeName) override {
- PartialDiagnosticAt Warning(Loc, S.PDiag(
- diag::warn_return_typestate_for_unconsumable_type) << TypeName);
-
- Warnings.emplace_back(std::move(Warning), OptionalNotes());
- }
-
void warnReturnTypestateMismatch(SourceLocation Loc, StringRef ExpectedState,
StringRef ObservedState) override {
Index: lib/Analysis/Consumed.cpp
===================================================================
--- lib/Analysis/Consumed.cpp
+++ lib/Analysis/Consumed.cpp
@@ -1203,19 +1203,9 @@
} else
ReturnType = D->getCallResultType();
- if (const ReturnTypestateAttr *RTSAttr = D->getAttr<ReturnTypestateAttr>()) {
- const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
- if (!RD || !RD->hasAttr<ConsumableAttr>()) {
- // FIXME: This should be removed when template instantiation propagates
- // attributes at template specialization definition, not
- // declaration. When it is removed the test needs to be enabled
- // in SemaDeclAttr.cpp.
- WarningsHandler.warnReturnTypestateForUnconsumableType(
- RTSAttr->getLocation(), ReturnType.getAsString());
- ExpectedReturnState = CS_None;
- } else
- ExpectedReturnState = mapReturnTypestateAttrState(RTSAttr);
- } else if (isConsumableType(ReturnType)) {
+ if (const ReturnTypestateAttr *RTSAttr = D->getAttr<ReturnTypestateAttr>())
+ ExpectedReturnState = mapReturnTypestateAttrState(RTSAttr);
+ else if (isConsumableType(ReturnType)) {
if (isAutoCastType(ReturnType)) // We can auto-cast the state to the
ExpectedReturnState = CS_None; // expected state.
else
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -8942,6 +8942,9 @@
bool checkNSReturnsRetainedReturnType(SourceLocation loc, QualType type);
+ bool CheckParamOrReturnTypestateAttr(const Attr *A, const Decl *D,
+ const Decl *Old);
+
//===--------------------------------------------------------------------===//
// C++ Coroutines TS
//
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3229,9 +3229,14 @@
def warn_attr_on_unconsumable_class : Warning<
"consumed analysis attribute is attached to member of class '%0' which isn't "
"marked as consumable">, InGroup<Consumed>, DefaultIgnore;
-def warn_return_typestate_for_unconsumable_type : Warning<
- "return state set for an unconsumable type '%0'">, InGroup<Consumed>,
- DefaultIgnore;
+def warn_consumable_attr_on_static : Warning<
+ "consumed analysis attribute is attached to a "
+ "%select{static method|constructor}0 of class '%1'">,
+ InGroup<Consumed>, DefaultIgnore;
+def warn_consumable_attr_for_unconsumable_type : Warning<
+ "attribute %0 invalid for %select{parameter|return value}1 of type %2: "
+ "expected a consumable class type as a value, reference, or rvalue reference">,
+ InGroup<Consumed>, DefaultIgnore;
def warn_return_typestate_mismatch : Warning<
"return value not in expected state; expected '%0', observed '%1'">,
InGroup<Consumed>, DefaultIgnore;
Index: include/clang/Analysis/Analyses/Consumed.h
===================================================================
--- include/clang/Analysis/Analyses/Consumed.h
+++ include/clang/Analysis/Analyses/Consumed.h
@@ -89,16 +89,6 @@
StringRef ExpectedState,
StringRef ObservedState) {}
- // FIXME: This can be removed when the attr propagation fix for templated
- // classes lands.
- /// Warn about return typestates set for unconsumable types.
- ///
- /// \param Loc -- The location of the attributes.
- ///
- /// \param TypeName -- The name of the unconsumable type.
- virtual void warnReturnTypestateForUnconsumableType(SourceLocation Loc,
- StringRef TypeName) {}
-
/// Warn about return typestate mismatches.
///
/// \param Loc -- The SourceLocation of the return statement.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
