[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

Raphael Isemann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 25 02:06:32 PST 2020


teemperor added a comment.

My only problem is the reimplementation of the default visit methods with (from what I can tell) the same behavior as the default. If you delete all of these then that would also fix Shafik's point that all the custom dispatch code is technically untested in the unit test (I think not testing the dispatch code from the TypeLocVisitor is fine).



================
Comment at: clang/lib/AST/ASTImporter.cpp:8133
+  Error VisitTypedefTypeLoc(TypedefTypeLoc From) {
+    return VisitTypeSpecTypeLoc(From);
+  }
----------------
I know the ASTImporter is doing these reimplementation of the default visitor behavior quite often, but I feel in this case it would be much more readable to omit all these default implementations and just let the TypeLocVisitor do the dispatch to parent class logic. Especially since from what I understand it's unlikely that we need custom logic for all these TypeLoc subclasses in the future? It would also make this code less fragile in case the inheritance hierarchy every gets another intermediate class that might require special handling.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71018/new/

https://reviews.llvm.org/D71018





More information about the cfe-commits mailing list