LegalizeAdulthood added inline comments.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h:1
+#if !defined(READABILITY_DUPLICATE_INCLUDE_H)
+#define READABILITY_DUPLICATE_INCLUDE_H
----------------
carlosgalvezp wrote:
> Nit: `#ifndef`
It's a style thing. I don't prefer `#ifndef` because it only differs from
`#ifdef` by a single letter.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/sys/types.h:1
+// empty file used by readability-duplicate-include.cpp
----------------
carlosgalvezp wrote:
> Nit: write "This file is intentionally empty" for consistency with the other
> files you added? It's already clear from the folder structure that this file
> is an input for the `readability-duplicate-include` test.
Oops, yeah, meant to do that with all of them and missed this one.
When I first authored this check 7 years ago, they didn't have the Inputs
folder.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp:19
+int f;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^int d;$}}
----------------
carlosgalvezp wrote:
> Nit: it would be good to keep `[readability-duplicate-include]` part of the
> warning consistent - on Line 8 you have it but not on the other lines. So
> either keep it for all warnings or remove it from line 8.
`CHECK-MESSAGES` will match whatever substring you provide. To make
the test file less noisy, I do a full match on the entire diagnostic for the
first warning and then just the necessary bits for the remaining checks.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D7982/new/
https://reviews.llvm.org/D7982
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
