[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

Sun Oct 13 09:51:42 PDT 2019

MyDeveloperDay added a comment.

In D68554#1707537 <https://reviews.llvm.org/D68554#1707537>, @thakis wrote:

> > Thanks for doing this, I am struggling to find the MacOS bot log that failed, are any available via Buildbot? (I notice the log looks like your own machine)
>
> The mac bots are on a different system for some reason: http://green.lab.llvm.org/green/ (see also http://lists.llvm.org/pipermail/llvm-dev/2019-October/135771.html , I wasn't aware of that until a few days ago either).
>
> > So as you said it will be a little slower to build clang-format, however, I do notice that all the other clang/tools are pretty much are all building with Frontend. so anyone building a larger range of clang tools probably has everything already built.
>
> I believe all the other clang tools conceptually do semantic analysis, so that makes more sense for them :)

  Agreed..

> 
> 
>> I think the main build failure was just the lit tests, I think i would prefer to fix those and reland this as-is, then look for a better diags solution if @klimek thinks this is something I should be concerned about. I really wanted to be able to use a standard Diagnostic front end with all the support for console coloring etc...I'll have to take a look at LLVM's diagnostics to see how that is done.
> 
> I hope my view on this isn't entirely dismissed (see `git shortlog -nes clang/lib/Format/`).

  Actually I hope not too, I thought this Frontend mechanism was a bit overkill too, but I held back from just doing `llvm::errs() << ..." because I felt that was reinventing the wheel (but it would probably be more space-efficient) especially in terms of dependencies

> In addition to the actual size cost of this change: about a third of clang-format's size is the diagnostics table, but before this change here it's kept alive by only two references from lib/Basic and it's reasonably easy to remove these, which should make the binary 1 MB smaller. After this change,

that's much harder to do.

  Yes I get that, if I can switch to LLVM diags then I guess can avoid that.  I also wondered if there was some way of me doing this without the `TextDiagnosticPrinter` (which is the only thing that caused Frontend to need to come in, the rest of the Diagnositcs stuff (DiagnosticOptions,DiagnosticEngine etc..) was already being used.

> LLVM's text diag stuff also supports colors and whatnot.

  Could you or anyone else point me to a good example, I'd definitely like to take a look.

> And another idea: maybe it makes sense to expose these clang-format diagnostics you're adding (which I do think are a cool feature!) via clang-format? That already depends on everything and the kitchen sync, so you could easily output diagnostics from there – and it'd allow clang-format itself to be a tool focused on doing one thing well (namely, formatting code).

  `via clang-format?` did you mean via clang.exe or clang-check.exe?   I did think of this when I started doing this work, I just came to the conclusion that if I did that it wouldn't be long before someone requested the same features in clang-format itself.

  I think from what you've said, trying to break the Frontend dependency is probably important, as for a dependency on lib/Basic I feel I was just asked to make that worse {D68914} perhaps that's why the code was duplicated in the first place...

  bear with me I'm still learning all this stuff.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68554/new/

https://reviews.llvm.org/D68554