[PATCH] Introduce Registry::getCompletions.

Peter Collingbourne peter at pcc.me.uk
Fri Nov 22 17:58:04 PST 2013



================
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



More information about the cfe-commits mailing list