Ryan, The patch looks good to me. See a couple of minor comments below.
Cheers, Anna. - Profile(ID, LHS, Op, RHS, T); + Profile(ID, LHS, this->getOpcode(), RHS, this->getType()); Is there a reason you are using 'this' instead of just calling the getter? +/// SymIntExpr - Represents symbolic expression like 'x' + 3. We use doxygen style comments in the later commits. For example, see the comment for RuntimeDefinition class in CallEvent.h. On Apr 10, 2013, at 6:19 PM, Ryan Govostes <[email protected]> wrote: > The attached patch introduces a BinarySymExpr base class for SymSymExpr, > SymIntExpr, and IntSymExpr, and factors getType() and getOpcode() out of > these subclasses and into the base class. I could not factor getLHS() and > getRHS() into the base class because these return unrelated types between > subclasses. > > This permits the simplification of repetitive code such as: > > case SymExpr::SymIntKind: > return cast<SymIntExpr>(SE)->getOpcode(); > case SymExpr::IntSymKind: > return cast<IntSymExpr>(SE)->getOpcode(); > case SymExpr::SymSymKind: > return cast<SymSymExpr>(SE)->getOpcode(); > > However I did not notice any existing parts of the static analyzer that would > currently benefit from this refactoring. > > Ryan > > <BinarySymExpr-0001.diff> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
