From: DeLesley Hutchins Date: Fri, 11 Oct 2013 22:30:48 +0000 (+0000) Subject: Consumed analysis: switch from tests_consumed/unconsumed to a general X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1bf6343612e394051fffc587e6899de6901065e0;p=clang Consumed analysis: switch from tests_consumed/unconsumed to a general tests_typestate attribute. Patch by chris.wailes@gmail.com. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@192513 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/Analyses/Consumed.h b/include/clang/Analysis/Analyses/Consumed.h index e6747bffe2..f097e3da7c 100644 --- a/include/clang/Analysis/Analyses/Consumed.h +++ b/include/clang/Analysis/Analyses/Consumed.h @@ -74,17 +74,6 @@ namespace consumed { virtual void warnReturnTypestateMismatch(SourceLocation Loc, StringRef ExpectedState, StringRef ObservedState) {} - - /// \brief Warn about unnecessary-test errors. - /// \param VariableName -- The name of the variable that holds the unique - /// value. - /// - /// \param VariableState -- The known state of the value. - /// - /// \param Loc -- The SourceLocation of the unnecessary test. - virtual void warnUnnecessaryTest(StringRef VariableName, - StringRef VariableState, - SourceLocation Loc) {} /// \brief Warn about use-while-consumed errors. /// \param MethodName -- The name of the method that was incorrectly diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td index dd87e3e8c0..4af914bf0c 100644 --- a/include/clang/Basic/Attr.td +++ b/include/clang/Basic/Attr.td @@ -967,14 +967,12 @@ def CallableWhen : InheritableAttr { ["Unknown", "Consumed", "Unconsumed"]>]; } -def TestsUnconsumed : InheritableAttr { - let Spellings = [GNU<"tests_unconsumed">]; - let Subjects = [CXXMethod]; -} - -def TestsConsumed : InheritableAttr { - let Spellings = [GNU<"tests_consumed">]; +def TestsTypestate : InheritableAttr { + let Spellings = [GNU<"tests_typestate">]; let Subjects = [CXXMethod]; + let Args = [EnumArgument<"TestState", "ConsumedState", + ["consumed", "unconsumed"], + ["Consumed", "Unconsumed"]>]; } def Consumes : InheritableAttr { diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index fb2092197d..57108058b5 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -506,7 +506,6 @@ def ThreadSafetyBeta : DiagGroup<"thread-safety-beta">; // Uniqueness Analysis warnings def Consumed : DiagGroup<"consumed">; -def ConsumedStrict : DiagGroup<"consumed-strict", [Consumed]>; // Note that putting warnings in -Wall will not disable them by default. If a // warning should be active _only_ when -Wall is passed in, mark it as diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 5980f2ce57..194389f547 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2216,6 +2216,8 @@ def warn_attr_on_unconsumable_class : Warning< def warn_return_typestate_for_unconsumable_type : Warning< "return state set for an unconsumable type '%0'">, InGroup, DefaultIgnore; +def warn_invalid_test_typestate : Warning< + "invalid test typestate '%0'">, InGroup, DefaultIgnore; def warn_return_typestate_mismatch : Warning< "return value not in expected state; expected '%0', observed '%1'">, InGroup, DefaultIgnore; @@ -2223,11 +2225,6 @@ def warn_loop_state_mismatch : Warning< "state of variable '%0' must match at the entry and exit of loop">, InGroup, DefaultIgnore; -// ConsumedStrict warnings -def warn_unnecessary_test : Warning< - "unnecessary test. Variable '%0' is known to be in the '%1' state">, - InGroup, DefaultIgnore; - def warn_impcast_vector_scalar : Warning< "implicit conversion turns vector to scalar: %0 to %1">, InGroup, DefaultIgnore; diff --git a/lib/Analysis/Consumed.cpp b/lib/Analysis/Consumed.cpp index 85dd821892..d1c03d091b 100644 --- a/lib/Analysis/Consumed.cpp +++ b/lib/Analysis/Consumed.cpp @@ -137,7 +137,7 @@ static bool isKnownState(ConsumedState State) { } static bool isTestingFunction(const FunctionDecl *FunDecl) { - return FunDecl->hasAttr(); + return FunDecl->hasAttr(); } static ConsumedState mapConsumableAttrState(const QualType QT) { @@ -187,6 +187,17 @@ static StringRef stateToString(ConsumedState State) { llvm_unreachable("invalid enum"); } +static ConsumedState testsFor(const FunctionDecl *FunDecl) { + assert(isTestingFunction(FunDecl)); + switch (FunDecl->getAttr()->getTestState()) { + case TestsTypestateAttr::Unconsumed: + return CS_Unconsumed; + case TestsTypestateAttr::Consumed: + return CS_Consumed; + } + llvm_unreachable("invalid enum"); +} + namespace { struct VarTestResult { const VarDecl *Var; @@ -341,7 +352,6 @@ class ConsumedStmtVisitor : public ConstStmtVisitor { ConsumedStateMap *StateMap; MapType PropagationMap; void forwardInfo(const Stmt *From, const Stmt *To); - void handleTestingFunctionCall(const CallExpr *Call, const VarDecl *Var); bool isLikeMoveAssignment(const CXXMethodDecl *MethodDecl); void propagateReturnType(const Stmt *Call, const FunctionDecl *Fun, QualType ReturnType); @@ -428,22 +438,6 @@ void ConsumedStmtVisitor::forwardInfo(const Stmt *From, const Stmt *To) { PropagationMap.insert(PairType(To, Entry->second)); } -void ConsumedStmtVisitor::handleTestingFunctionCall(const CallExpr *Call, - const VarDecl *Var) { - - ConsumedState VarState = StateMap->getState(Var); - - if (VarState != CS_Unknown) { - SourceLocation CallLoc = Call->getExprLoc(); - - if (!CallLoc.isMacroID()) - Analyzer.WarningsHandler.warnUnnecessaryTest(Var->getNameAsString(), - stateToString(VarState), CallLoc); - } - - PropagationMap.insert(PairType(Call, PropagationInfo(Var, CS_Unconsumed))); -} - bool ConsumedStmtVisitor::isLikeMoveAssignment( const CXXMethodDecl *MethodDecl) { @@ -643,7 +637,8 @@ void ConsumedStmtVisitor::VisitCXXMemberCallExpr( if (PInfo.isVar()) { if (isTestingFunction(MethodDecl)) - handleTestingFunctionCall(Call, PInfo.getVar()); + PropagationMap.insert(PairType(Call, + PropagationInfo(PInfo.getVar(), testsFor(MethodDecl)))); else if (MethodDecl->hasAttr()) StateMap->setState(PInfo.getVar(), consumed::CS_Consumed); } @@ -731,7 +726,8 @@ void ConsumedStmtVisitor::VisitCXXOperatorCallExpr( if (PInfo.isVar()) { if (isTestingFunction(FunDecl)) - handleTestingFunctionCall(Call, PInfo.getVar()); + PropagationMap.insert(PairType(Call, + PropagationInfo(PInfo.getVar(), testsFor(FunDecl)))); else if (FunDecl->hasAttr()) StateMap->setState(PInfo.getVar(), consumed::CS_Consumed); } diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp index 53e53b28f4..88b7dc4e5d 100644 --- a/lib/Sema/AnalysisBasedWarnings.cpp +++ b/lib/Sema/AnalysisBasedWarnings.cpp @@ -1501,15 +1501,6 @@ public: Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); } - void warnUnnecessaryTest(StringRef VariableName, StringRef VariableState, - SourceLocation Loc) { - - PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_unnecessary_test) << - VariableName << VariableState); - - Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); - } - void warnUseOfTempInInvalidState(StringRef MethodName, StringRef State, SourceLocation Loc) { diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index 1985de4c1b..a07d6ca79d 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -1079,8 +1079,11 @@ static void handleCallableWhenAttr(Sema &S, Decl *D, States.size(), Attr.getAttributeSpellingListIndex())); } -static void handleTestsConsumedAttr(Sema &S, Decl *D, - const AttributeList &Attr) { + +static void handleTestsTypestateAttr(Sema &S, Decl *D, + const AttributeList &Attr) { + if (!checkAttributeNumArgs(S, Attr, 1)) return; + if (!isa(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) << Attr.getName() << ExpectedMethod; @@ -1090,25 +1093,29 @@ static void handleTestsConsumedAttr(Sema &S, Decl *D, if (!checkForConsumableClass(S, cast(D), Attr)) return; - D->addAttr(::new (S.Context) - TestsConsumedAttr(Attr.getRange(), S.Context, - Attr.getAttributeSpellingListIndex())); -} - -static void handleTestsUnconsumedAttr(Sema &S, Decl *D, - const AttributeList &Attr) { - if (!isa(D)) { - S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) << - Attr.getName() << ExpectedMethod; - return; - } + TestsTypestateAttr::ConsumedState TestState; - if (!checkForConsumableClass(S, cast(D), Attr)) + 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) - TestsUnconsumedAttr(Attr.getRange(), S.Context, - Attr.getAttributeSpellingListIndex())); + TestsTypestateAttr(Attr.getRange(), S.Context, TestState, + Attr.getAttributeSpellingListIndex())); } static void handleReturnTypestateAttr(Sema &S, Decl *D, @@ -4782,7 +4789,7 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, handleAcquiredAfterAttr(S, D, Attr); break; - // Uniqueness analysis attributes. + // Consumed analysis attributes. case AttributeList::AT_Consumable: handleConsumableAttr(S, D, Attr); break; @@ -4792,11 +4799,8 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, case AttributeList::AT_CallableWhen: handleCallableWhenAttr(S, D, Attr); break; - case AttributeList::AT_TestsConsumed: - handleTestsConsumedAttr(S, D, Attr); - break; - case AttributeList::AT_TestsUnconsumed: - handleTestsUnconsumedAttr(S, D, Attr); + case AttributeList::AT_TestsTypestate: + handleTestsTypestateAttr(S, D, Attr); break; case AttributeList::AT_ReturnTypestate: handleReturnTypestateAttr(S, D, Attr); diff --git a/test/SemaCXX/warn-consumed-analysis-strict.cpp b/test/SemaCXX/warn-consumed-analysis-strict.cpp deleted file mode 100644 index 8ef3e8833b..0000000000 --- a/test/SemaCXX/warn-consumed-analysis-strict.cpp +++ /dev/null @@ -1,66 +0,0 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed-strict -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_UNCONSUMED __attribute__ ((tests_unconsumed)) - -#define TEST_VAR(Var) Var.isValid() - -typedef decltype(nullptr) nullptr_t; - -template -class CONSUMABLE(unconsumed) ConsumableClass { - T var; - - public: - ConsumableClass(); - ConsumableClass(nullptr_t p) RETURN_TYPESTATE(consumed); - ConsumableClass(T val); - ConsumableClass(ConsumableClass &other); - ConsumableClass(ConsumableClass &&other); - - ConsumableClass& operator=(ConsumableClass &other); - ConsumableClass& operator=(ConsumableClass &&other); - ConsumableClass& operator=(nullptr_t) CONSUMES; - - template - ConsumableClass& operator=(ConsumableClass &other); - - template - ConsumableClass& operator=(ConsumableClass &&other); - - void operator()(int a) CONSUMES; - void operator*() const CALLABLE_WHEN("unconsumed"); - void unconsumedCall() const CALLABLE_WHEN("unconsumed"); - - bool isValid() const TESTS_UNCONSUMED; - operator bool() const TESTS_UNCONSUMED; - bool operator!=(nullptr_t) const TESTS_UNCONSUMED; - - void constCall() const; - void nonconstCall(); - - void consume() CONSUMES; -}; - -void testIfStmt() { - ConsumableClass var; - - if (var.isValid()) { // expected-warning {{unnecessary test. Variable 'var' is known to be in the 'consumed' state}} - - // Empty - - } else { - // Empty - } -} - -void testNoWarnTestFromMacroExpansion() { - ConsumableClass var(42); - - if (TEST_VAR(var)) { - *var; - } -} diff --git a/test/SemaCXX/warn-consumed-analysis.cpp b/test/SemaCXX/warn-consumed-analysis.cpp index 28b7794789..b11a74f95e 100644 --- a/test/SemaCXX/warn-consumed-analysis.cpp +++ b/test/SemaCXX/warn-consumed-analysis.cpp @@ -6,7 +6,7 @@ #define CONSUMABLE(state) __attribute__ ((consumable(state))) #define CONSUMES __attribute__ ((consumes)) #define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state))) -#define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed)) +#define TESTS_TYPESTATE(state) __attribute__ ((tests_typestate(state))) typedef decltype(nullptr) nullptr_t; @@ -36,9 +36,10 @@ public: void unconsumedCall() const CALLABLE_WHEN("unconsumed"); void callableWhenUnknown() const CALLABLE_WHEN("unconsumed", "unknown"); - bool isValid() const TESTS_UNCONSUMED; - operator bool() const TESTS_UNCONSUMED; - bool operator!=(nullptr_t) const TESTS_UNCONSUMED; + bool isValid() const TESTS_TYPESTATE(unconsumed); + operator bool() const TESTS_TYPESTATE(unconsumed); + bool operator!=(nullptr_t) const TESTS_TYPESTATE(unconsumed); + bool operator==(nullptr_t) const TESTS_TYPESTATE(consumed); void constCall() const; void nonconstCall(); @@ -146,6 +147,12 @@ void testIfStmt() { } else { *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}} } + + if (var == nullptr) { + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}} + } else { + // Empty + } } void testComplexConditionals0() { diff --git a/test/SemaCXX/warn-consumed-parsing.cpp b/test/SemaCXX/warn-consumed-parsing.cpp index 153c3b79e4..0cc8579c94 100644 --- a/test/SemaCXX/warn-consumed-parsing.cpp +++ b/test/SemaCXX/warn-consumed-parsing.cpp @@ -4,7 +4,7 @@ #define CONSUMABLE(state) __attribute__ ((consumable(state))) #define CONSUMES __attribute__ ((consumes)) #define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state))) -#define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed)) +#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 @@ -18,18 +18,18 @@ int returnTypestateForUnconsumable() { class AttrTester0 { void consumes() __attribute__ ((consumes(42))); // expected-error {{attribute takes no arguments}} - bool testsUnconsumed() __attribute__ ((tests_unconsumed(42))); // expected-error {{attribute takes no arguments}} + 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 var1 TESTS_UNCONSUMED; // expected-warning {{'tests_unconsumed' 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 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 function1() TESTS_UNCONSUMED; // expected-warning {{'tests_unconsumed' 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 function3() CONSUMABLE(consumed); // expected-warning {{'consumable' attribute only applies to classes}} @@ -38,7 +38,7 @@ class CONSUMABLE(unknown) AttrTester1 { 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; - bool testsUnconsumed() TESTS_UNCONSUMED; + bool testsUnconsumed() TESTS_TYPESTATE(consumed); }; AttrTester1 returnTypestateTester0() RETURN_TYPESTATE(not_a_state); // expected-warning {{'return_typestate' attribute argument not supported: 'not_a_state'}} @@ -47,7 +47,7 @@ AttrTester1 returnTypestateTester1() RETURN_TYPESTATE(42); // expected-error {{' 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}} - bool testsUnconsumed() TESTS_UNCONSUMED; // 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}} }; class CONSUMABLE(42) AttrTester3; // expected-error {{'consumable' attribute requires an identifier}}