[PATCH] D19871: Add an AST matcher for CastExpr kind

[Samuel Benzaquen via cfe-commits]

sbenza added inline comments.

================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:102
@@ +101,3 @@
+  static clang::CastKind getCastKind(llvm::StringRef AttrKind) {
+    return llvm::StringSwitch<clang::CastKind>(AttrKind)
+      .Case("CK_Dependent", CK_Dependent)
----------------
etienneb wrote:
> aaron.ballman wrote:
> > This might be an awful idea, but let's explore it.
> > 
> > What if we moved the CastKind enumerator names into a .def (or .inc) file and use macros to generate the enumeration as well as this monster switch statement? We do this in other places where it makes sense to do so, such as in Expr.h:
> > ```
> >   enum AtomicOp {
> > #define BUILTIN(ID, TYPE, ATTRS)
> > #define ATOMIC_BUILTIN(ID, TYPE, ATTRS) AO ## ID,
> > #include "clang/Basic/Builtins.def"
> >     // Avoid trailing comma
> >     BI_First = 0
> >   };
> > ```
> Does the dynamic matching is used somewhere else than clang-query?
> I wonder the impact of refactoring to support them if it's barely used.
> It can't be worse than before as it wasn't supported at all (the matcher didn't exists).
> 
> I believe there is a larger cleanup to do to support correctly dynamic matcher like "equals".
> And, this case is one among others.
> 
> I'm not a fan of this huge switch that may just get out-of-sync with the original enum.
> 
> I'm still in favor of adding this matcher to the unsupported list until we push the more complicated fix.
> [which may fall in my plate anyway]
> 
> Any toughs?
I'm not a fan of this either.
If the enum has to be used in this way, it should be refactored to be generated with an .inc/.def file.


http://reviews.llvm.org/D19871