[PATCH] [analyzer] Factor common code from {SymSym,SymInt,IntSym}Expr to BinarySymExpr

Anna Zaks ganna at apple.com
Wed Apr 10 20:53:14 PDT 2013


The patch looks good to me. See a couple of minor comments below.


-    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 <rzg at apple.com> 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>

More information about the cfe-commits mailing list