[PATCH] replaced callable_when_unconsumed annotation with generic callable_when annotation
Christian Wailes
chriswailes at google.com
Fri Sep 6 11:15:00 PDT 2013
With regards to the string literals and identifiers: I agree that the
parser should be able to handle lists of unresolved identifiers and would
be willing to work on the feature. Currently, I don't know how to change
the parser to get that working, and this is my last day at Google. Plus,
making that change should probably be a separate patch.
- Chris
P.S. Richard was removed from this email chain because his Microsoft
Exchange server keeps bouncing my emails.
On Fri, Sep 6, 2013 at 11:08 AM, Christian Wailes <chriswailes at google.com>wrote:
>
>
>
> On Fri, Sep 6, 2013 at 7: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.
>>
>>
> LibAnalysis doesn't link against LibSema and therefore can't call PDiag or
> any of the other Sema functions necessary to issue warnings. This gets
> around it by creating the implementation in Sema and routing calls to it in
> Analysis via the VTable. I thought it was a bit weird as well.
>
>
>> > };
>> >
>> > - 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.
>>
>>
> It could, yes, but I would still have to provide a method to map between
> the CallableWhenAttr enum and the ConsumedState enum. I think it would end
> up being more code due to the second function definition.
>
>
>> > +
>> > 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?
>>
>
> Pointers to consumable types aren't handled yet. This will change in the
> near future.
>
>
>>
>> > + 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...
>>
>>
> A StringSwitch won't work in this case. If there was an extra value in
> the CallableWhenAttr::ConsumedState enum the default could return that and
> I could test CallableState and issue the warning and return
> appropriately. Unfortunately there is no extra enum value. I agree that
> this is a common case and it would be possible to auto-generate code to
> handle it. Additionally, it would be nice to be able to declare an enum in
> Attr.td, have it emitted along with a string-to-enum mapping function, and
> then be able to re-use that enum in multiple attributes... but that is code
> for another patch.
>
>
>> > +
>> > + 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).
>>
>
> Which name, and which style guideline? The QualifiedTypeName follows the
> Clang LLVM style guidelines, and the rest are coppied from other places in
> the ClangAttrEmitter.cpp file.
>
>
>>
>> > + 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.
>>
>>
> > +
>> > + QualifiedTypeName = getAttrName().str() + "Attr::" + type;
>> > +
>> > + // FIXME: Emit a proper error
>> > + assert(!uniques.empty());
>>
>> Please fix the fixme.
>>
>
> This should probably be a seperate patch, as this FIXME is copied from
> above. As well, I'm not entirely sure how Tablegen emits warnings
> concerning problems with the .td files. This is to warn if the enum
> declaration array for EnumArgument (or VariadicEnumArgument) contains zero
> values.
>
>
>>
>> > + }
>> > +
>> > + 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
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130906/575c7063/attachment.html>
More information about the cfe-commits
mailing list