[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-07-11 Thread Nathan James via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc3bdc9814d94: [clang-tidy] Reworked enum options 
handling(again) (authored by njames93).

Changed prior to commit:
  https://reviews.llvm.org/D82188?vs=275520=277227#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -6,6 +6,20 @@
 
 namespace clang {
 namespace tidy {
+
+enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+
+template <> struct OptionEnumMapping {
+  static llvm::ArrayRef> getEnumMapping() {
+static constexpr std::pair Mapping[] = {
+{Colours::Red, "Red"},   {Colours::Orange, "Orange"},
+{Colours::Yellow, "Yellow"}, {Colours::Green, "Green"},
+{Colours::Blue, "Blue"}, {Colours::Indigo, "Indigo"},
+{Colours::Violet, "Violet"}};
+return makeArrayRef(Mapping);
+  }
+};
+
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
@@ -208,16 +222,10 @@
 #undef CHECK_ERROR_INT
 }
 
+// FIXME: Figure out why this test causes crashes on mac os.
+#ifndef __APPLE__
 TEST(ValidConfiguration, ValidEnumOptions) {
 
-  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
-  static constexpr std::pair Mapping[] = {
-  {"Red", Colours::Red},   {"Orange", Colours::Orange},
-  {"Yellow", Colours::Yellow}, {"Green", Colours::Green},
-  {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo},
-  {"Violet", Colours::Violet}};
-  static const auto Map = makeArrayRef(Mapping);
-
   ClangTidyOptions Options;
   auto  = Options.CheckOptions;
 
@@ -237,34 +245,37 @@
 #define CHECK_ERROR_ENUM(Name, Expected)   \
   CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
 
-  CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red);
-  CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet);
-  CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true),
-Colours::Red);
+  CHECK_VAL(TestCheck.getIntLocal("Valid"), Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValid"), Colours::Violet);
+
   CHECK_VAL(
-  TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true),
-  Colours::Violet);
-  CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map),
+  TestCheck.getIntLocal("ValidWrongCase", /*IgnoreCase*/ true),
+  Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValidWrongCase",
+/*IgnoreCase*/ true),
+Colours::Violet);
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalInvalid"),
"invalid configuration value "

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-07-04 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 275520.
njames93 added a comment.

Solved the mac issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -6,6 +6,20 @@
 
 namespace clang {
 namespace tidy {
+
+enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+
+template <> struct OptionEnumMapping {
+  static llvm::ArrayRef> getEnumMapping() {
+static constexpr std::pair Mapping[] = {
+{Colours::Red, "Red"},   {Colours::Orange, "Orange"},
+{Colours::Yellow, "Yellow"}, {Colours::Green, "Green"},
+{Colours::Blue, "Blue"}, {Colours::Indigo, "Indigo"},
+{Colours::Violet, "Violet"}};
+return makeArrayRef(Mapping);
+  }
+};
+
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
@@ -208,16 +222,10 @@
 #undef CHECK_ERROR_INT
 }
 
+// FIXME: Figure out why this test causes crashes on mac os.
+#ifndef __APPLE__
 TEST(ValidConfiguration, ValidEnumOptions) {
 
-  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
-  static constexpr std::pair Mapping[] = {
-  {"Red", Colours::Red},   {"Orange", Colours::Orange},
-  {"Yellow", Colours::Yellow}, {"Green", Colours::Green},
-  {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo},
-  {"Violet", Colours::Violet}};
-  static const auto Map = makeArrayRef(Mapping);
-
   ClangTidyOptions Options;
   auto  = Options.CheckOptions;
 
@@ -237,34 +245,37 @@
 #define CHECK_ERROR_ENUM(Name, Expected)   \
   CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
 
-  CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red);
-  CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet);
-  CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true),
-Colours::Red);
+  CHECK_VAL(TestCheck.getIntLocal("Valid"), Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValid"), Colours::Violet);
+
   CHECK_VAL(
-  TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true),
-  Colours::Violet);
-  CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map),
+  TestCheck.getIntLocal("ValidWrongCase", /*IgnoreCase*/ true),
+  Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValidWrongCase",
+/*IgnoreCase*/ true),
+Colours::Violet);
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalInvalid"),
"invalid configuration value "
"'Purple' for option 'GlobalInvalid'");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalValidWrongCase", Map),
+

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-07-04 Thread Nathan James via Phabricator via cfe-commits 
njames93 marked an inline comment as done.
njames93 added inline comments.



