[cfe-commits] [PATCH] Matchers for NestedNameSpecifiers

Daniel Jasper reviews at llvm-reviews.chandlerc.com
Wed Sep 12 10:35:41 PDT 2012


  All done and also added:
  - nestedNameSpecifierLoc as an alternative to loc(nestedNameSpecifier()) so that the NestedNameSpecifierLoc-hierarchy can be traverse
  - hasPrefix matcher, which matches a prefix of either a NestedNameSpecifier or a NNSLoc


================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2315
@@ +2314,3 @@
+/// \brief Matches nested name specifiers that specify a type matching the
+/// given \c QualType matcher with no qualifiers.
+/// FIXME: This is a temporary solution. Switch to using Type-matchers as soon
----------------
Manuel Klimek wrote:
> s/with no/without/
Done.

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:733
@@ -733,1 +732,3 @@
 
+/// \brief The \c LocExtractor has a static method extract that can extract the
+/// underlying object of type \c T out of a \c TLoc object.
----------------
Manuel Klimek wrote:
> insert "to"
No longer an issue.

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:739
@@ +738,3 @@
+template <typename TLoc, typename T> struct LocExtractor;
+template<> struct LocExtractor<NestedNameSpecifierLoc, NestedNameSpecifier> {
+  static const NestedNameSpecifier *extract(const NestedNameSpecifierLoc &Loc) {
----------------
Manuel Klimek wrote:
> Any reasons not to put this as overloaded methods into LocMatcher?
No. Done.

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:744
@@ +743,3 @@
+};
+template<> struct LocExtractor<TypeLoc, Type> {
+  static const Type *extract(const TypeLoc &Loc) {
----------------
Manuel Klimek wrote:
> I'm opposed to adding in code that's not used :)
Removed.

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:825
@@ +824,3 @@
+    return BindableMatcher<T>(new TrueMatcher<T>);
+  Matcher<T> InnerMatcher = makeMatcher(new TrueMatcher<T>);
+  for (int i = InnerMatchers.size() - 1; i > 0; --i) {
----------------
Manuel Klimek wrote:
> I'd write something like:
> Matcher<T> *InnerMatcher = new TrueMatcher<T>;
> for (int i = InnerMatchers.size() - 1; i > 0; --i) {	
>     InnerMatcher = new AllOfMatcher<T, Matcher<T>, Matcher<T> >(	
>       *InnerMatchers[i], makeMatcher(InnerMatcher));	
>   }	
>   return BindableMatcher<T>(InnerMatcher);
Yeah.. Something like ;-)..

================
Comment at: include/clang/ASTMatchers/ASTTypeTraits.h:132
@@ +131,3 @@
+template<> struct DynTypedNode::BaseConverter<NestedNameSpecifierLoc, void> {
+  static const NestedNameSpecifierLoc *get(NodeTypeTag Tag, const char Storage[]) {
+    if (Tag == NT_NestedNameSpecifierLoc)
----------------
Manuel Klimek wrote:
> 80 columns
Done.

================
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:569
@@ +568,3 @@
+  match(NNS);
+  match(*NNS.getNestedNameSpecifier());
+  return
----------------
Manuel Klimek wrote:
> Please add a comment explaining that the traversal is already done through the "parallel hierarchy" in the locs, and we only want to match the nns themselves for each loc.
Done.


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



More information about the cfe-commits mailing list