tellenbach added a comment.
No real comment on the issue itself but on the example as a former Eigen
maintainer (sorry for the noise if that's all obvious for you):
auto round (Tensor m) {
return (m + 0.5f).cast<int>().cast<float>();
}
does not return a Tensor but an expression encoding the computation which is
evaluated during the assignment `const Tensor t3 = round(a.log().exp());` using
an overloaded `operator=`. With "evaluation" I mean evaluation the in the sense
of performing the intended mathematical computation.
Many things can go wrong when using `auto` in such cases, of which two seem to
be relevant here:
1. Eigen can (this is an implementation detail(!)) decide to evaluate
subexpressions into temporaries. The returned expression would then reference
the result of such an evaluation beyond its lifetime.
2. If 1. does not happen, the unevaluated expression would reference its
arguments. The immediate `0.5f` is copied since that's cheap, but the tensor
would be referenced. Your example function `round` takes its parameter by-value
and the returned expression would reference it. I'm unsure if the lifetime
would be extended in this case (again, the exact details of C++ lifetime rules
are not my area of expertise). I think if the lifetime would be extended, ASAN
complaining after applying this patch is wrong, if the lifetime is not extended
ASAN should complain and the example is wrong.
Btw, these issues are so common that Eigen documents the recommendation to
avoid using `auto` with Eigen types all together:
https://eigen.tuxfamily.org/dox/TopicPitfalls.html#title3
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74094/new/
https://reviews.llvm.org/D74094
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
