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

[Adrian Prantl via Phabricator via lldb-commits]

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) {
       template_param_decls.push_back(TemplateTypeParmDecl::Create(
----------------
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)


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

https://reviews.llvm.org/D57363