r345971 - Reapply Logging: make os_log buffer size an integer constant expression.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 2 08:28:11 PDT 2018


On Fri, Nov 2, 2018 at 4:08 PM Tim Northover <tnorthover at apple.com> wrote:

> After that it was discovered OSLog depends on the other format helpers so
> there was still a circular dependency; I decided adding the others was a
> minor enough change to just go ahead.
>
Thanks - I missed the review.

> On 2 Nov 2018, at 15:03, Tim Northover <tnorthover at apple.com> wrote:
> >> (If the do belong here: the namespaces and comments don't seem to have
> been updated)
> >
> > Good point, I’ll get onto fixing that.
>
> Actually, do you have any particular ones in mind? After going in to make
> changes, they still look pretty good to me; I don’t think libclangAnalysis
> should have the monopoly on the term.
>
Thanks for looking at this!

>From my (very limited) understanding of os_log and AST, it's going to rely
on __builtin_os_log_format_buffer_size being computed at compile-time, so
AST now needs to understand the semantics of format strings.
In that case, I don't think it makes sense to think of the format string
parser as part of the analyzer - as the build deps suggest, it's now part
of AST and gets reused by analyzer. (Similar to how the analyzer uses other
bits of AST/clang). If there are parts only relevant to analyzer, it'd be
nice to move them out of the AST library, but I don't know to what extent
that's feasible.

So it does seem to me like all the uses of analyzer namespaces are suspect
- moving code from Analyzer to AST is a semantic difference (the layers
aren't *just* about making the linker happy, after all!)

Cheers, Sam
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181102/9f678c3b/attachment-0001.html>


More information about the cfe-commits mailing list