[PATCH] D159436: [clang-tidy] Add support for optional parameters in config.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:196-198
+        if (Value == "" || Value == "none" || Value == "null" ||
+          Value == "false" || (std::is_unsigned_v<T> && Value == "-1"))
+              return std::nullopt;
----------------
To be honest I wanted this to work only if we call get with Default being std::optional<T>, and such method should return optional, and store methods should be also updated.
And next time use github pull requests.


================
Comment at: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp:18
+// RUN:   -config='{CheckOptions: { \
+// RUN:     bugprone-easily-swappable-parameters.MinimumLength: "false", \
+// RUN:  }}' --
----------------
carlosgalvezp wrote:
> What if:
> 
> - "MinimumLength" is a boolean.
> - It's default value (if not specified) is True.
> - And a user wants to set it as "False" here
> 
> Wouldn't that cause problems?
> 
this functionality should work only for integers...
Idea is to have:
```
template <typename T>
std::enable_if_t<std::is_integral_v<T>, std::optional<T>>
getLocalOrGlobal(StringRef LocalName, std::optional<T> Default) const;

 template <typename T>
 std::enable_if_t<std::is_integral_v<T>, std::optional<T>>
 get(StringRef LocalName, std::optional<T> Default) const;
```

without impacting other functions... so that instead of doing tricks with -1 in code, we could simply create real optional options, and be able to 'store' them as optional, this mean that --dump-config should dump them as "none".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436