urnathan added inline comments. Herald added a project: All.
================ Comment at: clang/test/Modules/inconsist-export-template.cpp:19-23 +// FIXME: We should reject following specialization, +// since it tries to export a name which is already introduced. +export template <> +void f1<int>() { + ---------------- ChuanqiXu wrote: > iains wrote: > > iains wrote: > > > ChuanqiXu wrote: > > > > urnathan wrote: > > > > > I don't think we should be testing for ill-formed code here. We want > > > > > to verify that explicit instantiations, explicit specializations and > > > > > implicit instantiations all get the expected linkage -- both external > > > > > linkage on exported entities, module linkage on non-exported > > > > > module-purview entities. > > > > I think it could add an `expected-error` once we complete the check in > > > > compiler so I added the FIXME here. > > > would it be possible to find a suitable place in the source for the FIXME? > > > I would be concerned that it could get forgotten in the test-case. > > or maybe just file a PR for accepts invalid code? > I would like to send an issue after the patch to remind me not forgetting it. > The issue is needed after the patch since it would crash too. I prefer to > remain the FIXME here so that it would look like a pre-commit test case, > which should be good. The way to not forget about this is to file a bug and assign it to yourself. Not add a known-wrong testcase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120397/new/ https://reviews.llvm.org/D120397 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits