<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 31, 2011, at 12:56 AM, Jordy Rose wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; ">On Aug 30, 2011, at 20:16, Ted Kremenek wrote:<br><br><blockquote type="cite"><br></blockquote><blockquote type="cite">On Aug 29, 2011, at 11:49 PM, Jordy Rose wrote:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><blockquote type="cite">Ah. I was thinking something like this:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">1. GC Disabled<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">a) create CheckerManager and register checkers<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">b) run non-path-sensitive analyses<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">c) run path-sensitive analyses<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">2. GC Enabled<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">a) create CheckerManager and register checkers<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">b) run non-path-sensitive analyses<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">c) run path-sensitive analyses<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Let's call this plan A.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><blockquote type="cite">It sounds like you're thinking:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">1. Create CheckerManager and register checkers<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">2. Mgr->setGCEnabled(true)<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">a) run non-path-sensitive analyses<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">b) run path-sensitive analyses<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">3. GC Enabled<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">a) run non-path-sensitive analyses<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">b) run path-sensitive analyses<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Let's call this plan B.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">...and then checkers check if GC is enabled every time it's interesting, instead of once at registration time.<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">That's clearer. I think your plan sounds cleaner (plan A). Couple concerns:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">(1) I still want checkers to be written so that if they want GC-specific logic, they don't need to be written as a separate checker. For the Retain/Release checker, we can still keep conditional behavior just by passing the "GC enabled" flag to the checker at construction time.<br></blockquote><br>The only thing I'm a little concerned about (again) is that if the flag lives on the CheckerManager it's not easily accessible (you have to go through the AnalysisManager). Is a shortcut in CheckerContext good enough? Not all callbacks include CheckerContexts, but many do.</span></blockquote></div><br><div>I thought about this a bit over the last couple days. I really hate the idea of CheckerManager having any notion of this flag. It doesn't feel right at all. CheckerManager should just be managing checkers. It shouldn't be in the business of changing fundamental ways in how the checkers are run.</div><div><br></div><div><blockquote type="cite">What I'm working with looks something like this:<br><br>AnalysisConsumer -> PDC -> [TU] -> [GC/non-GC] -> AnalysisManager/CheckerManager -> [Decl] -> ExprEngine<br></blockquote></div><div><br></div><div>Following on this, I had another thought (and I'm going to backpedal on what I said earlier). There is no need for all checkers to get rerun between GC/non-GC. For the non-ExprEngine analyses, we only need to call them once. They have access to the LangOptions, so if they want to do something GC-specific, they can.</div><div><br></div><div>For the ExprEngine analyses, how about:</div><div><br></div><div> AnalysisConsumer -> PDC -> [TU] -> AnalysisManager/CheckerManager -> [Decl] -> [GC/non-GC] -> ExprEngine<br></div><div><br></div><div>We can just keep the flag in ExprEngine, and have CheckerContext consult that. At some point, all checker callbacks should have a CheckerContext object, so this eventually lead to a nice clean API. There is also no analysis waste here.</div><div><br></div><div>The reason I prefer this approach is that ExprEngine is special when it comes to the hybrid GC mode. We *know* that GC-semantics can change how the whole analysis works, so we need to run it twice. Other non-ExprEngine analyses (if they care), can do the same thing.</div><div><br></div><div><br></div></body></html>