martong added a comment.
In D64078#1572675 <https://reviews.llvm.org/D64078#1572675>, @a_sidorin wrote:
> Hi Gabor,
> it is a nice design question if source locations can participate in
> structural match or not. The comparison tells us that the same code written
> in different files is not structurally equivalent and I cannot agree with it.
> They can be not the same, but their structure is the same. The question is:
> why do we get to this comparison? Do we find a non-equivalent decl by lookup?
> I guess there can be another way to resolve this issue, and I'll be happy to
> help if you share what is the problem behind the patch.
Hey Alexei, thanks for looking into this. I agree, that those lambdas are
actually "structurally" equivalent even if they are defined in different
locations...
But still we have to differentiate them somehow.
Consider this code:
void f() {
auto L0 = [](){};
auto L1 = [](){};
}
First we import `L0` then `L1`. Currently we end up having only one
CXXRecordDecl for the two different lambdas. And that is a problem if the body
of their op() is different.
This happens because when we import `L1` then lookup finds the existing `L0`
and since they are structurally equivalent we just map the imported L0 to be
the counterpart of L1 <https://reviews.llvm.org/L1>.
I tried to use `getLambdaManglingNumber()` as you suggested in
https://reviews.llvm.org/D64075 and that does solve this particular case, but
cannot solve to other issue we have in the test case
`LambdasInFunctionParamsAreDifferentiated`:
template <typename F0, typename F1>
void f(F0 L0 = [](){}, F1 L1 = [](){}) {}
Here after importing L0, L1 <https://reviews.llvm.org/L1> is mapped to L0 again.
I tried an alternative solution, which is to disable lookup completely if the
decl in the "from" context is a lambda. That passed all tests.
However, that could have other problems: what if the lambda is defined in a
header file and included in several TUs? I think we'd have as many duplicates
as many includes we have. I think we could live with that, because the lambda
classes are TU local anyway, we cannot just access them from another TU.
I have created another patch for that solution: https://reviews.llvm.org/D66348
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64078/new/
https://reviews.llvm.org/D64078
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits