a_sidorin added a comment.
Hello Balázs,
Please clang-format the tests and delete injected-class-name-decl-1. Don't see
any other issues.
================
Comment at: lib/AST/ASTImporter.cpp:2132
+ if (!DCXX->isInjectedClassName()) {
+ // In a record describing a template the type should be a
+ // InjectedClassNameType (see Sema::CheckClassTemplate). Update the
----------------
Nit: "an InjectedClassNameType".
================
Comment at: test/ASTMerge/injected-class-name-decl-1/Inputs/inject1.cpp:16
+} // namespace google
+namespace a {
+template <typename> class g;
----------------
martong wrote:
> 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.
I think we should delete this test. As I see, it passes even in upstream clang,
so it doesn't really make sense to keep it.
Repository:
rC Clang
https://reviews.llvm.org/D47450
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
