================
Comment at: lib/ASTMatchers/Dynamic/VariantValue.cpp:160
@@ -84,1 +159,3 @@
+      if (CanConstructCallback(*AsPolymorphic->Matchers[i]))
+        return AsPolymorphic->Matchers[i];
     }
----------------
Stefanus Du Toit wrote:
> This changes the behaviour of this function - previously if more than one 
> matcher passed the test it would return null, now it returns the first one. I 
> don't know if you intended that, but if you did, you should update the 
> comment at the function's declaration.
There should be only one, because the caller already checked (asserts on 
hasTypedMatcher()).
Replaced the return NULL with llvm_unreachable, since it is; and updated the 
comment in the header.

================
Comment at: lib/ASTMatchers/Dynamic/VariantValue.cpp:156
@@ +155,3 @@
+  case RT_Nothing: return NULL;
+  case RT_Single: return AsSingle->Matcher.get();
+  case RT_Polymorphic:
----------------
Stefanus Du Toit wrote:
> Shouldn't this still call CanConstructCallback()?
The caller is asserting that hasTypedMatcher<T>() is true. This is because the 
user must check hasTypedMatcher first, and then call getTypedMatcher.
I added another assertion here for documentation.


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

Reply via email to