fix(lifetime): extending the lifetime of temporary objects #160
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix for issue #125. The current implementation does not take into account the prvalue that creates an issue while storing it. The prvalue is destroyed after the end of its scope, and if you try access it, you are entering into undefined land of C++. To solve this issue, we employ the trait `std::is_lvalue_reference` to determine if need to extend the lifetime or not. If extension is needed, we store the value with the help of universal forwarding. Otherwise, we extend lifetime using `const &`.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Decoupled each `constexpr if` conditions into function using requires clause, which cleaned-up the compare function. Also, added the check for static extents that removes the runtime check.
|
This Pull request Passed all of clang-tidy tests. 👍 |
Since the patch is fixing the lifetime of the tensor, we cannot combine a prvalue with the old ublas expression. Therefore, we have to bind the prvalue with the name to increase the lifetime.
amitsingh19975
force-pushed
the
bugfix/lifetime
branch
from
a128dfd
to
c69676c
Compare
This function casts any `tensor_expression` to its child class, and it also handles recursive casting to get the real expression that is stored inside the layers of `tensor_expression`.
amitsingh19975
force-pushed
the
bugfix/lifetime
branch
from
c69676c
to
8ec747c
Compare
amitsingh19975
commented
Feb 13, 2022
Comment on lines
208
to
210
| constexpr bool operator==( typename T::value_type lhs, boost::numeric::ublas::detail::tensor_expression<T,D> const& rhs) noexcept{ | ||
| return boost::numeric::ublas::detail::compare( rhs, [lhs](auto const& r){ return lhs == r; } ); | ||
| } |
There was a problem hiding this comment.
Should we use the std::move, or are we only expecting the tensor to have standard C++ arithmetic types and don't need to care about other types, even user-defined types?
Suggested change
| constexpr bool operator==( typename T::value_type lhs, boost::numeric::ublas::detail::tensor_expression<T,D> const& rhs) noexcept{ | |
| return boost::numeric::ublas::detail::compare( rhs, [lhs](auto const& r){ return lhs == r; } ); | |
| } | |
| constexpr bool operator==( typename T::value_type lhs, boost::numeric::ublas::detail::tensor_expression<T,D> const& rhs) noexcept{ | |
| return boost::numeric::ublas::detail::compare( rhs, [lhs = std::move(lhs)](auto const& r){ return lhs == r; } ); | |
| } |
Previously, we would create a new tensor from the expression template every time there is a comparision. Also, fixed the spelling to `cast_tensor_expression` from `cast_tensor_exression`.
amitsingh19975
force-pushed
the
bugfix/lifetime
branch
from
84cf964
to
cf25af7
Compare
… undefined behaviour
amitsingh19975
force-pushed
the
bugfix/lifetime
branch
from
cf25af7
to
2abe215
Compare
…between different extents types
amitsingh19975
force-pushed
the
bugfix/lifetime
branch
from
0ae5de2
to
51674d5
Compare
…bjects to catch type errors
amitsingh19975
force-pushed
the
bugfix/lifetime
branch
from
fce460b
to
b7de038
Compare
…ove redundant casting
The standard does not define the complex class other than float, double, and long double. It leaves the implementation on the hands of vendors, and it could lead to the portability issue or undefined behaviour, which is not warranted.
amitsingh19975
force-pushed
the
bugfix/lifetime
branch
from
5162569
to
23ba8fc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current implementation does not take into account the prvalue that
creates an issue while storing it. The prvalue is destroyed after the
end of its scope, and if you try to access it, you are entering into
the undefined land of C++.
To solve this issue, we employ the trait
std::is_lvalue_referencetodetermine if need to extend the lifetime or not. If an extension is needed,
we store the value with the help of universal forwarding. Otherwise, we
extend the lifetime using
const &.Resolves #125, resolves #118