Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mask, Vector, and Simd[Mask]Array::operator[] should return a smart reference (proxy) #115

Closed
mattkretz opened this issue Mar 23, 2016 · 0 comments
Closed

Mask, Vector, and Simd[Mask]Array::operator[] should return a smart reference (proxy) #115

mattkretz opened this issue Mar 23, 2016 · 0 comments
Assignees
@mattkretz
Labels
type: enhancement WG21 — P0214
Milestone
Vc 1.3

Comments

@mattkretz
Copy link
Member

Feedback on P0214R0 from SG1 was to leave type punning alone and return a smart ref / rvalue value_type from operator[] (const).
@mattkretz mattkretz self-assigned this Mar 23, 2016
@mattkretz mattkretz added this to the Vc 1.3 milestone Mar 23, 2016
mattkretz added a commit that referenced this issue Mar 23, 2016
@mattkretz
Subscript Test: check copy, move, compound assign
8a4f7af 
* Test SimdArray, too
* Rewrite the `write` test because of the odd sizes of SimdArray
* Added a iterateNumericRange generator to simplify test code, in this
case the `write` test.
* Ensure no copy/move possible from non-const v[i]
* Ensure copy/move possible from const v[i]
* Ensure compound assignment works as expected for the underlying
value_type

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 23, 2016
@mattkretz
Add ElementReference - a generic smart reference
8acc932 
ElementReference<T> acts as much as possible like an lvalue reference of
T::value_type. It has:
* all move and copy ctors and assignment deleted, and
* an rvalue ref-qualifier on all assignment and in-/decrement operators

to limit the use of the proxy type as much as possible. The intent is to
either copy to value_type or to modify the referenced element directly,
i.e. without going through an lvalue.

The noexcept specification is derived from the noexcept-ness of the
Accessor functions to the original container/vector.

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 23, 2016
@mattkretz
Add test for operators not usable on lvalues
474fdbd 
Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 23, 2016
@mattkretz
Add test to ensure subscripting is noexcept
28a4923 
Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 23, 2016
@mattkretz
squash: Use U as Accessor by default
027761b 
Refs: gh-115

Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 23, 2016
@mattkretz
Return smart ref/rvalues from operator[]
f18e125 
Port all implementations & SimdArray to use Detail::ElementReference as
return type for non-const operator[]. const operator[] returns a
value_type copy directly. This change removes the need to return lvalue
references that alias memory of VectorType.

In addition this change introduces noexcept specifications for all
functions related to operator[].

The special case for short vectors on MIC are obsolete now because of
the explicit conversion to value_type on Vector::get.

Also dropped the optimized implementation of __builtin_constant_p
operator[] for GCC. As far as I've seen GCC can do these optimizations
by itself now.

Fixes: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 23, 2016
@mattkretz
Add ElementReference - a generic smart reference
9cc6a9d 
ElementReference<T> acts as much as possible like an lvalue reference of
T::value_type. It has:
* all move and copy ctors and assignment deleted, and
* an rvalue ref-qualifier on all assignment and in-/decrement operators

to limit the use of the proxy type as much as possible. The intent is to
either copy to value_type or to modify the referenced element directly,
i.e. without going through an lvalue.

The noexcept specification is derived from the noexcept-ness of the
Accessor functions to the original container/vector.

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 23, 2016
@mattkretz
Add test for operators not usable on lvalues
494a255 
Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 23, 2016
@mattkretz
Add test to ensure subscripting is noexcept
25ae212 
Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 23, 2016
@mattkretz
Return smart ref/rvalues from operator[]
695dbf4 
Port all implementations & SimdArray to use Detail::ElementReference as
return type for non-const operator[]. const operator[] returns a
value_type copy directly. This change removes the need to return lvalue
references that alias memory of VectorType.

In addition this change introduces noexcept specifications for all
functions related to operator[].

The special case for short vectors on MIC are obsolete now because of
the explicit conversion to value_type on Vector::get.

Also dropped the optimized implementation of __builtin_constant_p
operator[] for GCC. As far as I've seen GCC can do these optimizations
by itself now.

Fixes: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Simplify SFINAE: the noexcept expression suffices
bc05b97 
The noexcept operator SFINAEs if the expression in it is invalid.
Therefore the enable_if only needs to do the function argument count.

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Don't rely on lvalue ref in gather test
a3baffe 
Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Use const operator[] where possible
ffcad5f 
Otherwise the types of the std::tuple would be Detail::ElementReference,
which would be a very bad idea.

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Disambiguate call to set
e863a26 
Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Segment only needs to support const operator[]
bd9e338 
This change is required because non-const operator[] of vector types
returns ElementReference now.

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
New iterators unit test
5b75da6 
This test is important *now* since the Vector::operator[] doesn't return
lvalue references anymore. This changes the iterators category to
InputIterator. At the same time the iterator should still support as
much as possible of the RandomAccessIterator interface.

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Modified range-for test to expect InputIterator
8a2d830 
After Vector & Mask return ElementReference on operator[] iterators can
not return an lvalue reference on operator* and thus only implement
InputIterator and OutputIterator. Consequently, the unit test should
only test that range-for works for InputIterator.

Moved the test to iterators.cpp from utils.cpp since that is the natural
place for it now.

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Test unary operators on ElementReference
feb1be7 
Ensure that unary !, +, -, and ~ work with expressions of
Vector::operator[]. Though, ~ should only work for integral types.

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Test noexcept of Mask::operator[]
d4398de 
Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Test unary ! on Mask::operator[]
c1d0716 
Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Mask: Use ElementReference for operator[]
267b5cb 
* Dropped MaskEntry, which was a specialized ElementReference for Mask
* Dropped setEntry members in Mask types (remnant of MaskEntry)
* Added member type `reference` to all Mask types
* Declared Mask subscripting noexcept; conditional where it possibly
calls user code
* Changed SSE::Mask to return bool via MaskBool conversion instead of
going via toInt()

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Port Iterator to ElementReference
af82174 
* Set iterator_category to input_iterator_tag
* Make Iterator DefaultConstructible and CopyAssignable
* Implement all RandomAccessIterator operations
* Fix const-correctness (i.e. const mutable iterators still point to
mutable data)
* Drop the std::iterator base: It was useless and inflexible

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Rely on implicit copy ctor/assign
1a64135 
This works around the question of constexpr on the function which clang
3.5.0 and GCC 4.8 reject (old C++11 rules implied the const qualifier).

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Simplify template on inc-/decrement operators
0ba7cd8 
Apple clang complains about the variadic template parameter on
operator++/-- (for no good reason, IMHO). Using a defaulted template
parameter should make this more portable, though.

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Mar 30, 2016
@mattkretz
Rewrite SimdMaskArray::shifted to work around clang ICE
744d3f1 
The operator[] change lead to an internal compiler error in clang 3.5.
Reimplementing SimdMaskArray::shifted with SimdMaskArray::generate makes
the code nicer to read/understand and successfully works around the ICE
in clang.

Refs: gh-115
Signed-off-by: Matthias Kretz <kretz@kde.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mattkretz mattkretz

Labels
type: enhancement WG21 — P0214
Projects
None yet
Milestone
Vc 1.3
Development

No branches or pull requests
1 participant
@mattkretz