[libc-commits] [PATCH] D77533: [libc] Make all header comments consistent.

[Alex Brachet via Phabricator via libc-commits]

abrachet accepted this revision.
abrachet added a comment.

Just some nits inline.

> One thing though, this change is now wider than the original description. So, update the description here (and also in your local git repo if you use git llvm push.)

It might also be useful to add [NFC] to the name while you're at it.

In D77533#1964874 <https://reviews.llvm.org/D77533#1964874>, @PaulkaToast wrote:

> In D77533#1964684 <https://reviews.llvm.org/D77533#1964684>, @sivachandra wrote:
>
> > In D77533#1964606 <https://reviews.llvm.org/D77533#1964606>, @abrachet wrote:
> >
> > > Also, although we only want header files to have the C++ tag, shouldn't we also left justify the text in our .cpp files too?
> >
> >
> > Yes, at least for consistency.
> >
> > For future integrity, are there are any clang-format/clang-tidy rules which can warn us about such misses? For bad/missing headers guards, there are clang-tidy rules available for LLVM.
>
>
> I'll look into this, I'm sure a check exists or we can make one.


My guess although I'm not sure is it would be difficult to get this into clang-format because it's very LLVM specific. I also don't know if it would work in clang-tidy, I'm not sure if comments are expressed in the AST but @PaulkaToast would know more than me :) It would be very useful for one of these tools to do this for us for sure.



================
Comment at: libc/config/linux/errno.h.in:1
-//===---------------- Linux specific errno.h definitions ------------------===//
+//===--- Linux specific errno.h definitions -------------------------------===//
 //
----------------
This has 3 leading `-`


================
Comment at: libc/include/__llvm-libc-common.h:1
-//===------- Common definitions for LLVM-libc public header files- --------===//
+//===-- Common definitions for LLVM-libc public header files- -------------===//
 //
----------------
sivachandra wrote:
> abrachet wrote:
> > Looks like there is a `- ---` which we should make ` ---`. Also should these have `-*- C++ -*-`?
> We shouldn't have `C++` here as these are public C headers. The rest of the comment is valid.
You're right we shouldn't have them in the public headers. libcxx does this but they also have no extension at all not just .h, also most of headers are generated so its very little use to us.


================
Comment at: libc/src/threads/linux/thread_start_args.h.def:1
-//===---- Implementation of the get_start_args_addr function -----*- C++ -*===//
+//===-- Implementation of the get_start_args_addr function -------*- C++ -*===//
 //
----------------
Add an extra `-` at the end, the `*` is right next to the `=`


================
Comment at: libc/utils/CPP/StringRef.h:1
-//===----------------- A standalone StringRef type  -------------*- C++ -*-===//
+//===-- A standalone StringRef type  ----------------------------*- C++ -*-===//
 //
----------------
Looks like two spaces between type and -


================
Comment at: libc/utils/HdrGen/PublicAPICommand.cpp:1
-//===--------------- Implementation of PublicAPICommand ----------*-C++ -*-===//
+//===-- Implementation of PublicAPICommand ----------*---------------------===//
 //
----------------
There is a `*` here


================
Comment at: libc/utils/benchmarks/LibcMemoryBenchmarkMain.cpp:1
-//===-------- Benchmark  --------------------------------------------------===//
+//===-- Benchmark  --------------------------------------------------------===//
 //
----------------
Two trailing spaces here


================
Comment at: libc/utils/benchmarks/LibcMemoryBenchmarkTest.cpp:1
+//===--`Benchmark` Memory Test --------------------------------------------===//
+//
----------------
No space between `-` and ```


================
Comment at: libc/utils/testutils/ExecuteFunction.h:1
-//===---------------------- ExecuteFunction.h -------------------*- C++ -*-===//
+//===--- ExecuteFunction.h --------------------------------------*- C++ -*-===//
 //
----------------
Three leading `-`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77533