Comment at: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp:225
 
+// FIXME: Figure out why this test causes crashes on mac os.
+#ifndef __APPLE__

@aaron.ballman @thakis I figured out a way to prevent the test cases failing on 
mac, still can't figure out the root cause. Would this be acceptable for now?

As far as I can see some subtle linker bug in the default mac os linker isn't 
happy when this test case is included. This actual test case runs just fine, 
but the test in ClangTidyDiagnosticsConsumerTest doesn't run properly. It 
appears to instantiate the context and then instantiate the check, but it never 
attempts to register it, causing the whole test case to fail.
clang-tidy builds just fine which has many uses of this new enum handling and 
check-clang-tools also runs without a hitch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

In D82188#2119980 , @thakis wrote:

> In this case, trunk was broken for > 12h, so I'd expect there's lots of 
> evidence of this being broken on cmake bots too. However, the mac bots aren't 
> on buildbot but on http://green.lab.llvm.org/green/ for whatever reason, and 
> I find that page hard to read. 
> http://green.lab.llvm.org/green/job/clang-stage1-RA/ has a bunch of red on 
> the LHS … ah yeah 
> http://green.lab.llvm.org/green/job/clang-stage1-RA/11966/console has "  
> Extra Tools Unit Tests :: 
> clang-tidy/./ClangTidyTests/ClangTidyDiagnosticConsumer.SortsErrors" at the 
> bottom.
>
> So this showed up on the regular mac bots, yes.
>
>  ---
>
> The tree's been broken for over half a day to this. I'd suggest taking 
> breakage supports a bit more serious going forward, instead of replying with 
> "it can't possibly be this patch" and "it must be bot setup weirdness" going 
> forward :)


Thanks for clarifying about the regular mac bots. Is this isolated to just mac 
bots?

If it is isolated then the logical reason for this failure would be a subtle 
bug in the host toolchain. 
When I landed for a second time, I noticed that the `TestCheck` in 
`ClangTidyDiagnosticsConsumerTest.cpp` gets instantiated, but the 
`registerMatchers` function never gets called, this is the root cause for why 
the test fails.
However you just need to read the the code to see how that situation shouldn't 
happen.
Without a mac to test on, I wont be able to investigate further(no pre merge 
mac test bot also makes this significantly harder for me to test), Tried 
testing on older clang compiler and wasn't able to reproduce any error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

In D82188#2119393 , @njames93 wrote:

> @thakis Do those bots use `gn`, could that be the cause of the failures?


It's possible, but unlikely, given that your change doesn't touch any build 
files.

…and indeed, I checked out `8f73c4432b5fa8510c99a5053c07dc70a610e1fb^` (the 
commit before the revert) and ran `ninja -j200 check-clang-tools` in a cmake 
build and it repros there too:

  -- Build files have been written to: /Users/thakis/src/llvm-build-project
  [2185/2306] Linking CXX static library lib/libclangDriver.a
  
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool:
 warning same member name (VE.cpp.o) in output file used for input files: 
tools/clang/lib/Driver/CMakeFiles/obj.clangDriver.dir/ToolChains/VE.cpp.o and: 
tools/clang/lib/Driver/CMakeFiles/obj.clangDriver.dir/ToolChains/Arch/VE.cpp.o 
(due to use of basename, truncation, blank padding or duplicate input files)
  [2305/2306] Running the Clang extra tools' regression tests
  FAIL: Extra Tools Unit Tests :: 
