alexfh added a comment. > The strategy has changed. Now this check does not ignore template in > instantiation, instead, it generate a
> single warning for some instantiation and ignore others, so that fix is able > to apply. I'm not sure this is the right thing to do, since there might be a case when the presence of the pattern we consider a bug depends on the instantiation: struct A { virtual void func() = 0; }; struct B { virtual void funk(); }; template<typename T> struct D : public T { virtual void func() {...} }; void f() { A *da = new D<A>; da->func(); B *db = new D<B>; // If the check applies a fix issued when inspecting this instantiation, it will break the code on the previous line. } We still might want to warn in this case though. ================ Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:253 @@ +252,3 @@ + SourceLocation Loc = DerivedMD->getLocStart(); + if (WarningSet.find(Loc.getRawEncoding()) != WarningSet.end()) + continue; ---------------- This should be done using a single lookup: if (!WarningSet.insert(Loc).second) continue; ================ Comment at: clang-tidy/misc/VirtualNearMissCheck.h:49 @@ -48,3 +48,3 @@ - /// key: the unique ID of a method; + /// key: the unique ID of a method /// value: whether the method is possible to be overridden. ---------------- The "unique ID of a method" would better be described as a "pointer to the method definition" or "pointer to the canonical declaration of the method" depending on what is actually used as a key. Also, please use proper capitalization and punctuation ("Key", and the trailing period). ================ Comment at: clang-tidy/misc/VirtualNearMissCheck.h:59 @@ -58,1 +58,3 @@ + /// key: the source location id of a generated warning + std::unordered_set<unsigned> WarningSet; ---------------- Please use proper capitalization and punctuation. Also, I'd just say that we keep source locations of issued warnings to avoid duplicate warnings caused by multiple instantiations. ================ Comment at: clang-tidy/misc/VirtualNearMissCheck.h:60 @@ +59,3 @@ + /// key: the source location id of a generated warning + std::unordered_set<unsigned> WarningSet; + ---------------- You don't need to `getRawEncoding`. This can be a `std::set<SourceLocation>` or even better a `llvm::SmallPtrSet<SourceLocation, 8>`, for example. http://reviews.llvm.org/D16922 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits