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

Fix/no cx11 warning #18

Merged
merged 5 commits into from
Oct 21, 2014
Merged

Fix/no cx11 warning #18

merged 5 commits into from
Oct 21, 2014

Conversation

jefferis
Copy link
Contributor

@jefferis jefferis commented Sep 3, 2014

This branch removes all warnings with -std=c++03 -Wall -pedantic at least for Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn).

@jefferis
fix: terminal comma in enum requires C++11
90eb133 
* According to clang finishing an enum with a comma is a C++11 extension
* it doesn't seem worth a compiler warning for one comma!
@jefferis
unsigned long long -> uint64_t
eb08a7d 
* apparently unsigned long long is a C++11 extension and is only guaranteed to be >=64 bits
* so use uint64_t which exists since C99
@jefferis
STY: newlines at EOF
db2a97d 
* clang -Wall -pedantic actually complains about these
* I no longer see any compiler warnings
@jefferis
Copy link
Contributor Author

jefferis commented Sep 3, 2014

@simonlynen: apologies I did a push -f 2 mins after the first push when I noticed a typo in a commit message on this branch. I think that upset your CI server so you may need to retrigger the build. But I'm pretty sure it should check ok.

@jefferis
Copy link
Contributor Author

jefferis commented Sep 3, 2014 

jefferis:build (fix/no-cx11-warning)$ make test
Running tests...
Test project /Users/jefferis/dev/c/libnabo/build
      Start  1: validation-2D-exhaustive
 1/18 Test  #1: validation-2D-exhaustive ..............   Passed    0.01 sec
      Start  2: validation-2D-random
 2/18 Test  #2: validation-2D-random ..................   Passed    0.11 sec
      Start  3: validation-3D-exhaustive
 3/18 Test  #3: validation-3D-exhaustive ..............   Passed    0.13 sec
      Start  4: validation-3D-random
 4/18 Test  #4: validation-3D-random ..................   Passed    0.23 sec
      Start  5: validation-3D-large-random
 5/18 Test  #5: validation-3D-large-random ............   Passed    0.46 sec
      Start  6: validation-3D-large-random-radius
 6/18 Test  #6: validation-3D-large-random-radius .....   Passed    0.33 sec
      Start  7: bench-3D-large-exhaustive-10000-K1
 7/18 Test  #7: bench-3D-large-exhaustive-10000-K1 ....   Passed    6.86 sec
      Start  8: bench-3D-large-exhaustive-1000-K1
 8/18 Test  #8: bench-3D-large-exhaustive-1000-K1 .....   Passed   12.63 sec
      Start  9: bench-3D-large-exhaustive-100-K1
 9/18 Test  #9: bench-3D-large-exhaustive-100-K1 ......   Passed   24.81 sec
      Start 10: bench-3D-large-random-K1
10/18 Test #10: bench-3D-large-random-K1 ..............   Passed    3.38 sec
      Start 11: bench-3D-large-exhaustive-10000-K10
11/18 Test #11: bench-3D-large-exhaustive-10000-K10 ...   Passed   16.18 sec
      Start 12: bench-3D-large-exhaustive-1000-K10
12/18 Test #12: bench-3D-large-exhaustive-1000-K10 ....   Passed   23.89 sec
      Start 13: bench-3D-large-exhaustive-100-K10
13/18 Test #13: bench-3D-large-exhaustive-100-K10 .....   Passed   39.90 sec
      Start 14: bench-3D-large-random-K20
14/18 Test #14: bench-3D-large-random-K20 .............   Passed    6.81 sec
      Start 15: bench-3D-large-exhaustive-10000-K30
15/18 Test #15: bench-3D-large-exhaustive-10000-K30 ...   Passed   34.80 sec
      Start 16: bench-3D-large-exhaustive-1000-K30
16/18 Test #16: bench-3D-large-exhaustive-1000-K30 ....   Passed   45.18 sec
      Start 17: bench-3D-large-exhaustive-100-K30
17/18 Test #17: bench-3D-large-exhaustive-100-K30 .....   Passed   70.11 sec
      Start 18: bench-3D-large-random-K30
18/18 Test #18: bench-3D-large-random-K30 .............   Passed   13.42 sec

100% tests passed, 0 tests failed out of 18

Total Test time (real) = 299.26 sec

simonlynen
simonlynen reviewed Sep 3, 2014
View reviewed changes
tests/helpers.h
@@ -140,7 +140,7 @@ namespace boost
*/
struct timer
{
typedef unsigned long long Time;
typedef uint64_t Time;
Copy link
Collaborator

Choose a reason for hiding this comment

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

please add an include for <cstdint>

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or <stdint.h> for c++03

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha. Sorry. I wondered about that but I didn't get a complaint. Strange. And I know that you need <stdint.h> vs <cstdint> depending on standard.

@simonlynen
Copy link
Collaborator

@jefferis Adding the include should fix the build. Otherwise this looks good to me.

@jefferis
STY: 3 more newlines at EOF
af05477 
* missed because I forgot to make clean (ahem)
@jefferis
ensure that uint64_t is available
927fcf2 
* I missed this because on my setup Eigen/Core (which is included by nabo.h)
  ensures that this is available.
@simonlynen
Copy link
Collaborator

lgtm

simonlynen added a commit that referenced this pull request Oct 21, 2014
@simonlynen
Merge pull request #18 from jefferis/fix/no-cx11-warning
a6f2419 
Fix/no cx11 warning
@simonlynen simonlynen merged commit a6f2419 into norlab-ulaval:master Oct 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jefferis @simonlynen @stephanemagnenat