dblaikie added a comment.
You mention a missed case in the description - where the target of a using decl
is used and that causes the unused-using not to warn about the using decl? That
seems like a big limitation. Does that mean the 'used' bit is being tracked in
the wrong place, on the target of the using decl instead of on the using decl
itself?
================
Comment at: lib/Sema/MultiplexExternalSemaSource.cpp:278-279
+ llvm::SmallSetVector<const UsingDecl*, 4> &Decls) {
+ for(size_t i = 0; i < Sources.size(); ++i)
+ Sources[i]->ReadUnusedUsingCandidates(Decls);
+}
----------------
Could you use a range-based-for here?
================
Comment at: lib/Serialization/ASTReader.cpp:8101-8103
+ UsingDecl *D = dyn_cast_or_null<UsingDecl>(
+ GetDecl(UnusedUsingCandidates[I]));
+ if (D)
----------------
roll the declaration into the condition, perhaps:
if (auto *D = dyn_cast_or_null...)
Decls.insert(D);
================
Comment at: test/Modules/warn-unused-using.cpp:5
+
+// For modules, the warning should only fire the first time, when the module is
+// built.
----------------
This would only warn for the unused local using, but wouldn't fire for an
namespace-scoped unused using? Perhaps there should be a test for that?
================
Comment at: test/SemaCXX/warn-unused-using.cpp:6-8
+ typedef int INT;
+ typedef char CHAR;
+ typedef float FLOAT;
----------------
Do these need to be different types? Maybe just name them t1/t2/t3/t4, etc - or
name them after their use in test cases to indicate what makes each type
interesting/different, if that's suitable.
================
Comment at: test/SemaCXX/warn-unused-using.cpp:11-12
+
+using nsp::Print; // expected-warning {{unused using 'Print'}}
+using nsp::var; // expected-warning {{unused using 'var'}}
+
----------------
What's the difference between these two cases?
================
Comment at: test/SemaCXX/warn-unused-using.cpp:15-20
+ using nsp::Print; // expected-warning {{unused using 'Print'}}
+ {
+ using nsp::var; // expected-warning {{unused using 'var'}}
+ {
+ using nsp::INT; // expected-warning {{unused using 'INT'}}
+ using nsp::FLOAT; // expected-warning {{unused using 'FLOAT'}}
----------------
And these 4? (I guess one in a nested scope is significant in some way? But not
sure about the second scope, and the two (rather than one) using decls in there)
================
Comment at: test/SemaCXX/warn-unused-using.cpp:28
+ FLOAT f;
+ f = 2.0;
+}
----------------
Probably don't need this use? (can turn off all the other warnings, rather than
crafting warning-free code when testing only one warning)
================
Comment at: test/SemaCXX/warn-unused-using.cpp:35
+struct B : A {
+ using A::Foo; // expected-warning {{unused using 'Foo'}}
+ virtual void Foo(double x) const;
----------------
This tests the fact that 'Foo' is overridden in B anyway, so this using decl
doesn't bring anything in?
I'm assuming in the case where the using decl does bring at least one name in,
this warning doesn't fire? (maybe test that?)
================
Comment at: test/SemaCXX/warn-unused-using.cpp:39-42
+void one() {
+ B b;
+ b.Foo(1.0);
+}
----------------
Does B need to be used at all, here? Oh, to demonstrate that 'Foo' is using one
overload, but not using the using decl?
Repository:
rC Clang
https://reviews.llvm.org/D44826
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
