r192515 - Consumed analysis: replace the consumes attribute with a set_typestate
Aaron Ballman
aaron at aaronballman.com
Sat Oct 12 08:01:24 PDT 2013
A couple of points below:
On Fri, Oct 11, 2013 at 7:03 PM, DeLesley Hutchins <delesley at google.com> wrote:
> Author: delesley
> Date: Fri Oct 11 18:03:26 2013
> New Revision: 192515
>
> URL: http://llvm.org/viewvc/llvm-project?rev=192515&view=rev
> Log:
> Consumed analysis: replace the consumes attribute with a set_typestate
> attribute. Patch by chris.wailes at gmail.com; reviewed and edited by delesley.
>
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Analysis/Consumed.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp
> cfe/trunk/test/SemaCXX/warn-consumed-parsing.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=192515&r1=192514&r2=192515&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Fri Oct 11 18:03:26 2013
> @@ -967,19 +967,6 @@ def CallableWhen : InheritableAttr {
> ["Unknown", "Consumed", "Unconsumed"]>];
> }
>
> -def TestsTypestate : InheritableAttr {
> - let Spellings = [GNU<"tests_typestate">];
> - let Subjects = [CXXMethod];
> - let Args = [EnumArgument<"TestState", "ConsumedState",
> - ["consumed", "unconsumed"],
> - ["Consumed", "Unconsumed"]>];
> -}
> -
> -def Consumes : InheritableAttr {
> - let Spellings = [GNU<"consumes">];
> - let Subjects = [CXXMethod];
> -}
> -
> def ReturnTypestate : InheritableAttr {
> let Spellings = [GNU<"return_typestate">];
> let Subjects = [Function];
> @@ -988,6 +975,22 @@ def ReturnTypestate : InheritableAttr {
> ["Unknown", "Consumed", "Unconsumed"]>];
> }
>
> +def SetTypestate : InheritableAttr {
> + let Spellings = [GNU<"set_typestate">];
> + let Subjects = [CXXMethod];
> + let Args = [EnumArgument<"NewState", "ConsumedState",
> + ["unknown", "consumed", "unconsumed"],
> + ["Unknown", "Consumed", "Unconsumed"]>];
> +}
> +
> +def TestsTypestate : InheritableAttr {
> + let Spellings = [GNU<"tests_typestate">];
> + let Subjects = [CXXMethod];
> + let Args = [EnumArgument<"TestState", "ConsumedState",
> + ["consumed", "unconsumed"],
> + ["Consumed", "Unconsumed"]>];
> +}
> +
> // Type safety attributes for `void *' pointers and type tags.
>
> def ArgumentWithTypeTag : InheritableAttr {
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=192515&r1=192514&r2=192515&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Oct 11 18:03:26 2013
> @@ -2216,8 +2216,8 @@ def warn_attr_on_unconsumable_class : Wa
> def warn_return_typestate_for_unconsumable_type : Warning<
> "return state set for an unconsumable type '%0'">, InGroup<Consumed>,
> DefaultIgnore;
> -def warn_invalid_test_typestate : Warning<
> - "invalid test typestate '%0'">, InGroup<Consumed>, DefaultIgnore;
> +def warn_unknown_consumed_state : Warning<
> + "unknown consumed analysis state '%0'">, InGroup<Consumed>, DefaultIgnore;
This is not needed (more info below).
> def warn_return_typestate_mismatch : Warning<
> "return value not in expected state; expected '%0', observed '%1'">,
> InGroup<Consumed>, DefaultIgnore;
>
> Modified: cfe/trunk/lib/Analysis/Consumed.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/Consumed.cpp?rev=192515&r1=192514&r2=192515&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/Consumed.cpp (original)
> +++ cfe/trunk/lib/Analysis/Consumed.cpp Fri Oct 11 18:03:26 2013
> @@ -157,6 +157,18 @@ static ConsumedState mapConsumableAttrSt
> llvm_unreachable("invalid enum");
> }
>
> +static ConsumedState mapSetTypestateAttrState(const SetTypestateAttr *STAttr) {
> + switch (STAttr->getNewState()) {
> + case SetTypestateAttr::Unknown:
> + return CS_Unknown;
> + case SetTypestateAttr::Unconsumed:
> + return CS_Unconsumed;
> + case SetTypestateAttr::Consumed:
> + return CS_Consumed;
> + }
> + llvm_unreachable("invalid_enum");
> +}
> +
> static ConsumedState
> mapReturnTypestateAttrState(const ReturnTypestateAttr *RTSAttr) {
> switch (RTSAttr->getState()) {
> @@ -639,8 +651,9 @@ void ConsumedStmtVisitor::VisitCXXMember
> if (isTestingFunction(MethodDecl))
> PropagationMap.insert(PairType(Call,
> PropagationInfo(PInfo.getVar(), testsFor(MethodDecl))));
> - else if (MethodDecl->hasAttr<ConsumesAttr>())
> - StateMap->setState(PInfo.getVar(), consumed::CS_Consumed);
> + else if (MethodDecl->hasAttr<SetTypestateAttr>())
> + StateMap->setState(PInfo.getVar(),
> + mapSetTypestateAttrState(MethodDecl->getAttr<SetTypestateAttr>()));
> }
> }
> }
> @@ -728,8 +741,9 @@ void ConsumedStmtVisitor::VisitCXXOperat
> if (isTestingFunction(FunDecl))
> PropagationMap.insert(PairType(Call,
> PropagationInfo(PInfo.getVar(), testsFor(FunDecl))));
> - else if (FunDecl->hasAttr<ConsumesAttr>())
> - StateMap->setState(PInfo.getVar(), consumed::CS_Consumed);
> + else if (FunDecl->hasAttr<SetTypestateAttr>())
> + StateMap->setState(PInfo.getVar(),
> + mapSetTypestateAttrState(FunDecl->getAttr<SetTypestateAttr>()));
> }
> }
> }
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=192515&r1=192514&r2=192515&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Oct 11 18:03:26 2013
> @@ -1026,20 +1026,6 @@ static bool checkForConsumableClass(Sema
> return true;
> }
>
> -static void handleConsumesAttr(Sema &S, Decl *D, const AttributeList &Attr) {
> - if (!isa<CXXMethodDecl>(D)) {
> - S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
> - Attr.getName() << ExpectedMethod;
> - return;
> - }
> -
> - if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
> - return;
> -
> - D->addAttr(::new (S.Context)
> - ConsumesAttr(Attr.getRange(), S.Context,
> - Attr.getAttributeSpellingListIndex()));
> -}
>
> static void handleCallableWhenAttr(Sema &S, Decl *D,
> const AttributeList &Attr) {
> @@ -1080,44 +1066,6 @@ static void handleCallableWhenAttr(Sema
> }
>
>
> -static void handleTestsTypestateAttr(Sema &S, Decl *D,
> - const AttributeList &Attr) {
> - if (!checkAttributeNumArgs(S, Attr, 1)) return;
> -
> - if (!isa<CXXMethodDecl>(D)) {
> - S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
> - Attr.getName() << ExpectedMethod;
> - return;
> - }
> -
> - if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
> - return;
> -
> - TestsTypestateAttr::ConsumedState TestState;
> -
> - if (Attr.isArgIdent(0)) {
> - StringRef Param = Attr.getArgAsIdent(0)->Ident->getName();
> -
> - if (Param == "consumed") {
> - TestState = TestsTypestateAttr::Consumed;
> - } else if (Param == "unconsumed") {
> - TestState = TestsTypestateAttr::Unconsumed;
> - } else {
> - S.Diag(Attr.getLoc(), diag::warn_invalid_test_typestate) << Param;
> - return;
> - }
> -
> - } else {
> - S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) <<
> - Attr.getName() << AANT_ArgumentIdentifier;
> - return;
> - }
> -
> - D->addAttr(::new (S.Context)
> - TestsTypestateAttr(Attr.getRange(), S.Context, TestState,
> - Attr.getAttributeSpellingListIndex()));
> -}
> -
> static void handleReturnTypestateAttr(Sema &S, Decl *D,
> const AttributeList &Attr) {
> ReturnTypestateAttr::ConsumedState ReturnState;
> @@ -1168,6 +1116,84 @@ static void handleReturnTypestateAttr(Se
> Attr.getAttributeSpellingListIndex()));
> }
>
> +
> +static void handleSetTypestateAttr(Sema &S, Decl *D, const AttributeList &Attr) {
> + if (!checkAttributeNumArgs(S, Attr, 1)) return;
> +
> + if (!isa<CXXMethodDecl>(D)) {
> + S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
> + Attr.getName() << ExpectedMethod;
> + return;
> + }
> +
> + if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
> + return;
> +
> + SetTypestateAttr::ConsumedState NewState;
> +
> + if (Attr.isArgIdent(0)) {
> + StringRef Param = Attr.getArgAsIdent(0)->Ident->getName();
> +
> + if (Param == "unknown") {
> + NewState = SetTypestateAttr::Unknown;
> + } else if (Param == "consumed") {
> + NewState = SetTypestateAttr::Consumed;
> + } else if (Param == "unconsumed") {
> + NewState = SetTypestateAttr::Unconsumed;
> + } else {
> + S.Diag(Attr.getLoc(), diag::warn_unknown_consumed_state) << Param;
> + return;
> + }
> +
> + } else {
> + S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) <<
> + Attr.getName() << AANT_ArgumentIdentifier;
> + return;
> + }
This whole block (from isArgIdent down) can be replaced with
ConvertStrToTypeSetState (or something akin to that) -- it's td
generated code for converting enumerations to their proper value,
including many diagnostics. As an example, here's visibility:
VisibilityAttr::VisibilityType type;
if (!VisibilityAttr::ConvertStrToVisibilityType(TypeStr, type)) {
S.Diag(LiteralLoc, diag::warn_attribute_type_not_supported)
<< Attr.getName() << TypeStr;
return;
}
You should be using the warn_attribute_type_not_support diagnostic for
compatibility.
> +
> + D->addAttr(::new (S.Context)
> + SetTypestateAttr(Attr.getRange(), S.Context, NewState,
> + Attr.getAttributeSpellingListIndex()));
> +}
> +
> +static void handleTestsTypestateAttr(Sema &S, Decl *D,
> + const AttributeList &Attr) {
> + if (!checkAttributeNumArgs(S, Attr, 1)) return;
> +
> + if (!isa<CXXMethodDecl>(D)) {
> + S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
> + Attr.getName() << ExpectedMethod;
> + return;
> + }
> +
> + if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
> + return;
> +
> + TestsTypestateAttr::ConsumedState TestState;
> +
> + if (Attr.isArgIdent(0)) {
> + StringRef Param = Attr.getArgAsIdent(0)->Ident->getName();
> +
> + if (Param == "consumed") {
> + TestState = TestsTypestateAttr::Consumed;
> + } else if (Param == "unconsumed") {
> + TestState = TestsTypestateAttr::Unconsumed;
> + } else {
> + S.Diag(Attr.getLoc(), diag::warn_unknown_consumed_state) << Param;
> + return;
> + }
> +
> + } else {
> + S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) <<
> + Attr.getName() << AANT_ArgumentIdentifier;
> + return;
> + }
Same comments apply here as above.
> +
> + D->addAttr(::new (S.Context)
> + TestsTypestateAttr(Attr.getRange(), S.Context, TestState,
> + Attr.getAttributeSpellingListIndex()));
> +}
> +
> static void handleExtVectorTypeAttr(Sema &S, Scope *scope, Decl *D,
> const AttributeList &Attr) {
> TypedefNameDecl *TD = dyn_cast<TypedefNameDecl>(D);
> @@ -4793,18 +4819,18 @@ static void ProcessDeclAttribute(Sema &S
> case AttributeList::AT_Consumable:
> handleConsumableAttr(S, D, Attr);
> break;
> - case AttributeList::AT_Consumes:
> - handleConsumesAttr(S, D, Attr);
> - break;
> case AttributeList::AT_CallableWhen:
> handleCallableWhenAttr(S, D, Attr);
> break;
> - case AttributeList::AT_TestsTypestate:
> - handleTestsTypestateAttr(S, D, Attr);
> - break;
> case AttributeList::AT_ReturnTypestate:
> handleReturnTypestateAttr(S, D, Attr);
> break;
> + case AttributeList::AT_SetTypestate:
> + handleSetTypestateAttr(S, D, Attr);
> + break;
> + case AttributeList::AT_TestsTypestate:
> + handleTestsTypestateAttr(S, D, Attr);
> + break;
>
> // Type safety attributes.
> case AttributeList::AT_ArgumentWithTypeTag:
>
> Modified: cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp?rev=192515&r1=192514&r2=192515&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp Fri Oct 11 18:03:26 2013
> @@ -4,7 +4,7 @@
>
> #define CALLABLE_WHEN(...) __attribute__ ((callable_when(__VA_ARGS__)))
> #define CONSUMABLE(state) __attribute__ ((consumable(state)))
> -#define CONSUMES __attribute__ ((consumes))
> +#define SET_TYPESTATE(state) __attribute__ ((set_typestate(state)))
> #define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))
> #define TESTS_TYPESTATE(state) __attribute__ ((tests_typestate(state)))
>
> @@ -23,7 +23,7 @@ public:
>
> ConsumableClass<T>& operator=(ConsumableClass<T> &other);
> ConsumableClass<T>& operator=(ConsumableClass<T> &&other);
> - ConsumableClass<T>& operator=(nullptr_t) CONSUMES;
> + ConsumableClass<T>& operator=(nullptr_t) SET_TYPESTATE(consumed);
>
> template <typename U>
> ConsumableClass<T>& operator=(ConsumableClass<U> &other);
> @@ -31,7 +31,7 @@ public:
> template <typename U>
> ConsumableClass<T>& operator=(ConsumableClass<U> &&other);
>
> - void operator()(int a) CONSUMES;
> + void operator()(int a) SET_TYPESTATE(consumed);
> void operator*() const CALLABLE_WHEN("unconsumed");
> void unconsumedCall() const CALLABLE_WHEN("unconsumed");
> void callableWhenUnknown() const CALLABLE_WHEN("unconsumed", "unknown");
> @@ -44,7 +44,8 @@ public:
> void constCall() const;
> void nonconstCall();
>
> - void consume() CONSUMES;
> + void consume() SET_TYPESTATE(consumed);
> + void unconsume() SET_TYPESTATE(unconsumed);
> };
>
> class CONSUMABLE(unconsumed) DestructorTester {
> @@ -484,7 +485,7 @@ void testConditionalMerge() {
> *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
> }
>
> -void testConsumes0() {
> +void testSetTypestate() {
> ConsumableClass<int> var(42);
>
> *var;
> @@ -492,15 +493,19 @@ void testConsumes0() {
> var.consume();
>
> *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
> +
> + var.unconsume();
> +
> + *var;
> }
>
> -void testConsumes1() {
> +void testConsumes0() {
> ConsumableClass<int> var(nullptr);
>
> *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
> }
>
> -void testConsumes2() {
> +void testConsumes1() {
> ConsumableClass<int> var(42);
>
> var.unconsumedCall();
>
> Modified: cfe/trunk/test/SemaCXX/warn-consumed-parsing.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-consumed-parsing.cpp?rev=192515&r1=192514&r2=192515&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-consumed-parsing.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-consumed-parsing.cpp Fri Oct 11 18:03:26 2013
> @@ -1,10 +1,10 @@
> // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s
>
> #define CALLABLE_WHEN(...) __attribute__ ((callable_when(__VA_ARGS__)))
> -#define CONSUMABLE(state) __attribute__ ((consumable(state)))
> -#define CONSUMES __attribute__ ((consumes))
> -#define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))
> -#define TESTS_TYPESTATE(state) __attribute__ ((tests_typestate(state)))
> +#define CONSUMABLE(state) __attribute__ ((consumable(state)))
> +#define SET_TYPESTATE(state) __attribute__ ((set_typestate(state)))
> +#define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))
> +#define TESTS_TYPESTATE(state) __attribute__ ((tests_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
> @@ -17,27 +17,27 @@ int returnTypestateForUnconsumable() {
> }
>
> class AttrTester0 {
> - void consumes() __attribute__ ((consumes(42))); // expected-error {{attribute takes no arguments}}
> + void consumes() __attribute__ ((set_typestate())); // expected-error {{attribute takes one argument}}
> bool testsUnconsumed() __attribute__ ((tests_typestate())); // expected-error {{attribute takes one argument}}
> void callableWhen() __attribute__ ((callable_when())); // expected-error {{attribute takes at least 1 argument}}
> };
>
> -int var0 CONSUMES; // expected-warning {{'consumes' attribute only applies to methods}}
> +int var0 SET_TYPESTATE(consumed); // expected-warning {{'set_typestate' attribute only applies to methods}}
> int var1 TESTS_TYPESTATE(consumed); // expected-warning {{'tests_typestate' attribute only applies to methods}}
> -int var2 CALLABLE_WHEN(42); // expected-warning {{'callable_when' attribute only applies to methods}}
> +int var2 CALLABLE_WHEN("consumed"); // expected-warning {{'callable_when' attribute only applies to methods}}
> int var3 CONSUMABLE(consumed); // expected-warning {{'consumable' attribute only applies to classes}}
> int var4 RETURN_TYPESTATE(consumed); // expected-warning {{'return_typestate' attribute only applies to functions}}
>
> -void function0() CONSUMES; // expected-warning {{'consumes' attribute only applies to methods}}
> +void function0() SET_TYPESTATE(consumed); // expected-warning {{'set_typestate' attribute only applies to methods}}
> void function1() TESTS_TYPESTATE(consumed); // expected-warning {{'tests_typestate' attribute only applies to methods}}
> -void function2() CALLABLE_WHEN(42); // expected-warning {{'callable_when' attribute only applies to methods}}
> +void function2() CALLABLE_WHEN("consumed"); // expected-warning {{'callable_when' attribute only applies to methods}}
> void function3() CONSUMABLE(consumed); // expected-warning {{'consumable' attribute only applies to classes}}
>
> class CONSUMABLE(unknown) AttrTester1 {
> void callableWhen0() CALLABLE_WHEN("unconsumed");
> void callableWhen1() CALLABLE_WHEN(42); // expected-error {{'callable_when' attribute requires a string}}
> void callableWhen2() CALLABLE_WHEN("foo"); // expected-warning {{'callable_when' attribute argument not supported: foo}}
> - void consumes() CONSUMES;
> + void consumes() SET_TYPESTATE(consumed);
> bool testsUnconsumed() TESTS_TYPESTATE(consumed);
> };
>
> @@ -46,7 +46,7 @@ AttrTester1 returnTypestateTester1() RET
>
> class AttrTester2 {
> void callableWhen() CALLABLE_WHEN("unconsumed"); // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}
> - void consumes() CONSUMES; // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}
> + void consumes() SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}
> bool testsUnconsumed() TESTS_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}
> };
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
~Aaron
More information about the cfe-commits
mailing list