On Tue, Jan 27, 2015 at 2:57 AM, Dimitry Andric <[email protected]> wrote: > In http://reviews.llvm.org/D7154#113752, @hans wrote: > >> I would prefer to pass in the FormatStringType instead of a bool though. > > > Yes, I would have preferred that too, but since FormatStringType comes from > include/clang/Sema/Sema.h, and the actual usage is in > lib/Analysis/PrintfFormatString.cpp, I'd have to add includes to that latter > file, and prefix everything with Sema:: there. It just made it all more > messy, and I tried to avoid that.
It would be a layering violation to bring Sema into Analysis (I believe), so I would avoid it. That being said, it would be nice to find a cleaner way to do it than this Boolean, but I don't see that as a show-stopper. This weakly LGTM -- the code is reasonable, but I'm not certain about the need. I don't see this as adding a huge maintenance burden, so I think it's fine, but others may have different opinions. ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
