Looks good to me, although eventually in a future patch we should just have the CFG's BuildOptions be stored in AnalysisContext instead of replicating all these flags.
On Sep 29, 2010, at 11:42 PM, Marcin Świderski wrote: > Thanks for noticing this. I will fix this and commit if there are no other > issues. > > W dniu 30 września 2010 08:25 użytkownik Zhongxing Xu <[email protected]> > napisał: > Hi Marcin, > > Overall the patch looks good. But one problem: > > @@ -126,7 +130,8 @@ > idx::TranslationUnit > *TU) { > AnalysisContext *&AC = Contexts[D]; > if (!AC) > - AC = new AnalysisContext(D, TU, UseUnoptimizedCFG); > + AC = new AnalysisContext(D, TU, UseUnoptimizedCFG, AddImplicitDtors, > + AddInitializers); > > Notice that the fourth parameter is bool addehedges = false. We could set it > to false for now. In the future we would add an option for it, too. > > You could spot this bug by actually running clang with the patch on a test > case: > > $ clang -cc1 -analyze -cfg-view -cfg-add-implicit-dtors scope.cpp > > And see no effect with the new option. > > I did test the patch some other way and it appeared to work properly. > Sometimes tests can be broken though... > > And for each patch, you can run 'make test' to ensure it does not break > regression tests. > > Yes, I do it all the time. > > 2010/9/30 Marcin Świderski <[email protected]> > > Patch adds two new command line arguments: > -cfg-add-implicit-dtors - sets CFG::BuildOptions::AddImplicitDtors for > AnalysisCosumer to true > -cfg-add-initializers - sets CFG::BuildOptions::AddInitializers for > AnalysisCosumer to true > > Please approve for commit. > > Marcin > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
