MINIFICPP-2248 Refactor string::join_pack #1680
Conversation
| } | ||
| template<typename Str> struct char_type_of_impl {}; | ||
| template<typename CharT> struct char_type_of_impl<std::basic_string<CharT>> : std::type_identity<CharT> {}; | ||
| template<typename CharT> struct char_type_of_impl<CharT*> : std::type_identity<std::remove_cv_t<CharT>> {}; |
There was a problem hiding this comment.
const char* and char* are both considered strings of char.
| template<typename CharT> struct char_type_of_impl<std::basic_string<CharT>> : std::type_identity<CharT> {}; | ||
| template<typename CharT> struct char_type_of_impl<CharT*> : std::type_identity<std::remove_cv_t<CharT>> {}; | ||
| template<typename CharT> struct char_type_of_impl<std::basic_string_view<CharT>> : std::type_identity<CharT> {}; | ||
| template<typename Str> using char_type_of = typename char_type_of_impl<std::decay_t<Str>>::type; |
There was a problem hiding this comment.
Decay here is intentional: not only do we want to ignore cv-qualifiers and references, but char arrays should be decayed to pointers, to match the second specialization above.
| @@ -1,2 +1,2 @@ | |||
| set noparent | |||
| filter=-runtime/reference,-runtime/string,-build/c++11,-build/include_subdir,-whitespace/forcolon,-build/namespaces_literals,-readability/check,-build/include_what_you_use,-readability/nolint | |||
| filter=-runtime/reference,-runtime/string,-build/c++11,-build/include_subdir,-whitespace/forcolon,-build/namespaces_literals,-readability/check,-build/include_what_you_use,-readability/nolint,-readability/braces | |||
There was a problem hiding this comment.
Got this after both concept declarations:
/home/szaszm/nifi-minifi-cpp-3/libminifi/include/utils/StringUtils.h:193: You don't need a ; after a } [readability/braces] [4]
There was a problem hiding this comment.
@martinzink has an open issue for this for a while now google/styleguide#752
There was a problem hiding this comment.
To be honest, I have the feeling that we would be better off by dropping cpplint, and rely more on clang-tidy, especially as you recent changes in #1671 get ready and do the google-specific checks.
Too many false positives, and it looks outdated barely maintained. On the other hand, it runs much faster than clang-tidy.
There was a problem hiding this comment.
I think it's still worth having cpplint as well, I didn't encounter too many false positives and it has checks that clang tidy does not. It also runs on the Windows codebase while clang tidy only runs on Linux at the moment.
| @@ -1,2 +1,2 @@ | |||
| set noparent | |||
| filter=-runtime/reference,-runtime/string,-build/c++11,-build/include_subdir,-whitespace/forcolon,-build/namespaces_literals,-readability/check,-build/include_what_you_use,-readability/nolint | |||
| filter=-runtime/reference,-runtime/string,-build/c++11,-build/include_subdir,-whitespace/forcolon,-build/namespaces_literals,-readability/check,-build/include_what_you_use,-readability/nolint,-readability/braces | |||
There was a problem hiding this comment.
@martinzink has an open issue for this for a while now google/styleguide#752
Signed-off-by: Marton Szasz <szaszm@apache.org>
szaszm
force-pushed
the
MINIFICPP-2248
branch
from
e26bfc4
to
a40ac18
Compare
-fconceptsflag fromRangeV3.cmakereadability/bracesof cpplint, because it can't handle conceptsThank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically main)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.