clang-tidy/./ClangTidyTests/ClangTidyDiagnosticConsumer.SortsErrors (799 of 924)
   TEST 'Extra Tools Unit Tests :: 
clang-tidy/./ClangTidyTests/ClangTidyDiagnosticConsumer.SortsErrors' FAILED 

  Note: Google Test filter = ClangTidyDiagnosticConsumer.SortsErrors
  [==] Running 1 test from 1 test case.
  [--] Global test environment set-up.
  [--] 1 test from ClangTidyDiagnosticConsumer
  [ RUN  ] ClangTidyDiagnosticConsumer.SortsErrors
  
/Users/thakis/src/llvm-project/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp:27:
 Failure
Expected: 2ul
Which is: 2
  To be equal to: Errors.size()
Which is: 0
  0  ClangTidyTests   0x000106468c65 
llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
  1  ClangTidyTests   0x000106467b36 llvm::sys::RunSignalHandlers() 
+ 198
  2  ClangTidyTests   0x000106469246 SignalHandler(int) + 262
  3  libsystem_platform.dylib 0x7fff6b5055fd _sigtramp + 29
  4  libsystem_malloc.dylib   0x7fff6b4c5a3d tiny_malloc_from_free_list + 
555
  5  ClangTidyTests   0x00010647186f testing::Test::Run() + 527
  6  ClangTidyTests   0x0001064726e6 testing::TestInfo::Run() + 566
  7  ClangTidyTests   0x000106472f17 testing::TestCase::Run() + 247
  8  ClangTidyTests   0x00010647b737 
testing::internal::UnitTestImpl::RunAllTests() + 1287
  9  ClangTidyTests   0x00010647b1bb testing::UnitTest::Run() + 171
  10 ClangTidyTests   0x00010646a4c9 main + 121
  11 libdyld.dylib0x7fff6b30ccc9 start + 1
  
  
  
  Failed Tests (1):
Extra Tools Unit Tests :: 
clang-tidy/./ClangTidyTests/ClangTidyDiagnosticConsumer.SortsErrors
  
  
  Testing Time: 21.55s
Unsupported  :   3
Passed   : 918
Expectedly Failed:   2
Failed   :   1

I then did the same again with the change reverted, and everything passes there.

> From what I can see `gn` isn't fully supported by llvm and could certainly be 
> the cause of failing builds.
>  Do we have any build bots on mac that don't use `gn` but passed the test 
> case?

It's correct that the `gn` build isn't supported (not just not fully: not at 
all) and if this would break things only there, then that wouldn't be a reason 
for revert. In practice, things very rarely are broken in the GN build only, 
and the GN bots cycle an order of magnitude faster, so they tend to show 
problems before the cmake bots do.

In this case, trunk was broken for > 12h, so I'd expect there's lots of 
evidence of this being broken on cmake bots too. However, the mac bots aren't 
on buildbot but on http://green.lab.llvm.org/green/ for whatever reason, and I 
find that page hard to read. 
http://green.lab.llvm.org/green/job/clang-stage1-RA/ has a bunch of red on the 
LHS … ah yeah http://green.lab.llvm.org/green/job/clang-stage1-RA/11966/console 
has "  Extra Tools Unit Tests :: 
clang-tidy/./ClangTidyTests/ClangTidyDiagnosticConsumer.SortsErrors" at the 
bottom.

So this showed up on the regular mac bots, yes.

---

The tree's been broken for over half a day to this. I'd suggest taking breakage 
supports a bit more serious going forward, instead of replying with "it can't 
possibly be this patch" and "it must be bot setup weirdness" going forward :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 274012.
njames93 added a comment.

