[PATCH] D83223: [clang-tidy] Header guard check can skip past license comment

Alexander Kornienko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 21 02:15:12 PDT 2020


alexfh added a comment.

In D83223#2147306 <https://reviews.llvm.org/D83223#2147306>, @aaron.ballman wrote:

> In D83223#2147247 <https://reviews.llvm.org/D83223#2147247>, @njames93 wrote:
>
> > In D83223#2147072 <https://reviews.llvm.org/D83223#2147072>, @aaron.ballman wrote:
> >
> > > >   // This is not identified as a license comment as the
> > > >   // block is followed by code.
> > > >   void foo();
> > >
> > > FWIW: https://github.com/GrammaTech/gtirb-pprinter/blob/master/include/gtirb_pprinter/AttPrettyPrinter.hpp or https://github.com/GrammaTech/gtirb/blob/master/include/gtirb/AuxData.hpp (so there are projects which do not put a newline between the license and code).
> >
> >
> > Short of creating an AI that understands context it won't be possible to determine the difference between license and general documentation, in any case I feel this heuristic is the safest way to ensure good coverage with minimised risk of inserting the guard in the middle of documentation,
>
>
> My instinct is that we shouldn't be trying to play those games in the first place and should consider *all* leading comments and empty (whitespace-only) lines as part of the "license" and expect the first significant token to be the header guard. e.g., this isn't about the license at all, it's about whether you can have prose before the header guard or not. It's not uncommon for projects to put prose before header guards, nor is it uncommon for it to go after the header guards. tbh, that feels a bit like an option for the feature rather than an automatic behavior because I could also see a project wanting to enforce a consistent style.


A drive-by note: I'm also for treating all leading comments equally (and referring to them as just "comments", not "license comments").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83223





More information about the cfe-commits mailing list