Skip to content
This repository has been archived by the owner on Aug 5, 2022. It is now read-only.

Fix ElementHandle::setAs.*Array methods #351

Merged
merged 1 commit into from
Feb 8, 2016

Conversation

dawagner
Copy link
Contributor

@dawagner dawagner commented Feb 8, 2016

Direct accesses to array parameters were incorrectly identified, resulting in
an error when the client tried to set them. This was caused by a programming
mistake in using overloaded methods because T, const T, T & and const T& are different. This is fixed by using the SFINAE principle and correctly
handing all the cases described above.

/** @return 0 by default, ie for non overloaded types. */
size_t getUserInputSize(...) { return 0; }
/** @return collection size if it has a size() method. does not participate to overload otherwise. */
template <class T, class..., class = decltype(&T::size)>
size_t getUserInputSize(T collection) { return collection.size(); }
size_t f(...) { return 0; }
template <class T>
size_t f(const std::vector<T> &t) { return t.size(); }
@@            master    #351   diff @@
======================================
Files          210     210
Stmts         7161    7163     +2
Branches         0       0
Methods          0       0
======================================
+ Hit           5644    5698    +54
Partial          0       0
+ Missed        1517    1465    -52

Review entire Coverage Diff as of 30a7c5f

Powered by Codecov. Updated on successful CI builds.

  

@dawagner dawagner added the bug label Feb 8, 2016
@dawagner
Copy link
Contributor Author

dawagner commented Feb 8, 2016

@krocard @tcahuzax please review.

@codecov-io
Copy link

Current coverage is 79.54%

Merging #351 into master will increase coverage by +0.73% as of 83192a1

@@            master    #351   diff @@
======================================
  Files          210     210       
  Stmts         7161    7163     +2
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit           5644    5698    +54
  Partial          0       0       
+ Missed        1517    1465    -52

Review entire Coverage Diff as of 83192a1

Powered by Codecov. Updated on successful CI builds.

{
return values.size();
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using SFINAL to test for size() method existence

/** @return 0 by default, ie for non overloaded types. */
 size_t getUserInputSize(...) { return 0; }

/** @return collection size if it has a size() method. does not participate to overload otherwise. */
template <class T, class..., class = decltype(&T::size)>
size_t getUserInputSize(T collection) { return collection.size(); }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it is worth it: It works but only if I add an overload for std::string (it has a size method but we don't want to consider it as a collection). I can't guarantee, ahead of time, that no future "scalar" type will have a size() method. We would rely too much on future tests begin correctly written.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, then this should work:

template <class T>
size_t getUserInputSize(const T&) { return 0; }

template <class T>
size_t getUserInputSize(const std::vector<T> &t) { return t.size(); }

Direct accesses to array parameters were incorrectly identified, resulting in
an error when the client tried to set them. This was caused by a programming
mistake in using overloaded methods because `T`, `const T`, `T &` and `const
T&` are different.

Signed-off-by: David Wagner <[email protected]>
@krocard
Copy link
Contributor

krocard commented Feb 8, 2016

👍

dawagner added a commit that referenced this pull request Feb 8, 2016
Fix ElementHandle::setAs.*Array methods
@dawagner dawagner merged commit 3d38fee into intel:master Feb 8, 2016
@dawagner dawagner deleted the fix-elementhandle-setArray branch February 8, 2016 16:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants