[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

Adrian Prantl via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Jan 28 16:38:39 PST 2019

aprantl added inline comments.

Comment at: packages/Python/lldbsuite/test/expression_command/radar_47565290/Makefile:1
+LEVEL = ../../make
Could you give the directory a more descriptive name instead of `radar_47565290`? It's fine to mention a rdar:// link in the commit message, but for most people radar numbers are completely opaque.

Comment at: packages/Python/lldbsuite/test/expression_command/radar_47565290/TestClassTemplateSpecializationParametersHandling.py:2
+Test Expression Parser code gen for ClassTemplateSpecializationDecl. This is a fix for rdar://problem/47564499
Same here, mentioning the radar in the commit message should be sufficient. This forces us to write better/more complete comments, too :-)

Comment at: source/Symbol/ClangASTContext.cpp:1562
+        template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
+            *ast, decl_context, SourceLocation(), SourceLocation(), depth,
+            num_template_params, identifier_info,
does this get more or less readable if we replace `SourceLocation()` with `{}`?

Comment at: source/Symbol/ClangASTContext.cpp:1572
+      }
+    } else if (identifier_info) {
Looks like this is the same code as above. Could this be organized differently to avoid duplicating the code? (I didn't look whether these are all different constructors, so maybe this is already as good as it gets)



More information about the lldb-commits mailing list