[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

David Blaikie via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 8 14:33:16 PST 2020


dblaikie added inline comments.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1467-1468
   if (Method->isStatic())
     return cast_or_null<llvm::DISubroutineType>(
         getOrCreateType(QualType(Func, 0), Unit));
+  return getOrCreateInstanceMethodType(Method->getThisType(), Func, Unit, decl);
----------------
Looks like this patch probably doesn't address the case where the function is static. Though adding support for static should be done in a separate/next patch. It might turn out that handling that case motivates sinking this functionality further down into more shared functionality, but we can deal with that in the review for that separate patch.

(also, there's work being done to add various non-member function declarations to LLVM debug info emission (for call site debug info) - at some point (not in this patch) it's probably worth checking whether those non-member function declarations might need this sort of handling as well)


================
Comment at: clang/test/CodeGenCXX/debug-info-auto-return.cpp:9
+
+// CHECK: ![[t:[0-9]+]] = !DISubroutineType(types: ![[t1:[0-9]+]])
+// CHECK-NEXT: ![[t1:[0-9]+]] = !{![[t2:[0-9]+]], {{.*}}
----------------
This looks incorrect - I expect what you want here is [[t]] to match the previously defined t on the prior CHECK. (similarly with the other matches - use [[x:pattern]] to define the pattern, then [[x]] to reference the previously defined pattern - otherwise the two won't be associated (if you don't need any association between two pattern matches, you can use {{pattern}} for unnamed pattern matching))

Also, please use descriptive names for these matches & I /think/ the convention is usually upper case? I'd think "DECL_TYPE", "DEF_TYPE", "AUTO_TYPE", and "DOUBLE_TYPE" might be good names.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1168-1175
+    DITypeRefArray DeclArgs, DefinitionArgs;
+    DeclArgs = SPDecl->getType()->getTypeArray();
+    DefinitionArgs = SP->getType()->getTypeArray();
+
+    if (DeclArgs.size() && DefinitionArgs.size())
+      if (DeclArgs[0] != DefinitionArgs[0])
+        addType(SPDie, DefinitionArgs[0]);
----------------
Please split this out into a separate patch/review with its own LLVM-level test (& this would be committed before the clang-side)


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

https://reviews.llvm.org/D70524





More information about the cfe-commits mailing list