Anastasia added a comment. In D69938#1741713 <https://reviews.llvm.org/D69938#1741713>, @rjmccall wrote:
> In D69938#1741080 <https://reviews.llvm.org/D69938#1741080>, @Anastasia wrote: > > > In D69938#1737196 <https://reviews.llvm.org/D69938#1737196>, @rjmccall > > wrote: > > > > > It does make logical sense to be able to qualify the call operator of a > > > lambda. The lambda always starts as a temporary, so presumably we want > > > the default address space qualifier on lambdas to be compatible with the > > > address space of temporaries. However, that's probably also true of the > > > default address qualifier on methods in general or you wouldn't be able > > > to call them on temporaries and locals. Thus, we should be able to use > > > the same default in both cases, which seems to be what you've implemented. > > > > > > It even makes sense to be able to qualify a lambda with an address space > > > that's not compatible with the address space of temporaries; that just > > > means that the lambda has to be copied elsewhere before it can be invoked. > > > > > > Just to clarify... Do you mean we should be able to compile this example: > > > > [&] __global { > > i++; > > } (); > > > > > > > > Or should this result in a diagnostic about an addr space mismatch? > > > It should result in a diagnostic on the call. But if you assigned that > lambda into global memory (somehow) it should work. Ok, makes sense so something like this should compile fine: __global auto glob = [&] __global { ... }; > > >> 3. Diagnose address space mismatch between variable decl and lambda expr >> qualifier Example: __private auto llambda = [&]() __local {i++;}; // error >> no legal conversion b/w __private and __local >> >> I think 1 is covered by this patch. I would like to implement 2 as a >> separate patch though to be able to commit fix for 1 earlier and unblock >> some developers waiting for the fix. I think 3 already works and I will just >> update the diagnostic in a separate patch too. > > I agree that 3 should just fall out. And yeah, implementing 2 as a separate > patch should be fine. Please make sure 3 is adequately tested in this patch. Ok, since we can't qualify lambda expr with addr spaces yet there is not much I can test at this point. `__constant` lambda variable seems to be the only case with a diagnostic for OpenCL. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69938/new/ https://reviews.llvm.org/D69938 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
