mikecrowe added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:189
+ getLangOpts().CPlusPlus2b
+ ? hasAnyName("::std::print", "::std::format")
+ : hasName("::std::format"))),
----------------
mikecrowe wrote:
> PiotrZSL wrote:
> > Please introduce configuration option to specify custom functions.
> > For example if some project (like mine) is wrapping fmt::format with some
> > variadic template function, then such function could be specified.
> > Same goes to things like some loggers.
> >
> > Check utils/OptionsUtils.h for configuration, and utils/Matchers.h
> > (matchesAnyListedName)
> > Please introduce configuration option to specify custom functions.
> > For example if some project (like mine) is wrapping fmt::format with some
> > variadic template function, then such function could be specified.
>
> That's exactly where this change originated (as part of [[
> https://github.com/mikecrowe/clang-tidy-fmt | my clang-tidy fmt fork ]],
> which I hope to submit for review soon once I've made it configurable too and
> improved the test cases.)
>
> > Same goes to things like some loggers.
> >
> > Check utils/OptionsUtils.h for configuration, and utils/Matchers.h
> > (matchesAnyListedName)
>
> Thanks for the pointer. I shall study those files for how to support this.
>
> Do you think that the change can land like in its current state first? Or
> would you prefer that the configuration option is added at the same time?
> Please introduce configuration option to specify custom functions.
> For example if some project (like mine) is wrapping fmt::format with some
> variadic template function, then such function could be specified.
I could add some sort of `readability-redundant-string-cstr.PrintFunction`
option or `readability-redundant-string-cstr.FormatFunction` option, but
reducing this to its fundamental behaviour would be "a function that takes
`const char *` arguments that is also willing to take `std::string` arguments".
I'm struggling to find a sensible name for such an option. Maybe
`readability-redundant-string-cstr.StdStringAcceptingFunction=::fmt::format`?
Or just `readability-redundant-string-cstr.FunctionCall=::fmt::format`?
> Same goes to things like some loggers.
Loggers may be classes, so there would need to be an option that specifies a
class name (or even a base class name) and a method name (which may be an
operator.) See [[
https://github.com/mikecrowe/clang-tidy-fmt/blob/7ace8a3ff41e9679104fe558835b0ef3cb33d969/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp#L196
| this hard-coded example ]] (If it's not obvious, there's a description in
the [[
https://github.com/mikecrowe/clang-tidy-fmt/blob/7ace8a3ff41e9679104fe558835b0ef3cb33d969/README.md?plain=1#L103
| README ]]. In such cases the options could be
`readability-redundant-string-cstr.Class=::BaseTrace` and
`readability-redundant-string-cstr.Method=Log` or
`readability-redundant-string-cstr.Operator=()`, but then it would be hard to
tie together the right classes and methods. That could be avoided with
something like
`readability-redundant-string-cstr.MemberFunctionCall=::BaseTrace::operator(),::NullTrace::operator()`
and some parsing I suppose.
Regardless, I'll try and get the simple case working and await suggestions for
appropriate option names.
Thanks again for the suggestions.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143342/new/
https://reviews.llvm.org/D143342
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