Figuring out mac crash


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -6,6 +6,20 @@
 
 namespace clang {
 namespace tidy {
+
+enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+
+template <> struct OptionEnumMapping {
+  static llvm::ArrayRef> getEnumMapping() {
+static constexpr std::pair Mapping[] = {
+{Colours::Red, "Red"},   {Colours::Orange, "Orange"},
+{Colours::Yellow, "Yellow"}, {Colours::Green, "Green"},
+{Colours::Blue, "Blue"}, {Colours::Indigo, "Indigo"},
+{Colours::Violet, "Violet"}};
+return makeArrayRef(Mapping);
+  }
+};
+
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
@@ -209,15 +223,6 @@
 }
 
 TEST(ValidConfiguration, ValidEnumOptions) {
-
-  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
-  static constexpr std::pair Mapping[] = {
-  {"Red", Colours::Red},   {"Orange", Colours::Orange},
-  {"Yellow", Colours::Yellow}, {"Green", Colours::Green},
-  {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo},
-  {"Violet", Colours::Violet}};
-  static const auto Map = makeArrayRef(Mapping);
-
   ClangTidyOptions Options;
   auto  = Options.CheckOptions;
 
@@ -237,29 +242,30 @@
 #define CHECK_ERROR_ENUM(Name, Expected)   \
   CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
 
-  CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red);
-  CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet);
-  CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true),
-Colours::Red);
+  CHECK_VAL(TestCheck.getIntLocal("Valid"), Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValid"), Colours::Violet);
   CHECK_VAL(
-  TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true),
-  Colours::Violet);
-  CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map),
+  TestCheck.getIntLocal("ValidWrongCase", /*IgnoreCase*/ true),
+  Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValidWrongCase",
+/*IgnoreCase*/ true),
+Colours::Violet);
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalInvalid"),
"invalid configuration value "
"'Purple' for option 'GlobalInvalid'");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalValidWrongCase"),

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

@thakis Do those bots use `gn`, could that be the cause of the failures? 
From what I can see `gn` isn't fully supported by llvm and could certainly be 
the cause of failing builds.
Do we have any build bots on mac that don't use `gn` but passed the test case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nathan James via Phabricator via cfe-commits 
njames93 marked an inline comment as done.
njames93 added inline comments.



Comment at: 
clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp:31
+  ~TestCheck() {
+assert(DidRegister && "Check never registered");
+assert(DidFire && "Check never fired");

This assert is failing on the mac builds - 
http://45.33.8.238/mac/16309/step_8.txt yet I can see no logical reason for it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

I've relanded this with some sanity checks in 
37cc4fa2eaa3d03ca8cd4947eb0d4c60e3c9b45c 
. If it 
fails again without those asserts I'll go back to the drawing board


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

In D82188#2118885 , @thakis wrote:

> It's the only change on the blame list, things pass consistently before the 
> change, and fail consistently after it.


Do you happen to have a proper build log for the failing build. That bot 
doesn't show what happened when building, in fact it says there was no work to 
do when building which seems to confuse me. My best guess for the fail is 
infrastructure related.

Edit: http://45.33.8.238/mac/16282/log.txt this is the log I want. It shows the 
failure and that this is the cause, but I'm still struggling to see what the 
actual cause is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

I reverted this in 8f73c4432b5fa8510c99a5053c07dc70a610e1fb 
 and 
check-clang-tools stopped failing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

It's the only change on the blame list, things pass consistently before the 
change, and fail consistently after it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nathan James via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9306fd042ce: [clang-tidy] Reworked enum options 
handling(again) (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -6,6 +6,20 @@
 
 namespace clang {
 namespace tidy {
+
+enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+
+template <> struct OptionEnumMapping {
+  static llvm::ArrayRef> getEnumMapping() {
+static constexpr std::pair Mapping[] = {
+{Colours::Red, "Red"},   {Colours::Orange, "Orange"},
+{Colours::Yellow, "Yellow"}, {Colours::Green, "Green"},
+{Colours::Blue, "Blue"}, {Colours::Indigo, "Indigo"},
+{Colours::Violet, "Violet"}};
+return makeArrayRef(Mapping);
+  }
+};
+
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
@@ -209,15 +223,6 @@
 }
 
 TEST(ValidConfiguration, ValidEnumOptions) {
-
-  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
-  static constexpr std::pair Mapping[] = {
-  {"Red", Colours::Red},   {"Orange", Colours::Orange},
-  {"Yellow", Colours::Yellow}, {"Green", Colours::Green},
-  {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo},
-  {"Violet", Colours::Violet}};
-  static const auto Map = makeArrayRef(Mapping);
-
   ClangTidyOptions Options;
   auto  = Options.CheckOptions;
 
@@ -237,29 +242,30 @@
 #define CHECK_ERROR_ENUM(Name, Expected)   \
   CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
 
-  CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red);
-  CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet);
-  CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true),
-Colours::Red);
+  CHECK_VAL(TestCheck.getIntLocal("Valid"), Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValid"), Colours::Violet);
   CHECK_VAL(
-  TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true),
-  Colours::Violet);
-  CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map),
+  TestCheck.getIntLocal("ValidWrongCase", /*IgnoreCase*/ true),
+  Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValidWrongCase",
+/*IgnoreCase*/ true),
+Colours::Violet);
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalInvalid"),
"invalid configuration value "
"'Purple' for option 'GlobalInvalid'");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalValidWrongCase"),

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

