[PATCH] [clang-tidy] Add support for boolean check options.
Marek Kurdej
curdeius at gmail.com
Mon Oct 6 01:52:14 PDT 2014
We can generalize go a step further `get()` and `store()` to all types that specialize `llvm::yaml` traits.
This could remove the code duplication and simplify the creation of clang-tidy checks having options of enumeration type.
================
Comment at: clang-tidy/ClangTidy.h:50
@@ -49,3 +49,3 @@
/// \p Default.
- std::string get(StringRef LocalName, std::string Default) const;
+ std::string get(StringRef LocalName, const char *Default) const;
----------------
What's the reason for changing `std::string Default` to `const char* Default`?
It "breaks" somehow the symmetry of `T get(StringRef, T)`, i.e. taking `T` type as Default and returning `T`.
================
Comment at: clang-tidy/ClangTidy.h:73
@@ +72,3 @@
+ /// \p Default.
+ bool get(StringRef LocalName, bool Default) const {
+ std::string Value = get(LocalName, "");
----------------
curdeius wrote:
> Couldn't we just generalize this approach to all the types that specialize `llvm::yaml::XYZTraits` (XYZ being one of Scalar/ScalarEnumeration/...) ?
>
> An ugly hack (working at least for `enum`s specializing `ScalarEnumerationTraits`) would be:
>
> ```
> template <typename T>
> typename std::enable_if<!std::is_integral<T>::value &&
> !std::is_convertible<T, std::string>::value,
> T>::type
> get(StringRef LocalName, T Default) const {
> std::string Value = get(LocalName, "");
> T Result = Default;
> if (!Value.empty()) {
> std::stringstream Content;
> Content << LocalName.str() << ": " << Value;
> llvm::yaml::Input Input(Content.str());
> Input.setCurrentDocument();
> Input.mapOptional(LocalName.data(), Result);
> if (Input.error())
> Result = Default;
> }
> return Result;
> }
> ```
>
> The hack is ugly since it uses `Input::setCurrentDocument()` (which should probably be private) and `Input::mapOptional()` and not `Input::operator>>`. That is so, because otherwise we would need creating a type specializing `llvm::yaml::MappingTraits` which would essentially do an operation equivalent to `Input.mapOptional(LocalName.str(), Result);`.
That's essentially the same code as in `llvm/lib/Support/YAMLTraits.cpp::651`.
================
Comment at: clang-tidy/ClangTidy.h:73-80
@@ +72,10 @@
+ /// \p Default.
+ bool get(StringRef LocalName, bool Default) const {
+ std::string Value = get(LocalName, "");
+ if (Value == "true")
+ return true;
+ if (Value == "false")
+ return false;
+ return Default;
+ }
+
----------------
Couldn't we just generalize this approach to all the types that specialize `llvm::yaml::XYZTraits` (XYZ being one of Scalar/ScalarEnumeration/...) ?
An ugly hack (working at least for `enum`s specializing `ScalarEnumerationTraits`) would be:
```
template <typename T>
typename std::enable_if<!std::is_integral<T>::value &&
!std::is_convertible<T, std::string>::value,
T>::type
get(StringRef LocalName, T Default) const {
std::string Value = get(LocalName, "");
T Result = Default;
if (!Value.empty()) {
std::stringstream Content;
Content << LocalName.str() << ": " << Value;
llvm::yaml::Input Input(Content.str());
Input.setCurrentDocument();
Input.mapOptional(LocalName.data(), Result);
if (Input.error())
Result = Default;
}
return Result;
}
```
The hack is ugly since it uses `Input::setCurrentDocument()` (which should probably be private) and `Input::mapOptional()` and not `Input::operator>>`. That is so, because otherwise we would need creating a type specializing `llvm::yaml::MappingTraits` which would essentially do an operation equivalent to `Input.mapOptional(LocalName.str(), Result);`.
================
Comment at: clang-tidy/ClangTidy.h:103-104
@@ +102,4 @@
+ /// a \c bool value \p Value to \p Options.
+ void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+ bool Value) const;
+
----------------
Similarly to above `get()` method, one can generalize the approach.
Another ugly hack would be:
```
template <typename T>
typename std::enable_if<!std::is_integral<T>::value &&
!std::is_convertible<T, std::string>::value,
void>::type
store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
T Value) const {
std::string Text;
llvm::raw_string_ostream Content(Text);
llvm::yaml::Output Output(Content);
Output.beginMapping();
if (Output.preflightDocument(0))
Output.mapOptional(LocalName.data(), Value);
Output.endMapping();
auto T = Content.str();
// Skip past ": " between key and value.
auto Off = T.find(':', LocalName.size());
assert(Off != std::string::npos);
T = T.substr(Off + 2, std::string::npos);
// FIXME(mkurdej): Strip enclosing apostrophes "'" if present.
Options[NamePrefix + LocalName.str()] = T;
}
```
http://reviews.llvm.org/D5602
More information about the cfe-commits
mailing list