LegalizeAdulthood added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:51
ClangTidyContext *Context)
- : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions),
+ : NamePrefix((CheckName + ".").str()), CheckOptions(CheckOptions),
Context(Context) {}
----------------
Why is `StringRef::operator+` considered "better" than `std::string::operator+`?
================
Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:56
ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
- const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str());
+ const auto &Iter = CheckOptions.find((NamePrefix + LocalName).str());
if (Iter != CheckOptions.end())
----------------
`find` takes a `StringRef` so why convert to `std::string` here?
================
Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:123
StringRef Value) const {
- Options[NamePrefix + LocalName.str()] = Value;
+ Options[(NamePrefix + LocalName).str()] = Value;
}
----------------
`operator[]` takes a `StringRef` so why convert to `std::string` here?
================
Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:158
/// ``None``.
- llvm::Optional<std::string> get(StringRef LocalName) const;
+ llvm::Optional<StringRef> get(StringRef LocalName) const;
----------------
I think I would feel safer if the changes to the infrastructure were pushed
separately from the changes to the checks, with some "bake time" in between so
we have more confidence in the changes.
While debugging my own checks, I've seen that there are surprises with the
lifetimes of `StringRef`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124341/new/
https://reviews.llvm.org/D124341
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