In D82188#2118769 , @thakis wrote:

> This breaks chevk-clang-tools in mac: http://45.33.8.238/mac/16292/step_8.txt
>
> Please take a look and revert for now if it takes a while to fix.


Are you sure that this is the offending patch? I don't touch the diagnostics 
consumer in this patch and this is essentially NFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-29 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

This breaks chevk-clang-tools in mac: http://45.33.8.238/mac/16292/step_8.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-21 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 272336.
njames93 added a comment.

- Fix compilation failure on rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -6,6 +6,20 @@
 
 namespace clang {
 namespace tidy {
+
+enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+
+template <> struct OptionEnumMapping {
+  static llvm::ArrayRef> getEnumMapping() {
+static constexpr std::pair Mapping[] = {
+{Colours::Red, "Red"},   {Colours::Orange, "Orange"},
+{Colours::Yellow, "Yellow"}, {Colours::Green, "Green"},
+{Colours::Blue, "Blue"}, {Colours::Indigo, "Indigo"},
+{Colours::Violet, "Violet"}};
+return makeArrayRef(Mapping);
+  }
+};
+
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
@@ -209,15 +223,6 @@
 }
 
 TEST(ValidConfiguration, ValidEnumOptions) {
-
-  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
-  static constexpr std::pair Mapping[] = {
-  {"Red", Colours::Red},   {"Orange", Colours::Orange},
-  {"Yellow", Colours::Yellow}, {"Green", Colours::Green},
-  {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo},
-  {"Violet", Colours::Violet}};
-  static const auto Map = makeArrayRef(Mapping);
-
   ClangTidyOptions Options;
   auto  = Options.CheckOptions;
 
@@ -237,29 +242,30 @@
 #define CHECK_ERROR_ENUM(Name, Expected)   \
   CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
 
-  CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red);
-  CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet);
-  CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true),
-Colours::Red);
+  CHECK_VAL(TestCheck.getIntLocal("Valid"), Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValid"), Colours::Violet);
   CHECK_VAL(
-  TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true),
-  Colours::Violet);
-  CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map),
+  TestCheck.getIntLocal("ValidWrongCase", /*IgnoreCase*/ true),
+  Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValidWrongCase",
+/*IgnoreCase*/ true),
+Colours::Violet);
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalInvalid"),
"invalid configuration value "
"'Purple' for option 'GlobalInvalid'");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalValidWrongCase"),
"invalid configuration value 'vIOLET' for option "

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-21 Thread Nathan James via Phabricator via cfe-commits 
njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:31-32
+template  struct OptionEnumMapping {
+  // Must provide:
+  // static ArrayRef> getEnumMapping();
+};

