[PATCH] replaced callable_when_unconsumed annotation with generic callable_when annotation
Aaron Ballman
aaron.ballman at gmail.com
Fri Sep 6 09:45:22 PDT 2013
On Fri, Sep 6, 2013 at 10:34 AM, Aaron Ballman <aaron.ballman at gmail.com> wrote:
>> Index: include/clang/Analysis/Analyses/Consumed.h
>> ===================================================================
>> --- include/clang/Analysis/Analyses/Consumed.h
>> +++ include/clang/Analysis/Analyses/Consumed.h
>> @@ -25,6 +25,15 @@
>> namespace clang {
>> namespace consumed {
>>
>> + enum ConsumedState {
>> + // No state information for the given variable.
>> + CS_None,
>> +
>> + CS_Unknown,
>> + CS_Unconsumed,
>> + CS_Consumed
>> + };
>> +
>> class ConsumedStmtVisitor;
>>
>> typedef SmallVector<PartialDiagnosticAt, 1> OptionalNotes;
>> @@ -71,52 +80,29 @@
>> /// \param MethodName -- The name of the method that was incorrectly
>> /// invoked.
>> ///
>> - /// \param Loc -- The SourceLocation of the method invocation.
>> - virtual void warnUseOfTempWhileConsumed(StringRef MethodName,
>> - SourceLocation Loc) {}
>> -
>> - /// \brief Warn about use-in-unknown-state errors.
>> - /// \param MethodName -- The name of the method that was incorrectly
>> - /// invoked.
>> + /// \param State -- The state the object was used in.
>> ///
>> /// \param Loc -- The SourceLocation of the method invocation.
>> - virtual void warnUseOfTempInUnknownState(StringRef MethodName,
>> + virtual void warnUseOfTempInInvalidState(StringRef MethodName,
>> + StringRef State,
>> SourceLocation Loc) {}
>>
>> /// \brief Warn about use-while-consumed errors.
>> /// \param MethodName -- The name of the method that was incorrectly
>> /// invoked.
>> ///
>> - /// \param VariableName -- The name of the variable that holds the unique
>> - /// value.
>> - ///
>> - /// \param Loc -- The SourceLocation of the method invocation.
>> - virtual void warnUseWhileConsumed(StringRef MethodName,
>> - StringRef VariableName,
>> - SourceLocation Loc) {}
>> -
>> - /// \brief Warn about use-in-unknown-state errors.
>> - /// \param MethodName -- The name of the method that was incorrectly
>> - /// invoked.
>> + /// \param State -- The state the object was used in.
>> ///
>> /// \param VariableName -- The name of the variable that holds the unique
>> /// value.
>> ///
>> /// \param Loc -- The SourceLocation of the method invocation.
>> - virtual void warnUseInUnknownState(StringRef MethodName,
>> + virtual void warnUseInInvalidState(StringRef MethodName,
>> StringRef VariableName,
>> + StringRef State,
>> SourceLocation Loc) {}
>
> This is a bit late in coming, but why does ConsumedWarningsHandlerBase
> exist at all? There is only one derivation of it, and that implements
> all of the virtual functions.
>
>> };
>>
>> - enum ConsumedState {
>> - // No state information for the given variable.
>> - CS_None,
>> -
>> - CS_Unknown,
>> - CS_Unconsumed,
>> - CS_Consumed
>> - };
>> -
>> class ConsumedStateMap {
>>
>> typedef llvm::DenseMap<const VarDecl *, ConsumedState> MapType;
>> Index: include/clang/Basic/Attr.td
>> ===================================================================
>> --- include/clang/Basic/Attr.td
>> +++ include/clang/Basic/Attr.td
>> @@ -55,6 +55,9 @@
>> class TypeArgument<string name> : Argument<name>;
>> class UnsignedArgument<string name> : Argument<name>;
>> class SourceLocArgument<string name> : Argument<name>;
>> +
>> +// FIXME: There should be a VariadicArgument type that takes any other type
>> +// of argument and generates the appropriate type.
>
> Why? Not that I disagree, but what does that have to do with the current patch?
>
>> class VariadicUnsignedArgument<string name> : Argument<name>;
>> class VariadicExprArgument<string name> : Argument<name>;
>>
>> @@ -80,6 +83,13 @@
>> list<string> Enums = enums;
>> }
>>
>> +class VariadicEnumArgument<string name, string type, list<string> values,
>> + list<string> enums> : Argument<name> {
>> + string Type = type;
>> + list<string> Values = values;
>> + list<string> Enums = enums;
>> +}
>> +
>> // This handles one spelling of an attribute.
>> class Spelling<string name, string variety> {
>> string Name = name;
>> @@ -936,9 +946,12 @@
>> ["Unknown", "Consumed", "Unconsumed"]>];
>> }
>>
>> -def CallableWhenUnconsumed : InheritableAttr {
>> - let Spellings = [GNU<"callable_when_unconsumed">];
>> +def CallableWhen : InheritableAttr {
>> + let Spellings = [GNU<"callable_when">];
>> let Subjects = [CXXMethod];
>> + let Args = [VariadicEnumArgument<"CallableState", "ConsumedState",
>> + ["unknown", "consumed", "unconsumed"],
>> + ["Unknown", "Consumed", "Unconsumed"]>];
>> }
>>
>> def TestsUnconsumed : InheritableAttr {
>> Index: include/clang/Basic/DiagnosticSemaKinds.td
>> ===================================================================
>> --- include/clang/Basic/DiagnosticSemaKinds.td
>> +++ include/clang/Basic/DiagnosticSemaKinds.td
>> @@ -2188,12 +2188,12 @@
>> "Thread safety beta warning.">, InGroup<ThreadSafetyBeta>, DefaultIgnore;
>>
>> // Consumed warnings
>> -def warn_use_while_consumed : Warning<
>> - "invocation of method '%0' on object '%1' while it is in the 'consumed' "
>> +def warn_use_in_invalid_state : Warning<
>> + "invalid invocation of method '%0' on object '%1' while it is in the '%2' "
>> "state">, InGroup<Consumed>, DefaultIgnore;
>> -def warn_use_of_temp_while_consumed : Warning<
>> - "invocation of method '%0' on a temporary object while it is in the "
>> - "'consumed' state">, InGroup<Consumed>, DefaultIgnore;
>> +def warn_use_of_temp_in_invalid_state : Warning<
>> + "invalid invocation of method '%0' on a temporary object while it is in the "
>> + "'%1' state">, InGroup<Consumed>, DefaultIgnore;
>> 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;
>> @@ -2207,12 +2207,6 @@
>> InGroup<Consumed>, DefaultIgnore;
>>
>> // ConsumedStrict warnings
>> -def warn_use_in_unknown_state : Warning<
>> - "invocation of method '%0' on object '%1' while it is in an unknown state">,
>> - InGroup<ConsumedStrict>, DefaultIgnore;
>> -def warn_use_of_temp_in_unknown_state : Warning<
>> - "invocation of method '%0' on a temporary object while it is in an unknown "
>> - "state">, InGroup<ConsumedStrict>, DefaultIgnore;
>> def warn_unnecessary_test : Warning<
>> "unnecessary test. Variable '%0' is known to be in the '%1' state">,
>> InGroup<ConsumedStrict>, DefaultIgnore;
>> Index: lib/Analysis/Consumed.cpp
>> ===================================================================
>> --- lib/Analysis/Consumed.cpp
>> +++ lib/Analysis/Consumed.cpp
>> @@ -33,6 +33,8 @@
>>
>> // TODO: Add notes about the actual and expected state for
>> // TODO: Correctly identify unreachable blocks when chaining boolean operators.
>> +// TODO: Adjust the parser and AttributesList class to support lists of
>> +// identifiers.
>> // TODO: Warn about unreachable code.
>> // TODO: Switch to using a bitmap to track unreachable blocks.
>> // TODO: Mark variables as Unknown going into while- or for-loops only if they
>> @@ -66,6 +68,37 @@
>> llvm_unreachable("invalid enum");
>> }
>>
>> +static bool isCallableInState(const CallableWhenAttr *CWAttr,
>> + ConsumedState State) {
>> +
>> + CallableWhenAttr::callableState_iterator I = CWAttr->callableState_begin(),
>> + E = CWAttr->callableState_end();
>> +
>> + for (; I != E; ++I) {
>> +
>> + ConsumedState MappedAttrState = CS_None;
>> +
>> + switch (*I) {
>> + case CallableWhenAttr::Unknown:
>> + MappedAttrState = CS_Unknown;
>> + break;
>> +
>> + case CallableWhenAttr::Unconsumed:
>> + MappedAttrState = CS_Unconsumed;
>> + break;
>> +
>> + case CallableWhenAttr::Consumed:
>> + MappedAttrState = CS_Consumed;
>> + break;
>> + }
>> +
>> + if (MappedAttrState == State)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>
> Can't this loop be replaced with a standard algorithm? It smells an
> awful lot like a std::find_if.
>
>> +
>> static bool isConsumableType(const QualType &QT) {
>> if (const CXXRecordDecl *RD = QT->getAsCXXRecordDecl())
>> return RD->hasAttr<ConsumableAttr>();
>> @@ -174,6 +207,8 @@
>> BinTestTy BinTest;
>> };
>>
>> + QualType TempType;
>> +
>> public:
>> PropagationInfo() : InfoType(IT_None) {}
>>
>> @@ -208,7 +243,9 @@
>> BinTest.RTest.TestsFor = RTestsFor;
>> }
>>
>> - PropagationInfo(ConsumedState State) : InfoType(IT_State), State(State) {}
>> + PropagationInfo(ConsumedState State, QualType TempType)
>> + : InfoType(IT_State), State(State), TempType(TempType) {}
>> +
>> PropagationInfo(const VarDecl *Var) : InfoType(IT_Var), Var(Var) {}
>>
>> const ConsumedState & getState() const {
>> @@ -216,6 +253,11 @@
>> return State;
>> }
>>
>> + const QualType & getTempType() const {
>
> Formatting.
>
>> + assert(InfoType == IT_State);
>> + return TempType;
>> + }
>> +
>> const VarTestResult & getTest() const {
>> assert(InfoType == IT_Test);
>> return Test;
>> @@ -327,51 +369,36 @@
>> }
>> };
>>
>> -// TODO: When we support CallableWhenConsumed this will have to check for
>> -// the different attributes and change the behavior bellow. (Deferred)
>> void ConsumedStmtVisitor::checkCallability(const PropagationInfo &PInfo,
>> const FunctionDecl *FunDecl,
>> const CallExpr *Call) {
>>
>> - if (!FunDecl->hasAttr<CallableWhenUnconsumedAttr>()) return;
>> + if (!FunDecl->hasAttr<CallableWhenAttr>()) return;
>
> Formatting (I know the original code had the same formatting, but the
> statement should go on a newline).
>
>> + const CallableWhenAttr *CWAttr = FunDecl->getAttr<CallableWhenAttr>();
>>
>> if (PInfo.isVar()) {
>> const VarDecl *Var = PInfo.getVar();
>> + ConsumedState VarState = StateMap->getState(Var);
>>
>> - switch (StateMap->getState(Var)) {
>> - case CS_Consumed:
>> - Analyzer.WarningsHandler.warnUseWhileConsumed(
>> - FunDecl->getNameAsString(), Var->getNameAsString(),
>> - Call->getExprLoc());
>> - break;
>> + assert(VarState != CS_None && "Invalid state");
>>
>> - case CS_Unknown:
>> - Analyzer.WarningsHandler.warnUseInUnknownState(
>> - FunDecl->getNameAsString(), Var->getNameAsString(),
>> - Call->getExprLoc());
>> - break;
>> -
>> - case CS_None:
>> - case CS_Unconsumed:
>> - break;
>> - }
>> + if (isCallableInState(CWAttr, VarState))
>> + return;
>>
>> - } else {
>> - switch (PInfo.getState()) {
>> - case CS_Consumed:
>> - Analyzer.WarningsHandler.warnUseOfTempWhileConsumed(
>> - FunDecl->getNameAsString(), Call->getExprLoc());
>> - break;
>> + Analyzer.WarningsHandler.warnUseInInvalidState(
>> + FunDecl->getNameAsString(), Var->getNameAsString(),
>> + stateToString(VarState), Call->getExprLoc());
>>
>> - case CS_Unknown:
>> - Analyzer.WarningsHandler.warnUseOfTempInUnknownState(
>> - FunDecl->getNameAsString(), Call->getExprLoc());
>> - break;
>> -
>> - case CS_None:
>> - case CS_Unconsumed:
>> - break;
>> - }
>> + } else if (PInfo.isState()) {
>> +
>> + assert(PInfo.getState() != CS_None && "Invalid state");
>> +
>> + if (isCallableInState(CWAttr, PInfo.getState()))
>> + return;
>> +
>> + Analyzer.WarningsHandler.warnUseOfTempInInvalidState(
>> + FunDecl->getNameAsString(), stateToString(PInfo.getState()),
>> + Call->getExprLoc());
>> }
>> }
>>
>> @@ -421,7 +448,7 @@
>> ReturnState = mapConsumableAttrState(ReturnType);
>>
>> PropagationMap.insert(PairType(Call,
>> - PropagationInfo(ReturnState)));
>> + PropagationInfo(ReturnState, ReturnType)));
>> }
>> }
>>
>> @@ -522,7 +549,11 @@
>> }
>> }
>>
>> - propagateReturnType(Call, FunDecl, FunDecl->getCallResultType());
>> + QualType RetType = FunDecl->getCallResultType();
>> + if (RetType->isReferenceType())
>
> What about pointer types?
>
>> + RetType = RetType->getPointeeType();
>> +
>> + propagateReturnType(Call, FunDecl, RetType);
>> }
>> }
>>
>> @@ -540,7 +571,7 @@
>> if (Constructor->isDefaultConstructor()) {
>>
>> PropagationMap.insert(PairType(Call,
>> - PropagationInfo(consumed::CS_Consumed)));
>> + PropagationInfo(consumed::CS_Consumed, ThisType)));
>>
>> } else if (Constructor->isMoveConstructor()) {
>>
>> @@ -551,7 +582,7 @@
>> const VarDecl* Var = PInfo.getVar();
>>
>> PropagationMap.insert(PairType(Call,
>> - PropagationInfo(StateMap->getState(Var))));
>> + PropagationInfo(StateMap->getState(Var), ThisType)));
>>
>> StateMap->setState(Var, consumed::CS_Consumed);
>>
>> @@ -630,7 +661,8 @@
>>
>> } else if (!LPInfo.isVar() && RPInfo.isVar()) {
>> PropagationMap.insert(PairType(Call,
>> - PropagationInfo(StateMap->getState(RPInfo.getVar()))));
>> + PropagationInfo(StateMap->getState(RPInfo.getVar()),
>> + LPInfo.getTempType())));
>>
>> StateMap->setState(RPInfo.getVar(), consumed::CS_Consumed);
>>
>> @@ -648,27 +680,16 @@
>>
>> PropagationMap.insert(PairType(Call, LPInfo));
>>
>> - } else {
>> + } else if (LPInfo.isState()) {
>> PropagationMap.insert(PairType(Call,
>> - PropagationInfo(consumed::CS_Unknown)));
>> + PropagationInfo(consumed::CS_Unknown, LPInfo.getTempType())));
>> }
>>
>> } else if (LEntry == PropagationMap.end() &&
>> REntry != PropagationMap.end()) {
>>
>> - RPInfo = REntry->second;
>> -
>> - if (RPInfo.isVar()) {
>> - const VarDecl *Var = RPInfo.getVar();
>> -
>> - PropagationMap.insert(PairType(Call,
>> - PropagationInfo(StateMap->getState(Var))));
>> -
>> - StateMap->setState(Var, consumed::CS_Consumed);
>> -
>> - } else {
>> - PropagationMap.insert(PairType(Call, RPInfo));
>> - }
>> + if (REntry->second.isVar())
>> + StateMap->setState(REntry->second.getVar(), consumed::CS_Consumed);
>> }
>>
>> } else {
>> @@ -776,6 +797,7 @@
>> }
>> }
>>
>> +// TODO: See if I need to check for reference types here.
>> void ConsumedStmtVisitor::VisitVarDecl(const VarDecl *Var) {
>> if (isConsumableType(Var->getType())) {
>> if (Var->hasInit()) {
>> @@ -803,13 +825,12 @@
>>
>> if (VarState == CS_Unknown) {
>> ThenStates->setState(Test.Var, Test.TestsFor);
>> - if (ElseStates)
>> - ElseStates->setState(Test.Var, invertConsumedUnconsumed(Test.TestsFor));
>> + ElseStates->setState(Test.Var, invertConsumedUnconsumed(Test.TestsFor));
>>
>> } else if (VarState == invertConsumedUnconsumed(Test.TestsFor)) {
>> ThenStates->markUnreachable();
>>
>> - } else if (VarState == Test.TestsFor && ElseStates) {
>> + } else if (VarState == Test.TestsFor) {
>> ElseStates->markUnreachable();
>> }
>> }
>> @@ -832,31 +853,27 @@
>> ThenStates->markUnreachable();
>>
>> } else if (LState == LTest.TestsFor && isKnownState(RState)) {
>> - if (RState == RTest.TestsFor) {
>> - if (ElseStates)
>> - ElseStates->markUnreachable();
>> - } else {
>> + if (RState == RTest.TestsFor)
>> + ElseStates->markUnreachable();
>> + else
>> ThenStates->markUnreachable();
>> - }
>> }
>>
>> } else {
>> - if (LState == CS_Unknown && ElseStates) {
>> + if (LState == CS_Unknown) {
>> ElseStates->setState(LTest.Var,
>> invertConsumedUnconsumed(LTest.TestsFor));
>>
>> - } else if (LState == LTest.TestsFor && ElseStates) {
>> + } else if (LState == LTest.TestsFor) {
>> ElseStates->markUnreachable();
>>
>> } else if (LState == invertConsumedUnconsumed(LTest.TestsFor) &&
>> isKnownState(RState)) {
>>
>> - if (RState == RTest.TestsFor) {
>> - if (ElseStates)
>> - ElseStates->markUnreachable();
>> - } else {
>> + if (RState == RTest.TestsFor)
>> + ElseStates->markUnreachable();
>> + else
>> ThenStates->markUnreachable();
>> - }
>> }
>> }
>> }
>> @@ -868,7 +885,7 @@
>> else if (RState == invertConsumedUnconsumed(RTest.TestsFor))
>> ThenStates->markUnreachable();
>>
>> - } else if (ElseStates) {
>> + } else {
>> if (RState == CS_Unknown)
>> ElseStates->setState(RTest.Var,
>> invertConsumedUnconsumed(RTest.TestsFor));
>> @@ -1016,7 +1033,6 @@
>> if (const IfStmt *IfNode =
>> dyn_cast_or_null<IfStmt>(CurrBlock->getTerminator().getStmt())) {
>>
>> - bool HasElse = IfNode->getElse() != NULL;
>> const Stmt *Cond = IfNode->getCond();
>>
>> PInfo = Visitor.getInfo(Cond);
>> @@ -1026,15 +1042,12 @@
>> if (PInfo.isTest()) {
>> CurrStates->setSource(Cond);
>> FalseStates->setSource(Cond);
>> -
>> - splitVarStateForIf(IfNode, PInfo.getTest(), CurrStates,
>> - HasElse ? FalseStates : NULL);
>> + splitVarStateForIf(IfNode, PInfo.getTest(), CurrStates, FalseStates);
>>
>> } else if (PInfo.isBinTest()) {
>> CurrStates->setSource(PInfo.testSourceNode());
>> FalseStates->setSource(PInfo.testSourceNode());
>> -
>> - splitVarStateForIfBinOp(PInfo, CurrStates, HasElse ? FalseStates : NULL);
>> + splitVarStateForIfBinOp(PInfo, CurrStates, FalseStates);
>>
>> } else {
>> delete FalseStates;
>> Index: lib/Sema/AnalysisBasedWarnings.cpp
>> ===================================================================
>> --- lib/Sema/AnalysisBasedWarnings.cpp
>> +++ lib/Sema/AnalysisBasedWarnings.cpp
>> @@ -1471,36 +1471,20 @@
>> Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
>> }
>>
>> - void warnUseOfTempWhileConsumed(StringRef MethodName, SourceLocation Loc) {
>> + void warnUseOfTempInInvalidState(StringRef MethodName, StringRef State,
>> + SourceLocation Loc) {
>>
>> PartialDiagnosticAt Warning(Loc, S.PDiag(
>> - diag::warn_use_of_temp_while_consumed) << MethodName);
>> + diag::warn_use_of_temp_in_invalid_state) << MethodName << State);
>>
>> Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
>> }
>>
>> - void warnUseOfTempInUnknownState(StringRef MethodName, SourceLocation Loc) {
>> + void warnUseInInvalidState(StringRef MethodName, StringRef VariableName,
>> + StringRef State, SourceLocation Loc) {
>>
>> - PartialDiagnosticAt Warning(Loc, S.PDiag(
>> - diag::warn_use_of_temp_in_unknown_state) << MethodName);
>> -
>> - Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
>> - }
>> -
>> - void warnUseWhileConsumed(StringRef MethodName, StringRef VariableName,
>> - SourceLocation Loc) {
>> -
>> - PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_use_while_consumed) <<
>> - MethodName << VariableName);
>> -
>> - Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
>> - }
>> -
>> - void warnUseInUnknownState(StringRef MethodName, StringRef VariableName,
>> - SourceLocation Loc) {
>> -
>> - PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_use_in_unknown_state) <<
>> - MethodName << VariableName);
>> + PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_use_in_invalid_state) <<
>> + MethodName << VariableName << State);
>>
>> Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
>> }
>> @@ -1538,7 +1522,7 @@
>> (D.getDiagnosticLevel(diag::warn_double_lock, SourceLocation()) !=
>> DiagnosticsEngine::Ignored);
>> DefaultPolicy.enableConsumedAnalysis = (unsigned)
>> - (D.getDiagnosticLevel(diag::warn_use_while_consumed, SourceLocation()) !=
>> + (D.getDiagnosticLevel(diag::warn_use_in_invalid_state, SourceLocation()) !=
>> DiagnosticsEngine::Ignored);
>> }
>>
>> Index: lib/Sema/SemaDeclAttr.cpp
>> ===================================================================
>> --- lib/Sema/SemaDeclAttr.cpp
>> +++ lib/Sema/SemaDeclAttr.cpp
>> @@ -1038,9 +1038,10 @@
>> Attr.getAttributeSpellingListIndex()));
>> }
>>
>> -static void handleCallableWhenUnconsumedAttr(Sema &S, Decl *D,
>> - const AttributeList &Attr) {
>> - if (!checkAttributeNumArgs(S, Attr, 0)) return;
>> +static void handleCallableWhenAttr(Sema &S, Decl *D,
>> + const AttributeList &Attr) {
>> +
>> + if (!checkAttributeAtLeastNumArgs(S, Attr, 1)) return;
>>
>> if (!isa<CXXMethodDecl>(D)) {
>> S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
>> @@ -1051,9 +1052,38 @@
>> if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
>> return;
>>
>> + SmallVector<CallableWhenAttr::ConsumedState, 3> States;
>> + for (unsigned ArgIndex = 0; ArgIndex < Attr.getNumArgs(); ++ArgIndex) {
>> + CallableWhenAttr::ConsumedState CallableState;
>> +
>> + if (Attr.isArgExpr(ArgIndex) &&
>> + isa<StringLiteral>(Attr.getArgAsExpr(ArgIndex))) {
>> +
>> + Expr *Arg = Attr.getArgAsExpr(ArgIndex);
>> + StringRef StateString = cast<StringLiteral>(Arg)->getString();
>
> I don't think it should be taking string literals; that's inconsistent
> with other consumable-related attributes. It should be taking a list
> of unresolved identifiers.
>
>> +
>> + if (StateString == "unknown") {
>> + CallableState = CallableWhenAttr::Unknown;
>> + } else if (StateString == "consumed") {
>> + CallableState = CallableWhenAttr::Consumed;
>> + } else if (StateString == "unconsumed") {
>> + CallableState = CallableWhenAttr::Unconsumed;
>> + } else {
>> + S.Diag(Attr.getLoc(), diag::warn_unknown_consumed_state);
>> + return;
>> + }
>
> Formatting (the braces should be removed); might make sense to use a
> StringSwitch. Actually, the more I think of it, the more I think we
> should have this sort of thing auto generated. Every attribute
> enumeration is going to have the same general notion...
>
>> +
>> + States.push_back(CallableState);
>> + } else {
>> + S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) << Attr.getName()
>> + << AANT_ArgumentString;
>
> When the string literals are fixed, this should be changed to identifier.
>
>> + return;
>> + }
>> + }
>> +
>> D->addAttr(::new (S.Context)
>> - CallableWhenUnconsumedAttr(Attr.getRange(), S.Context,
>> - Attr.getAttributeSpellingListIndex()));
>> + CallableWhenAttr(Attr.getRange(), S.Context, States.data(),
>> + States.size(), Attr.getAttributeSpellingListIndex()));
>> }
>>
>> static void handleTestsConsumedAttr(Sema &S, Decl *D,
>> @@ -5100,8 +5130,8 @@
>> case AttributeList::AT_Consumes:
>> handleConsumesAttr(S, D, Attr);
>> break;
>> - case AttributeList::AT_CallableWhenUnconsumed:
>> - handleCallableWhenUnconsumedAttr(S, D, Attr);
>> + case AttributeList::AT_CallableWhen:
>> + handleCallableWhenAttr(S, D, Attr);
>> break;
>> case AttributeList::AT_TestsConsumed:
>> handleTestsConsumedAttr(S, D, Attr);
>> Index: test/SemaCXX/warn-consumed-analysis-strict.cpp
>> ===================================================================
>> --- test/SemaCXX/warn-consumed-analysis-strict.cpp
>> +++ test/SemaCXX/warn-consumed-analysis-strict.cpp
>> @@ -1,10 +1,10 @@
>> // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed-strict -std=c++11 %s
>>
>> -#define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed))
>> -#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 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()
>>
>> @@ -32,8 +32,8 @@
>> ConsumableClass<T>& operator=(ConsumableClass<U> &&other);
>>
>> void operator()(int a) CONSUMES;
>> - void operator*() const CALLABLE_WHEN_UNCONSUMED;
>> - void unconsumedCall() const CALLABLE_WHEN_UNCONSUMED;
>> + void operator*() const CALLABLE_WHEN("unconsumed");
>> + void unconsumedCall() const CALLABLE_WHEN("unconsumed");
>>
>> bool isValid() const TESTS_UNCONSUMED;
>> operator bool() const TESTS_UNCONSUMED;
>> @@ -45,9 +45,6 @@
>> void consume() CONSUMES;
>> };
>>
>> -void baf0(ConsumableClass<int> &var);
>> -void baf1(ConsumableClass<int> *var);
>> -
>> void testIfStmt() {
>> ConsumableClass<int> var;
>>
>> @@ -60,137 +57,6 @@
>> }
>> }
>>
>> -void testComplexConditionals() {
>> - ConsumableClass<int> var0, var1, var2;
>> -
>> - // Coerce all variables into the unknown state.
>> - baf0(var0);
>> - baf0(var1);
>> - baf0(var2);
>> -
>> - if (var0 && var1) {
>> - *var0;
>> - *var1;
>> -
>> - } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}
>> - }
>> -
>> - if (var0 || var1) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}
>> -
>> - } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> - }
>> -
>> - if (var0 && !var1) {
>> - *var0;
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> -
>> - } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}
>> - }
>> -
>> - if (var0 || !var1) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}
>> -
>> - } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1;
>> - }
>> -
>> - if (!var0 && !var1) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> -
>> - } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}
>> - }
>> -
>> - if (!(var0 || var1)) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> -
>> - } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}
>> - }
>> -
>> - if (!var0 || !var1) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}
>> -
>> - } else {
>> - *var0;
>> - *var1;
>> - }
>> -
>> - if (!(var0 && var1)) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}
>> -
>> - } else {
>> - *var0;
>> - *var1;
>> - }
>> -
>> - if (var0 && var1 && var2) {
>> - *var0;
>> - *var1;
>> - *var2;
>> -
>> - } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}
>> - *var2; // expected-warning {{invocation of method 'operator*' on object 'var2' while it is in an unknown state}}
>> - }
>> -
>> -#if 0
>> - // FIXME: Get this test to pass.
>> - if (var0 || var1 || var2) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}
>> - *var2; // expected-warning {{invocation of method 'operator*' on object 'var2' while it is in an unknown state}}
>> -
>> - } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> - *var2; // expected-warning {{invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
>> - }
>> -#endif
>> -}
>> -
>> -void testCallingConventions() {
>> - ConsumableClass<int> var(42);
>> -
>> - baf0(var);
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}
>> -
>> - var = ConsumableClass<int>(42);
>> - baf1(&var);
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}
>> -}
>> -
>> -void testConstAndNonConstMemberFunctions() {
>> - ConsumableClass<int> var(42);
>> -
>> - var.constCall();
>> - *var;
>> -
>> - var.nonconstCall();
>> - *var;
>> -}
>> -
>> -void testFunctionParam0(ConsumableClass<int> ¶m) {
>> - *param; // expected-warning {{invocation of method 'operator*' on object 'param' while it is in an unknown state}}
>> -}
>> -
>> void testNoWarnTestFromMacroExpansion() {
>> ConsumableClass<int> var(42);
>>
>> @@ -198,26 +64,3 @@
>> *var;
>> }
>> }
>> -
>> -void testSimpleForLoop() {
>> - ConsumableClass<int> var;
>> -
>> - for (int i = 0; i < 10; ++i) {
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}
>> - }
>> -
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}
>> -}
>> -
>> -void testSimpleWhileLoop() {
>> - int i = 0;
>> -
>> - ConsumableClass<int> var;
>> -
>> - while (i < 10) {
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}
>> - ++i;
>> - }
>> -
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}
>> -}
>> Index: test/SemaCXX/warn-consumed-analysis.cpp
>> ===================================================================
>> --- test/SemaCXX/warn-consumed-analysis.cpp
>> +++ test/SemaCXX/warn-consumed-analysis.cpp
>> @@ -1,10 +1,12 @@
>> // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s
>>
>> -#define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed))
>> -#define CONSUMABLE(state) __attribute__ ((consumable(state)))
>> -#define CONSUMES __attribute__ ((consumes))
>> -#define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))
>> -#define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed))
>> +// TODO: Switch to using macros for the expected warnings.
>> +
>> +#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))
>>
>> typedef decltype(nullptr) nullptr_t;
>>
>> @@ -30,8 +32,9 @@
>> ConsumableClass<T>& operator=(ConsumableClass<U> &&other);
>>
>> void operator()(int a) CONSUMES;
>> - void operator*() const CALLABLE_WHEN_UNCONSUMED;
>> - void unconsumedCall() const CALLABLE_WHEN_UNCONSUMED;
>> + void operator*() const CALLABLE_WHEN("unconsumed");
>> + void unconsumedCall() const CALLABLE_WHEN("unconsumed");
>> + void callableWhenUnknown() const CALLABLE_WHEN("unconsumed", "unknown");
>>
>> bool isValid() const TESTS_UNCONSUMED;
>> operator bool() const TESTS_UNCONSUMED;
>> @@ -47,7 +50,9 @@
>> void baf1(const ConsumableClass<int> &var);
>> void baf2(const ConsumableClass<int> *var);
>>
>> -void baf3(ConsumableClass<int> &&var);
>> +void baf3(ConsumableClass<int> &var);
>> +void baf4(ConsumableClass<int> *var);
>> +void baf5(ConsumableClass<int> &&var);
>>
>> ConsumableClass<int> returnsUnconsumed() {
>> return ConsumableClass<int>(); // expected-warning {{return value not in expected state; expected 'unconsumed', observed 'consumed'}}
>> @@ -58,39 +63,41 @@
>> return ConsumableClass<int>();
>> }
>>
>> +ConsumableClass<int> returnsUnknown() RETURN_TYPESTATE(unknown);
>> +
>> void testInitialization() {
>> ConsumableClass<int> var0;
>> ConsumableClass<int> var1 = ConsumableClass<int>();
>>
>> var0 = ConsumableClass<int>();
>>
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>>
>> if (var0.isValid()) {
>> *var0;
>> *var1;
>>
>> } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> }
>> }
>>
>> void testTempValue() {
>> - *ConsumableClass<int>(); // expected-warning {{invocation of method 'operator*' on a temporary object while it is in the 'consumed' state}}
>> + *ConsumableClass<int>(); // expected-warning {{invalid invocation of method 'operator*' on a temporary object while it is in the 'consumed' state}}
>> }
>>
>> void testSimpleRValueRefs() {
>> ConsumableClass<int> var0;
>> ConsumableClass<int> var1(42);
>>
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> *var1;
>>
>> var0 = static_cast<ConsumableClass<int>&&>(var1);
>>
>> *var0;
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> }
>>
>> void testIfStmt() {
>> @@ -99,11 +106,11 @@
>> if (var.isValid()) {
>> *var;
>> } else {
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> }
>>
>> if (!var.isValid()) {
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> } else {
>> *var;
>> }
>> @@ -111,17 +118,17 @@
>> if (var) {
>> // Empty
>> } else {
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> }
>>
>> if (var != nullptr) {
>> // Empty
>> } else {
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> }
>> }
>>
>> -void testComplexConditionals() {
>> +void testComplexConditionals0() {
>> ConsumableClass<int> var0, var1, var2;
>>
>> if (var0 && var1) {
>> @@ -129,8 +136,8 @@
>> *var1;
>>
>> } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> }
>>
>> if (var0 || var1) {
>> @@ -138,8 +145,8 @@
>> *var1;
>>
>> } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> }
>>
>> if (var0 && !var1) {
>> @@ -147,13 +154,13 @@
>> *var1;
>>
>> } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> }
>>
>> if (var0 || !var1) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>>
>> } else {
>> *var0;
>> @@ -161,8 +168,8 @@
>> }
>>
>> if (!var0 && !var1) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>>
>> } else {
>> *var0;
>> @@ -170,8 +177,8 @@
>> }
>>
>> if (!var0 || !var1) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>>
>> } else {
>> *var0;
>> @@ -179,8 +186,8 @@
>> }
>>
>> if (!(var0 && var1)) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>>
>> } else {
>> *var0;
>> @@ -188,8 +195,8 @@
>> }
>>
>> if (!(var0 || var1)) {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>>
>> } else {
>> *var0;
>> @@ -202,9 +209,9 @@
>> *var2;
>>
>> } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> - *var2; // expected-warning {{invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
>> }
>>
>> #if 0
>> @@ -215,9 +222,115 @@
>> *var2;
>>
>> } else {
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> - *var2; // expected-warning {{invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
>> + }
>> +#endif
>> +}
>> +
>> +void testComplexConditionals1() {
>> + ConsumableClass<int> var0, var1, var2;
>> +
>> + // Coerce all variables into the unknown state.
>> + baf3(var0);
>> + baf3(var1);
>> + baf3(var2);
>> +
>> + if (var0 && var1) {
>> + *var0;
>> + *var1;
>> +
>> + } else {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
>> + }
>> +
>> + if (var0 || var1) {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
>> +
>> + } else {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + }
>> +
>> + if (var0 && !var1) {
>> + *var0;
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> +
>> + } else {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
>> + }
>> +
>> + if (var0 || !var1) {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
>> +
>> + } else {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1;
>> + }
>> +
>> + if (!var0 && !var1) {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> +
>> + } else {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
>> + }
>> +
>> + if (!(var0 || var1)) {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> +
>> + } else {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
>> + }
>> +
>> + if (!var0 || !var1) {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
>> +
>> + } else {
>> + *var0;
>> + *var1;
>> + }
>> +
>> + if (!(var0 && var1)) {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
>> +
>> + } else {
>> + *var0;
>> + *var1;
>> + }
>> +
>> + if (var0 && var1 && var2) {
>> + *var0;
>> + *var1;
>> + *var2;
>> +
>> + } else {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
>> + *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'unknown' state}}
>> + }
>> +
>> +#if 0
>> + // FIXME: Get this test to pass.
>> + if (var0 || var1 || var2) {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}
>> + *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'unknown' state}}
>> +
>> + } else {
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
>> }
>> #endif
>> }
>> @@ -226,7 +339,7 @@
>> ConsumableClass<int> var;
>>
>> // Make var enter the 'unknown' state.
>> - baf1(var);
>> + baf3(var);
>>
>> if (!var) {
>> var = ConsumableClass<int>(42);
>> @@ -259,8 +372,34 @@
>> baf2(&var);
>> *var;
>>
>> - baf3(static_cast<ConsumableClass<int>&&>(var));
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> + baf3(var);
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
>> +
>> + var = ConsumableClass<int>(42);
>> + baf4(&var);
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
>> +
>> + var = ConsumableClass<int>(42);
>> + baf5(static_cast<ConsumableClass<int>&&>(var));
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> +}
>> +
>> +void testConstAndNonConstMemberFunctions() {
>> + ConsumableClass<int> var(42);
>> +
>> + var.constCall();
>> + *var;
>> +
>> + var.nonconstCall();
>> + *var;
>> +}
>> +
>> +void testFunctionParam0(ConsumableClass<int> param) {
>> + *param;
>> +}
>> +
>> +void testFunctionParam1(ConsumableClass<int> ¶m) {
>> + *param; // expected-warning {{invalid invocation of method 'operator*' on object 'param' while it is in the 'unknown' state}}
>> }
>>
>> void testReturnStates() {
>> @@ -270,24 +409,34 @@
>> *var;
>>
>> var = returnsConsumed();
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> +}
>> +
>> +void testCallableWhen() {
>> + ConsumableClass<int> var(42);
>> +
>> + *var;
>> +
>> + baf3(var);
>> +
>> + var.callableWhenUnknown();
>> }
>>
>> void testMoveAsignmentish() {
>> ConsumableClass<int> var0;
>> ConsumableClass<long> var1(42);
>>
>> - *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> + *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
>> *var1;
>>
>> var0 = static_cast<ConsumableClass<long>&&>(var1);
>>
>> *var0;
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>>
>> var1 = ConsumableClass<long>(42);
>> var1 = nullptr;
>> - *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> + *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
>> }
>>
>> void testConditionalMerge() {
>> @@ -297,7 +446,7 @@
>> // Empty
>> }
>>
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>>
>> if (var.isValid()) {
>> // Empty
>> @@ -305,7 +454,7 @@
>> // Empty
>> }
>>
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> }
>>
>> void testConsumes0() {
>> @@ -315,13 +464,13 @@
>>
>> var.consume();
>>
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> }
>>
>> void testConsumes1() {
>> ConsumableClass<int> var(nullptr);
>>
>> - *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
>> }
>>
>> void testConsumes2() {
>> @@ -330,10 +479,10 @@
>> var.unconsumedCall();
>> var(6);
>>
>> - var.unconsumedCall(); // expected-warning {{invocation of method 'unconsumedCall' on object 'var' while it is in the 'consumed' state}}
>> + var.unconsumedCall(); // expected-warning {{invalid invocation of method 'unconsumedCall' on object 'var' while it is in the 'consumed' state}}
>> }
>>
>> -void testNonsenseState() {
>> +void testUnreachableBlock() {
>> ConsumableClass<int> var(42);
>>
>> if (var) {
>> @@ -349,10 +498,10 @@
>> ConsumableClass<int> var;
>>
>> for (int i = 0; i < 10; ++i) {
>> - *var;
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
>> }
>>
>> - *var;
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
>> }
>>
>> void testSimpleWhileLoop() {
>> @@ -361,9 +510,9 @@
>> ConsumableClass<int> var;
>>
>> while (i < 10) {
>> - *var;
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
>> ++i;
>> }
>>
>> - *var;
>> + *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
>> }
>> Index: test/SemaCXX/warn-consumed-parsing.cpp
>> ===================================================================
>> --- test/SemaCXX/warn-consumed-parsing.cpp
>> +++ test/SemaCXX/warn-consumed-parsing.cpp
>> @@ -1,6 +1,6 @@
>> // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s
>>
>> -#define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed))
>> +#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)))
>> @@ -19,33 +19,34 @@
>> 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}}
>> - void callableWhenUnconsumed() __attribute__ ((callable_when_unconsumed(42))); // expected-error {{attribute takes no arguments}}
>> + 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 var2 CALLABLE_WHEN_UNCONSUMED; // expected-warning {{'callable_when_unconsumed' 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 function2() CALLABLE_WHEN_UNCONSUMED; // expected-warning {{'callable_when_unconsumed' 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}}
>>
>> class CONSUMABLE(unknown) AttrTester1 {
>> - void callableWhenUnconsumed() CALLABLE_WHEN_UNCONSUMED;
>> - void consumes() CONSUMES;
>> - bool testsUnconsumed() TESTS_UNCONSUMED;
>> + void callableWhen0() CALLABLE_WHEN("unconsumed");
>> + void callableWhen1() CALLABLE_WHEN(42); // expected-error {{'callable_when' attribute requires a string}}
>> + void consumes() CONSUMES;
>> + bool testsUnconsumed() TESTS_UNCONSUMED;
>> };
>>
>> AttrTester1 returnTypestateTester0() RETURN_TYPESTATE(not_a_state); // expected-warning {{unknown consumed analysis state 'not_a_state'}}
>> AttrTester1 returnTypestateTester1() RETURN_TYPESTATE(42); // expected-error {{'return_typestate' attribute requires an identifier}}
>>
>> class AttrTester2 {
>> - void callableWhenUnconsumed() 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}}
>> + 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}}
>> };
>>
>> class CONSUMABLE(42) AttrTester3; // expected-error {{'consumable' attribute requires an identifier}}
>> Index: utils/TableGen/ClangAttrEmitter.cpp
>> ===================================================================
>> --- utils/TableGen/ClangAttrEmitter.cpp
>> +++ utils/TableGen/ClangAttrEmitter.cpp
>> @@ -449,15 +449,15 @@
>> OS << " " << getType() << " *" << getLowerName() << ";";
>> }
>> void writePCHReadDecls(raw_ostream &OS) const {
>> - OS << " unsigned " << getLowerName() << "Size = Record[Idx++];\n";
>> - OS << " SmallVector<" << type << ", 4> " << getLowerName()
>> + OS << " unsigned " << getLowerName() << "Size = Record[Idx++];\n";
>> + OS << " SmallVector<" << type << ", 4> " << getLowerName()
>> << ";\n";
>> - OS << " " << getLowerName() << ".reserve(" << getLowerName()
>> + OS << " " << getLowerName() << ".reserve(" << getLowerName()
>> << "Size);\n";
>> - OS << " for (unsigned i = " << getLowerName() << "Size; i; --i)\n";
>> + OS << " for (unsigned i = " << getLowerName() << "Size; i; --i)\n";
>>
>> std::string read = ReadPCHRecord(type);
>> - OS << " " << getLowerName() << ".push_back(" << read << ");\n";
>> + OS << " " << getLowerName() << ".push_back(" << read << ");\n";
>> }
>
> The formatting changes are good, but should be a separate patch.
>
>> void writePCHReadArgs(raw_ostream &OS) const {
>> OS << getLowerName() << ".data(), " << getLowerName() << "Size";
>> @@ -563,6 +563,76 @@
>> OS << " }\n";
>> }
>> };
>> +
>> + class VariadicEnumArgument: public VariadicArgument {
>> + std::string type, QualifiedTypeName;
>> + std::vector<StringRef> values, enums, uniques;
>
> Formatting (the names should follow the style guidelines).
>
>> + public:
>> + VariadicEnumArgument(Record &Arg, StringRef Attr)
>> + : VariadicArgument(Arg, Attr, Arg.getValueAsString("Type")),
>> + type(Arg.getValueAsString("Type")),
>> + values(getValueAsListOfStrings(Arg, "Values")),
>> + enums(getValueAsListOfStrings(Arg, "Enums")),
>> + uniques(enums)
>> + {
>> + // Calculate the various enum values
>> + std::sort(uniques.begin(), uniques.end());
>> + uniques.erase(std::unique(uniques.begin(), uniques.end()), uniques.end());
>
> This strikes me as hiding a bug with the user's written enumerations.
> If they say: __attribute__((foo(one, one, one))) we should 1) report
> that to Sema because that information may be important to some
> attribute, and 2) Sema (in your case) should report the duplication.
> Ideally, there would be some helper method that Sema can optionally
> call to ensure uniqueness within the list and report a generic error
> about duplication.
Nevermind this comment -- this is uniquing for a different reason. I
see this was copy and pasted from EnumArgument.
>
>> +
>> + QualifiedTypeName = getAttrName().str() + "Attr::" + type;
>> +
>> + // FIXME: Emit a proper error
>> + assert(!uniques.empty());
>
> Please fix the fixme.
This still applies though. ;-)
>
>> + }
>> +
>> + void writeDeclarations(raw_ostream &OS) const {
>> + std::vector<StringRef>::const_iterator i = uniques.begin(),
>> + e = uniques.end();
>> + // The last one needs to not have a comma.
>> + --e;
>> +
>> + OS << "public:\n";
>> + OS << " enum " << type << " {\n";
>> + for (; i != e; ++i)
>> + OS << " " << *i << ",\n";
>> + OS << " " << *e << "\n";
>> + OS << " };\n";
>> + OS << "private:\n";
>> +
>> + VariadicArgument::writeDeclarations(OS);
>> + }
>> + void writeDump(raw_ostream &OS) const {
>> + OS << " for (" << getAttrName() << "Attr::" << getLowerName()
>> + << "_iterator I = SA->" << getLowerName() << "_begin(), E = SA->"
>> + << getLowerName() << "_end(); I != E; ++I) {\n";
>> + OS << " switch(*I) {\n";
>> + for (std::vector<StringRef>::const_iterator UI = uniques.begin(),
>> + UE = uniques.end(); UI != UE; ++UI) {
>> + OS << " case " << getAttrName() << "Attr::" << *UI << ":\n";
>> + OS << " OS << \" " << *UI << "\";\n";
>> + OS << " break;\n";
>> + }
>> + OS << " }\n";
>> + OS << " }\n";
>> + }
>> + void writePCHReadDecls(raw_ostream &OS) const {
>> + OS << " unsigned " << getLowerName() << "Size = Record[Idx++];\n";
>> + OS << " SmallVector<" << QualifiedTypeName << ", 4> " << getLowerName()
>> + << ";\n";
>> + OS << " " << getLowerName() << ".reserve(" << getLowerName()
>> + << "Size);\n";
>> + OS << " for (unsigned i = " << getLowerName() << "Size; i; --i)\n";
>> + OS << " " << getLowerName() << ".push_back(" << "static_cast<"
>> + << QualifiedTypeName << ">(Record[Idx++]));\n";
>> + }
>> + void writePCHWrite(raw_ostream &OS) const{
>> + OS << " Record.push_back(SA->" << getLowerName() << "_size());\n";
>> + OS << " for (" << getAttrName() << "Attr::" << getLowerName()
>> + << "_iterator i = SA->" << getLowerName() << "_begin(), e = SA->"
>> + << getLowerName() << "_end(); i != e; ++i)\n";
>> + OS << " " << WritePCHRecord(QualifiedTypeName, "(*i)");
>> + }
>> + };
>>
>> class VersionArgument : public Argument {
>> public:
>> @@ -724,6 +794,8 @@
>> Ptr = new SimpleArgument(Arg, Attr, "SourceLocation");
>> else if (ArgName == "VariadicUnsignedArgument")
>> Ptr = new VariadicArgument(Arg, Attr, "unsigned");
>> + else if (ArgName == "VariadicEnumArgument")
>> + Ptr = new VariadicEnumArgument(Arg, Attr);
>> else if (ArgName == "VariadicExprArgument")
>> Ptr = new VariadicExprArgument(Arg, Attr);
>> else if (ArgName == "VersionArgument")
>>
>
> ~Aaron
>
> On Thu, Sep 5, 2013 at 9:55 PM, Christian Wailes <chriswailes at google.com> wrote:
>> Hi dblaikie, delesley, rsmith, aaron.ballman,
>>
>> The callable_when annotation can take a list of states that a function can be called in. This reduced the total number of annotations needed and makes writing more complicated behaviour less burdensome.
>>
>> To allow the the callable_when attribute to hold a variable number of states a VariadicEnumArgument Tablegen attribute classes was added. Future work should replace the various Variadic attribute classes with a single class that takes another attribute as a parameter. A FIXME has been added to indicate this at the request of Richard Smith.
>>
>> http://llvm-reviews.chandlerc.com/D1613
>>
>> Files:
>> include/clang/Analysis/Analyses/Consumed.h
>> include/clang/Basic/Attr.td
>> include/clang/Basic/DiagnosticSemaKinds.td
>> lib/Analysis/Consumed.cpp
>> lib/Sema/AnalysisBasedWarnings.cpp
>> lib/Sema/SemaDeclAttr.cpp
>> test/SemaCXX/warn-consumed-analysis-strict.cpp
>> test/SemaCXX/warn-consumed-analysis.cpp
>> test/SemaCXX/warn-consumed-parsing.cpp
>> utils/TableGen/ClangAttrEmitter.cpp
~Aaron
More information about the cfe-commits
mailing list