svenvh added a comment.
> I have attempted to workaround this issue for the reported test cases,
> however it still doesn't quite work when the type is created from the
> template parameter (see FIXME test case in the patch). I presume we want to
> allow this? If so we might need to disable lazy template instantiation in
> this case. My guess is the only issue this this is that we will have
> performance penalty for the code of this format:
I don't have enough experience with lazy template instantiation to give
meaningful advice here. Though I'm not too worried about performance penalties
for the example you're giving, as I'd expect most realistic use cases will
require the full instantiation of a templated type sooner or later in the
source program anyway (e.g. near the calling point).
> While this matter might need more thoughts and investigations I wonder
> whether it makes sense to commit this fix for the time being since it's
> fixing the reported test case at least?
That sounds reasonable to me; clang currently hard-rejects a valid source due
to an over-conservative diagnostic which isn't a great user experience.
> So another approach we could take is to change this diagnostics into warnings
> and then if we can't fully detect the type provide the messaging that we
> can't detect whether the type is safe...
I think that makes sense, and should be fine to leave as a followup.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:8856
+ if (CXXRec) {
+ if (!CXXRec->hasDefinition())
+ CXXRec = CXXRec->getTemplateInstantiationPattern();
----------------
A comment explaining what we're trying to do here would be nice.
================
Comment at: clang/test/SemaOpenCLCXX/invalid-kernel.clcpp:99
+struct Outer {
+struct Inner{
+int i;
----------------
Indenting would help readability.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134445/new/
https://reviews.llvm.org/D134445
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
