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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to