[PATCH] D14326: ASTImporter: expressions, pt.2

Sean Callanan via cfe-commits cfe-commits at lists.llvm.org
Tue May 10 10:41:38 PDT 2016


spyffe requested changes to this revision.
spyffe added a comment.
This revision now requires changes to proceed.

Overall I'm very enthusiastic about this and only have a few nitpicks.  Thanks for the hard work!


================
Comment at: lib/AST/ASTImporter.cpp:2364
@@ +2363,3 @@
+  if (Arg.getKind() == TemplateArgument::Expression) {
+    Expr *E = Importer.Import(FromInfo.getAsExpr());
+    ToInfo = TemplateArgumentLocInfo(E);
----------------
I would expect this to be checked for `nullptr` and this function to error out.

================
Comment at: lib/AST/ASTImporter.cpp:2367
@@ +2366,3 @@
+  } else if (Arg.getKind() == TemplateArgument::Type) {
+    TypeSourceInfo *TSI = Importer.Import(FromInfo.getAsTypeSourceInfo());
+    ToInfo = TemplateArgumentLocInfo(TSI);
----------------
Should this be checked for `nullptr` as is done for other imports of `TypeSourceInfo`?

================
Comment at: lib/AST/ASTImporter.cpp:5517
@@ -5353,3 +5516,3 @@
                                          FoundD,
-                                         /*FIXME:TemplateArgs=*/nullptr);
+                                         /*TemplateArgs=*/ResInfo);
   if (E->hadMultipleCandidates())
----------------
This makes me happy :)

================
Comment at: lib/AST/ASTImporter.cpp:6317
@@ +6316,3 @@
+  if (!SubExpr && EWC->getSubExpr())
+    return NULL;
+
----------------
You probably mean to use `nullptr`


http://reviews.llvm.org/D14326





More information about the cfe-commits mailing list