On Fri, Nov 2, 2018 at 4:08 PM Tim Northover <tnortho...@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 <tnortho...@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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
