================
Comment at: clang-tidy/ClangTidy.h:52-63
@@ -49,12 +51,14 @@
/// \p Default.
- std::string get(StringRef LocalName, std::string Default) const;
+ std::string get(StringRef LocalName, StringRef Default) const;
- /// \brief Read a named option from the \c Context and parse it as an
integral
- /// type \c T.
+ /// \brief Read a named option from the \c Context and parse it as any
+ /// type \c T that has ScalarTraits<>.
///
/// Reads the option with the check-local name \p LocalName from the
/// \c CheckOptions. If the corresponding key is not present, returns
/// \p Default.
template <typename T>
- typename std::enable_if<std::is_integral<T>::value, T>::type
+ typename std::enable_if<llvm::yaml::has_ScalarTraits<T>::value,
+ llvm::ErrorOr<T>>::type
get(StringRef LocalName, T Default) const {
+ std::string String = get(LocalName, "");
----------------
curdeius wrote:
> alexfh wrote:
> > curdeius wrote:
> > > Won't it be counterintuitive for the user of this code that `get(...,
> > > string)` returns a `string` and that `get(..., T)` returns `ErrorOr<T>`?
> > The difference is that the string version doesn't do any parsing, so it
> > can't encounter any errors. So maybe it should be named differently from
> > the template version to avoid confusion.
> Yes, I know that, but still it's a bit weird in my eyes to have functions
> with same name behaving as differently as this.
> Renaming it is one valid approach.
> Another one would be to add an argument indicating that something went wrong,
> e.g. `get(..., T Default, bool *Invalid = nullptr)`.
> It would be up to the user to handle the error or not (on error, `get` will
> return Default).
> In any case, we don't pass any more information with error_code than with a
> boolean...
>
> What are your thoughts about it?
ScalarTraits::input returns a StringRef describing the error. We can also use
StringRef to avoid impedance mismatch and make use of these strings in our
error messages.
Or maybe ScalarTraits<> should be changed to use std::error_code with custom
errors. I'll ask the authors' opinion.
http://reviews.llvm.org/D5602
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