aaron.ballman wrote:
> Any reason not to do:
> ```
> static ArrayRef> getEnumMapping() = delete;
> ```
> so you get a nice error if you instantiate something without the required 
> definition?
I need to read the c++ standard, never even realised you could delete static 
functions like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-21 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 272317.
njames93 marked 2 inline comments as done.
njames93 added a comment.

Use `= delete` for getEnumMapping in template definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -6,6 +6,20 @@
 
 namespace clang {
 namespace tidy {
+
+enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+
+template <> struct OptionEnumMapping {
+  static llvm::ArrayRef> getEnumMapping() {
+static constexpr std::pair Mapping[] = {
+{Colours::Red, "Red"},   {Colours::Orange, "Orange"},
+{Colours::Yellow, "Yellow"}, {Colours::Green, "Green"},
+{Colours::Blue, "Blue"}, {Colours::Indigo, "Indigo"},
+{Colours::Violet, "Violet"}};
+return makeArrayRef(Mapping);
+  }
+};
+
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
@@ -209,15 +223,6 @@
 }
 
 TEST(ValidConfiguration, ValidEnumOptions) {
-
-  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
-  static constexpr std::pair Mapping[] = {
-  {"Red", Colours::Red},   {"Orange", Colours::Orange},
-  {"Yellow", Colours::Yellow}, {"Green", Colours::Green},
-  {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo},
-  {"Violet", Colours::Violet}};
-  static const auto Map = makeArrayRef(Mapping);
-
   ClangTidyOptions Options;
   auto  = Options.CheckOptions;
 
@@ -237,29 +242,30 @@
 #define CHECK_ERROR_ENUM(Name, Expected)   \
   CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
 
-  CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red);
-  CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet);
-  CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true),
-Colours::Red);
+  CHECK_VAL(TestCheck.getIntLocal("Valid"), Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValid"), Colours::Violet);
   CHECK_VAL(
-  TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true),
-  Colours::Violet);
-  CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map),
+  TestCheck.getIntLocal("ValidWrongCase", /*IgnoreCase*/ true),
+  Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValidWrongCase",
+/*IgnoreCase*/ true),
+Colours::Violet);
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalInvalid"),
"invalid configuration value "
"'Purple' for option 'GlobalInvalid'");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalValidWrongCase"),

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-21 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

I like the direction of this, thank you! LGTM with a small suggestion, but you 
should wait a few days in case one of the other reviewers has comments.




Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:31-32
+template  struct OptionEnumMapping {
+  // Must provide:
+  // static ArrayRef> getEnumMapping();
+};

Any reason not to do:
```
static ArrayRef> getEnumMapping() = delete;
```
so you get a nice error if you instantiate something without the required 
definition?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-21 Thread Nathan James via Phabricator via cfe-commits 
njames93 updated this revision to Diff 272306.
njames93 added a comment.

