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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
