Fix various compiler warnings #46
Conversation
There was a problem hiding this comment.
gcc complains size_t is undefined. Need to use std::size_t. Why this problem in just this file?
There was a problem hiding this comment.
Because some headers put size_t into the global namespace.
There was a problem hiding this comment.
Stefan:
Legal wants contributors to sign Contributor License Agreement which is now in
#48
It is new for Ondsel so your feedback will be most helpful.
Thanks,
Aik-Siong
Fix all the instances where the index variable is only used for accessing std::vector<T>. Only change the occurences where the index does not depend on any member variables or method parameters.
StefanBruens
force-pushed
the
fix_int_size
branch
from
5212756 to
b1e42f2
Compare
|
Hello, in addition to PR #70 Fix various compiler warnings , this PR would be interesting to merge in order to avoid compilation warnings (for instance in FreeCAD) I tried to review @StefanBruens's changes, looks all good (but not tested) Do not hesitate to comment if you still think there is anything wrong or any blocker that I am not aware of (expect the Regards, |
|
Outdated while waiting for Contributor Agreement. |
|
Hello, what is this Contributor Agreement issue ? How can I solve it ? @aiksiongkoh : can I resubmit the (pretty much) same PR ? Or do you also need a Contributor Agreement from me or the FreeCAD team ? |
|
Yes, we're still happy to consider contributions. A lot has changed since this was originally submitted. The contributor license agreement is in the root of the source. You don't have to do anything specific but since the CLA was added after the PR was submitted and the original contributor has disappeared, I think we should open a new one. If you want to see this merged, Can you please rebase this on the current master and submit a new PR with the relevant changes? |
This MR fixes the majority of the signed-vs-unsigned comparison warnings, see #29.
For the remaining warnings, some of the function parameter types and member variables would need changing, so these changes are left out for now.