[PATCH] D75184: [clang-tidy] Optional inheritance of file configs from parent directories

[Alexander Kornienko via Phabricator via cfe-commits]

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

Apologies for the delay! It's sort of a crazy time now =\

The code looks mostly good now modulo a few comments inline.



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:52
+      (IterGlobal == CheckOptions.end() ||
+       IterGlobal->second.Priority < IterLocal->second.Priority))
+    return IterLocal->second.Value;
----------------
supernit: Let's swap the compared values to make the code slightly more intuitive ("if local is higher priority, use local" vs "if global is lower priority, use local").


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:73
+    for (const auto &KeyValue : OptionMap)
+      Options.push_back(std::make_pair(KeyValue.first, KeyValue.second.Value));
+  }
----------------
nit: would emplace_back work?


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.h:101-104
+    ClangTidyValue(const char *Value) : Value(Value), Priority(0) {}
+    ClangTidyValue(const std::string &Value) : Value(Value), Priority(0) {}
+    ClangTidyValue(const std::string &Value, unsigned Priority)
+        : Value(Value), Priority(Priority) {}
----------------
Maybe just `ClangTidyValue(StringRef Value, unsigned Priority = 0)`?


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.h:107
+    std::string Value;
+    unsigned Priority;
+  };
----------------
Could you describe how the priority is used?


================
Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:41
+  in the parent directory (if any exists) will be taken and current config file
+  will be applied on top of the parent one. If any configuration options have
+  a corresponding command-line option, command-line option takes precedence.
----------------
Does the new logic related to local and global options deserve a separate mention here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75184