With regards to the tests, those already exist. They are in the warn-consumed-parsing.cpp file. As for the switch statements, I'll move those to fully-covered switches.
I was not claiming that keeping patches limited is a bad idea, or was of limited benefit in general. Yes, I should have submitted this as multiple patches, but to go back now and dig the different parts out of the commit out would be time consuming, for very little benefit *in this case*. All of the things touched are inside my experimental code, or are the removal of simple asserts. I'd really like to focus on producing actual work, and keep this as a lesson for next time. - Chris On Wed, Aug 21, 2013 at 9:19 AM, Reid Kleckner <[email protected]> wrote: > On Wed, Aug 21, 2013 at 6:54 AM, Aaron Ballman <[email protected]>wrote: > >> On Tue, Aug 20, 2013 at 7:16 PM, Christian Wailes >> <[email protected]> wrote: >> >> > + >> >> > + if (!FunDecl->hasAttr<CallableWhenUnconsumedAttr>()) return; >> >> > + >> >> > + if (PState.IsVar) { >> >> > + const VarDecl *Var = PState.getVar(); >> >> > + >> >> > + switch (StateMap->getState(Var)) { >> >> > + case CS_Consumed: >> >> > + Analyzer.WarningsHandler.warnUseWhileConsumed( >> >> > + FunDecl->getNameAsString(), Var->getNameAsString(), >> >> > + Call->getExprLoc()); >> >> > + break; >> >> > + >> >> > + case CS_Unknown: >> >> > + Analyzer.WarningsHandler.warnUseInUnknownState( >> >> > + FunDecl->getNameAsString(), Var->getNameAsString(), >> >> > + Call->getExprLoc()); >> >> > + break; >> >> > + >> >> > + default: >> >> > + break; >> >> >> >> Should the default be unreachable? >> >> >> > >> > No, there are other cases besides Consumed and Unknown. I just don't >> want >> > to do anything for those cases. >> >> I worry that this will cause extra warnings in some of the build bots >> because the other enumerants are not covered in the switch statement. >> The same is true elsewhere. >> > > It is a bit of a clang coding convention to use fully-covered switches > with no default. That way, when someone adds an enum value, the compiler > will warn on all of the switches missing cases. > > We shouldn't get warnings for a partially covered switch with a default > (like this one), although we will for fully covered switches with a default. > > There are only two enums vs one default, so I'd cover the switch to > leverage the warning. >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
