Copilot commented on code in PR #2155:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2155#discussion_r3051397186
##########
core-framework/common/include/utils/expected.h:
##########
@@ -245,25 +245,25 @@ concept HasStdExpected = requires { typename
std::expected<T, E>; };
template <typename T, typename E>
concept ExpectedTypesDoNotConflict =
(!HasStdExpected<T, E> ||
- !std::same_as<nonstd::expected<T, E>, std::expected<T, E>>);
+ !std::same_as<std::expected<T, E>, std::expected<T, E>>);
Review Comment:
`ExpectedTypesDoNotConflict` is currently always false whenever
`HasStdExpected<T, E>` is true, because `std::same_as<std::expected<T, E>,
std::expected<T, E>>` is always true. That disables the
`fmt::formatter<std::expected<...>>` specialization, so formatting
`std::expected` will either fail to compile or silently lose the intended
formatting behavior. Fix by removing/rewriting this conflict-avoidance concept
(e.g., guard the specialization based on whether {fmt} already provides a
std::expected formatter, or key off {fmt} version feature macros) so the
formatter is actually enabled when needed.
##########
core-framework/common/include/utils/expected.h:
##########
@@ -245,25 +245,25 @@ concept HasStdExpected = requires { typename
std::expected<T, E>; };
template <typename T, typename E>
concept ExpectedTypesDoNotConflict =
(!HasStdExpected<T, E> ||
- !std::same_as<nonstd::expected<T, E>, std::expected<T, E>>);
+ !std::same_as<std::expected<T, E>, std::expected<T, E>>);
// based on fmt::formatter<std::expected<T, E>, Char>
template <typename T, typename E, typename Char>
requires ExpectedTypesDoNotConflict<T, E> &&
(std::is_void_v<T> || fmt::is_formattable<T, Char>::value) &&
fmt::is_formattable<E, Char>::value
-struct fmt::formatter<nonstd::expected<T, E>, Char> {
+struct fmt::formatter<std::expected<T, E>, Char> {
Review Comment:
`ExpectedTypesDoNotConflict` is currently always false whenever
`HasStdExpected<T, E>` is true, because `std::same_as<std::expected<T, E>,
std::expected<T, E>>` is always true. That disables the
`fmt::formatter<std::expected<...>>` specialization, so formatting
`std::expected` will either fail to compile or silently lose the intended
formatting behavior. Fix by removing/rewriting this conflict-avoidance concept
(e.g., guard the specialization based on whether {fmt} already provides a
std::expected formatter, or key off {fmt} version feature macros) so the
formatter is actually enabled when needed.
##########
core-framework/src/utils/Cron.cpp:
##########
@@ -46,14 +46,15 @@ namespace org::apache::nifi::minifi::utils {
namespace {
// https://github.com/HowardHinnant/date/issues/550
-// Due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78714
-// the month parsing with '%b' and the weekday parsing with '%a' is
case-sensitive in gcc11
-// This has been fixed in gcc13
+// Due to libstdc++ bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78714
+// the month parsing with '%b' and the weekday parsing with '%a' can be
case-sensitive.
+// Normalizing to Title Case prevents parsing failures across all GCC versions.
std::stringstream getCaseInsensitiveCStream(const std::string& str) {
-#if defined(__GNUC__) && (__GNUC__ < 13)
+#if defined(__GNUC__) && !defined(__clang__)
auto patched_str = string::toLower(str);
- if (!patched_str.empty())
+ if (!patched_str.empty()) {
patched_str[0] = static_cast<char>(std::toupper(static_cast<unsigned
char>(patched_str[0])));
+ }
Review Comment:
The comment describes a libstdc++ behavior, but the preprocessor condition
excludes Clang (`!defined(__clang__)`). Clang builds that use libstdc++ can
still hit the same parsing behavior, so this change can reintroduce parsing
failures in clang+libstdc++ environments. Consider keying the workaround on
libstdc++ (e.g., `__GLIBCXX__` / `_GLIBCXX_RELEASE` checks) rather than the
compiler, or otherwise ensure the normalization applies whenever the underlying
stdlib exhibits the behavior.
--
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]
