szaszm commented on a change in pull request #1267:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1267#discussion_r813236588
##########
File path: libminifi/include/core/logging/Logger.h
##########
@@ -53,28 +54,72 @@ class LoggerControl {
std::atomic<bool> is_enabled_;
};
-inline char const* conditional_conversion(std::string const& str) {
- return str.c_str();
-}
+template<typename Arg>
+using has_const_c_str_method = decltype(std::declval<const Arg&>().c_str());
+
+template<typename Arg>
+using has_str_method = decltype(std::declval<Arg>().str());
+
+template<typename Arg, typename = void>
+struct StringConverter {
+ decltype(auto) operator()(Arg&& val) const {
+ return std::forward<Arg>(val);
+ }
+};
+
+template<typename Arg>
+struct StringConverter<Arg, std::enable_if_t<
+ !utils::meta::is_detected_v<has_const_c_str_method, Arg> &&
+ utils::meta::is_detected_v<has_str_method, Arg>>> {
+ decltype(auto) operator()(Arg&& val) const {
+ return std::forward<Arg>(val).str();
+ }
+};
+
+template<typename Arg>
+struct StringConverter<Arg, std::enable_if_t<
+ !utils::meta::is_detected_v<has_const_c_str_method, Arg> &&
+ !utils::meta::is_detected_v<has_str_method, Arg> &&
+ std::is_invocable_v<Arg>>> {
+ decltype(auto) operator()(Arg&& val) const {
+ return std::forward<Arg>(val)();
+ }
+};
+
+template<typename Arg, typename = void>
+struct CStringConverter;
+
+template<typename Arg>
+struct CStringConverter<Arg, std::enable_if_t<
+ std::is_same_v<decltype(std::declval<const Arg&>().c_str()), const
char*>>> {
+ const char* operator()(const Arg& val) {
+ return val.c_str();
+ }
+};
+
+template<typename Arg>
+struct CStringConverter<Arg, std::enable_if_t<
+ std::is_scalar_v<std::decay_t<Arg>>>> {
+ const Arg& operator()(const Arg& val) {
+ return val;
+ }
+};
-template<size_t N>
-inline char const* conditional_conversion(const utils::SmallString<N>& arr) {
- return arr.c_str();
+template<typename Arg>
+inline decltype(auto) conditional_stringify(Arg&& arg) {
+ return StringConverter<Arg>{}(std::forward<Arg>(arg));
Review comment:
I like the lazy evaluation feature, but I think if a string is really
expensive to create, then the calling site should check the log level, and we
should keep the logger simple ([and
stupid](https://en.wikipedia.org/wiki/KISS_principle)). Or at least I did it
like that at PutUDP.cpp:120.
I'm OK with this limited `c_str()` detection, but I can see it getting
removed in a future logger refactor. I want to eventually get closer to raw
spdlog + fmtlog logger syntax, which uses `std::formatter`-like formatters.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
