flx added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:52
AllowedTypes(
utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
----------------
lebedev.ri wrote:
> flx wrote:
> > lebedev.ri wrote:
> > > Just put it here?
> > I tried this first, but this list is substring matched against only on the
> > non-qualified type name, so std::function would not match anything and if
> > we added "function" here it would match many other types that contain the
> > word function.
> I would personally say it's a bug, especially because i personally don't like
> hardcoded "choices" that are impossible to change.
>
Are you referring to how the matching of "AllowedTypes" works or the fact that
std::function causes false positives?
The latter is currently a bug and by excluding it now its severity is
downgraded from bug to missing feature. Is your concern that you would like to
have std::function be matched even if it is a false positive?
Regarding "AllowedTypes" matching I'm also surprised to find it matches
substrings without namespaces as the name doesn't communicate this and makes it
an imprecise net to cast. Changing it could break existing users that have type
substrings configured though.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:409
+
+namespace std {
+
----------------
gribozavr2 wrote:
> Could you add a nested inline namespace to better imitate what declarations
> look like in libc++?
I'm not sure I follow. I looked through the other tests that declare a std
function and copied the declaration from modernize-avoid-bind.cpp.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89332/new/
https://reviews.llvm.org/D89332
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
