[PATCH] Add FreeBSD kernel printf extensions

Aaron Ballman aaron.ballman at gmail.com
Tue Jan 27 12:10:12 PST 2015


On Tue, Jan 27, 2015 at 2:57 AM, Dimitry Andric <dimitry at andric.com> 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




More information about the cfe-commits mailing list