[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Hello, Balázs,

You can find my comments inline.



================
Comment at: lib/AST/ASTImporter.cpp:2131
         D2CXX->setDescribedClassTemplate(ToDescribed);
+        if (!DCXX->isInjectedClassName()) {
+          // In a record describing a template the type should be a
----------------
Maybe we should fix it a bit upper (line 2100)?


================
Comment at: test/ASTMerge/injected-class-name-decl-1/Inputs/inject1.cpp:16
+} // namespace google
+namespace a {
+template <typename> class g;
----------------
This looks like raw creduce output. Is there a way to simplify this or make human-readable? Do we really need nested namespaces, unused decls and other stuff not removed by creduce? I know that creduce is bad at reducing templates because we often meet similar output internally. But it is usually not a problem to resolve some redundancy manually to assist creduce. In this case, we can start by removing `k` and `n`.
We can leave this code as-is only if removing declarations or simplifying templates affects import order causing the bug to disappear. But even in this case we have to humanize the test.


Repository:
  rC Clang

https://reviews.llvm.org/D47450