aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: clang-tools-extra/test/clang-tidy/Inputs/Headers/stdio.h:1
+#ifndef _STDIO_H_
+#define _STDIO_H_
----------------
stephanemoore wrote:
> stephanemoore wrote:
> > I noticed that some of the other example headers don't have `#ifdef` guards 
> > at all. I decided to still include them—perhaps mostly for my own sanity. I 
> > figured that it wasn't necessary to use the full 
> > `LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_INPUTS_HEADER_STDIO_H` for this 
> > header that is mimicking a system header so I went with the shorter 
> > `_STDIO_H_` which I think better represents what actual stdio.h headers 
> > would have. Let me know if you think something else makes more sense.
> Do I need the licensing preamble on this test input header?
Yes, please add it.


================
Comment at: clang-tools-extra/test/clang-tidy/Inputs/Headers/stdio.h:1-9
+#ifndef _STDIO_H_
+#define _STDIO_H_
+
+// A header intended to contain C standard input and output library
+// declarations.
+
+int printf(const char *, ...);
----------------
aaron.ballman wrote:
> stephanemoore wrote:
> > stephanemoore wrote:
> > > I noticed that some of the other example headers don't have `#ifdef` 
> > > guards at all. I decided to still include them—perhaps mostly for my own 
> > > sanity. I figured that it wasn't necessary to use the full 
> > > `LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_INPUTS_HEADER_STDIO_H` for this 
> > > header that is mimicking a system header so I went with the shorter 
> > > `_STDIO_H_` which I think better represents what actual stdio.h headers 
> > > would have. Let me know if you think something else makes more sense.
> > Do I need the licensing preamble on this test input header?
> Yes, please add it.
I don't think it matters all that much for our tests; I'm fine with the way you 
have it currently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58095



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to