Update doc comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -6,6 +6,20 @@
 
 namespace clang {
 namespace tidy {
+
+enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+
+template <> struct OptionEnumMapping {
+  static llvm::ArrayRef> getEnumMapping() {
+static constexpr std::pair Mapping[] = {
+{Colours::Red, "Red"},   {Colours::Orange, "Orange"},
+{Colours::Yellow, "Yellow"}, {Colours::Green, "Green"},
+{Colours::Blue, "Blue"}, {Colours::Indigo, "Indigo"},
+{Colours::Violet, "Violet"}};
+return makeArrayRef(Mapping);
+  }
+};
+
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
@@ -209,15 +223,6 @@
 }
 
 TEST(ValidConfiguration, ValidEnumOptions) {
-
-  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
-  static constexpr std::pair Mapping[] = {
-  {"Red", Colours::Red},   {"Orange", Colours::Orange},
-  {"Yellow", Colours::Yellow}, {"Green", Colours::Green},
-  {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo},
-  {"Violet", Colours::Violet}};
-  static const auto Map = makeArrayRef(Mapping);
-
   ClangTidyOptions Options;
   auto  = Options.CheckOptions;
 
@@ -237,29 +242,30 @@
 #define CHECK_ERROR_ENUM(Name, Expected)   \
   CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
 
-  CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red);
-  CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet);
-  CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true),
-Colours::Red);
+  CHECK_VAL(TestCheck.getIntLocal("Valid"), Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValid"), Colours::Violet);
   CHECK_VAL(
-  TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true),
-  Colours::Violet);
-  CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map),
+  TestCheck.getIntLocal("ValidWrongCase", /*IgnoreCase*/ true),
+  Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValidWrongCase",
+/*IgnoreCase*/ true),
+Colours::Violet);
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalInvalid"),
"invalid configuration value "
"'Purple' for option 'GlobalInvalid'");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalValidWrongCase"),
"invalid configuration value 'vIOLET' for option "

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-19 Thread Nathan James via Phabricator via cfe-commits 
njames93 marked an inline comment as done.
njames93 added inline comments.
Herald added a subscriber: wuzish.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:30
   Options.store(Opts, "GslHeader", GslHeader);
   Options.store(Opts, "IncludeStyle", IncludeStyle);
 }

As a side bonus, this originally incorrect code would cast IncludeStyle to an 
int, then store that value. Now it will store it as the correct enum. Any other 
enums passed to store will error out if there is no mapping specialization for 
them, forcing you to either declare the mapping or cast it to an integer type


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-06-19 Thread Nathan James via Phabricator via cfe-commits 
njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, gribozavr2.
Herald added subscribers: cfe-commits, arphaman, kbarton, xazax.hun, nemanjai.
Herald added a project: clang.

Following on from D77085 , I was never happy 
with the passing a mapping to the option get/store functions. This patch 
addresses this by using explicit specializations to handle the serializing and 
deserializing of enum options.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82188

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -6,6 +6,20 @@
 
 namespace clang {
 namespace tidy {
+
+enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+
+template <> struct OptionEnumMapping {
+  static llvm::ArrayRef> getEnumMapping() {
+static constexpr std::pair Mapping[] = {
+{Colours::Red, "Red"},   {Colours::Orange, "Orange"},
+{Colours::Yellow, "Yellow"}, {Colours::Green, "Green"},
+{Colours::Blue, "Blue"}, {Colours::Indigo, "Indigo"},
+{Colours::Violet, "Violet"}};
+return makeArrayRef(Mapping);
+  }
+};
+
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
@@ -209,15 +223,6 @@
 }
 
 TEST(ValidConfiguration, ValidEnumOptions) {
-
-  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
-  static constexpr std::pair Mapping[] = {
-  {"Red", Colours::Red},   {"Orange", Colours::Orange},
-  {"Yellow", Colours::Yellow}, {"Green", Colours::Green},
-  {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo},
-  {"Violet", Colours::Violet}};
-  static const auto Map = makeArrayRef(Mapping);
-
   ClangTidyOptions Options;
   auto  = Options.CheckOptions;
 
@@ -237,29 +242,30 @@
 #define CHECK_ERROR_ENUM(Name, Expected)   \
   CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
 
-  CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red);
-  CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet);
-  CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true),
-Colours::Red);
+  CHECK_VAL(TestCheck.getIntLocal("Valid"), Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValid"), Colours::Violet);
   CHECK_VAL(
-  TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true),
-  Colours::Violet);
-  CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map),
+  TestCheck.getIntLocal("ValidWrongCase", /*IgnoreCase*/ true),
+  Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValidWrongCase",
+/*IgnoreCase*/ true),
+Colours::Violet);
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalInvalid"),
"invalid