================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:605
@@ +604,3 @@
+  }
+  bool isPolymorphic() const LLVM_OVERRIDE { return true; }
+
----------------
Samuel Benzaquen wrote:
> Peter Collingbourne wrote:
> > Samuel Benzaquen wrote:
> > > You are not using LLVM_OVERRIDE consistently.
> > > You are missing it in many of the overrides.
> > I'm using it consistently wherever I intend to override a non-pure virtual 
> > function.
> I see. Shouldn't it be used for overriding any virtual method, even if it is 
> pure?
Well, it's most useful when overriding non-pure virtual member functions 
because it helps detect otherwise-conforming mistakes.

================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:663
@@ +662,3 @@
+/// Not strictly necessary, but DynCastAllOfMatcherDesc gives us better
+/// completion results for that type of matcher.
+template <typename BaseT, typename DerivedT>
----------------
Samuel Benzaquen wrote:
> Peter Collingbourne wrote:
> > Samuel Benzaquen wrote:
> > > Is the overload above used if you add this one?
> > No. We check on lines 409 and 410 of RegistryTest.cpp that the correct 
> > overload is chosen for dyncast matchers.
> What I mean is that if the overload above is not really used, we should get 
> rid of it, and merge VariadicFuncMatcherDesc with DynCastAllOfMatcherDesc.
It's currently used by TypeTraversePolymorphicMatchers and 
VariadicAllOfMatchers.


http://llvm-reviews.chandlerc.com/D2210
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to