<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Looks good. Please apply!<div><br><div><div>On Aug 29, 2008, at 12:12 AM, Zhongxing Xu wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">New patch attached.<br><br><div class="gmail_quote">On Fri, Aug 29, 2008 at 1:46 PM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> <div class="Ih2E3d">On Aug 27, 2008, at 11:16 PM, Zhongxing Xu wrote:<br> <br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="Ih2E3d"> This patch moves the rest of the symbolic analysis stuff into BasicConstraintManager.<br> <br> - Zhongxing Xu<br></div> <consteqty.patch>_______________________________________________<br> cfe-commits mailing list<br> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br> </blockquote> <br> <br> Hi Zhongxing,<br> <br> Somehow I missed this patch when I went through my email today.<br> <br> It looks pretty good! The only concern I can spot right now is the commenting out of the pretty-printing of equality and inequality relationships. That's a functionality regression that we shouldn't have, and generally we shouldn't be commenting out code (if we want to remove, we just remove it). It's also an opportunity to generalize the functionality to allow pretty-printing of arbitrary constraints managed by the ConstraintManager.<br> <br> It's clear that we will need to add a print method to ConstraintManager, similar to what we did with the StoreManager:<br> <br> virtual void print(Store store, std::ostream& Out, const char* nl, const char *sep) = 0;<br> <br> Of course, in the case of ConstraintManager, we just pass the state instead of the store.<br> <br> We can then move the pretty-printing logic for the inequality/equality relationships into BasicConstraintManager. That seems like a small amendment to your patch, and would complete the migration of the functionality and improve the API.<br> <br> Otherwise, great patch!<br><font color="#888888"> <br> Ted<br> </font></blockquote></div><br></div> <span><consteq2.patch></span></blockquote></div><br></div></body></html>