[PATCH] Introduce Registry::getCompletions.

Samuel Benzaquen sbenza at google.com
Tue Nov 19 06:41:01 PST 2013



================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:605
@@ +604,3 @@
+  }
+  bool isPolymorphic() const LLVM_OVERRIDE { return true; }
+
----------------
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?

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


http://llvm-reviews.chandlerc.com/D2210



More information about the cfe-commits mailing list