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

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 30 07:45:19 PDT 2018


martong added inline comments.


================
Comment at: test/ASTMerge/injected-class-name-decl-1/Inputs/inject1.cpp:16
+} // namespace google
+namespace a {
+template <typename> class g;
----------------
balazske wrote:
> a.sidorin wrote:
> > 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.
> Probably remove this test? There was some bug in a previous version of the fix that was triggered by this test. Before that fix (and on current master) this test does not fail so it is not possible to simplify it.
I vote on deleting this test then. We already have another clear and simple test.


Repository:
  rC Clang

https://reviews.llvm.org/D47450





More information about the cfe-commits mailing list