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.

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

Support newer GCC versions and minor cleanup #57

Merged
merged 2 commits into from
Nov 23, 2021

Conversation

victorreijgwart
Copy link
Member

@victorreijgwart victorreijgwart commented Apr 5, 2021

Some minor changes to make sure voxgraph compiles cleanly with newer GCC versions, such as on Ubuntu 20.04.

Compiled and tested on Ubuntu 18.04 and 20.04.

Changes

  • Stricter compile flags.
  • Make sure all class members are initialized in the right order (in each constructor's member initializer list).
  • Add default cases to switch statements to avoid warnings in newer GCC versions.
  • Remove misleading (ignored) const qualifiers for functions that return values by copy.
  • Updates to the .clang-format rules to automatically order #includes.

Copy link
Contributor

@alexmillane alexmillane left a comment

Choose a reason for hiding this comment

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

Cheers victor

@@ -1,7 +1,7 @@
cmake_minimum_required(VERSION 2.8.3)
project(voxgraph)

add_definitions(-std=c++11 -Werror)
add_definitions(-std=c++14 -Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wno-deprecated-copy -fPIC -DEIGEN_INITIALIZE_MATRICES_BY_NAN -fdiagnostics-color=auto)
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of interest what's the motivation for DEIGEN_INITIALIZE_MATRICES_BY_NAN? Is it catch uninitialized matrix access?

submap_creation_interval_(20),
verbose_(verbose) {}
verbose_(verbose),
submap_creation_interval_(20) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would usually make the default a constexpr somewhere.

@@ -3,6 +3,7 @@
#include <string>
#include <vector>

#include <boost/filesystem.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

One day we will get stl filesystem. sigh

const FrameId getReferenceFrameId() const {
return config_.reference_frame_id;
}
FrameId getReferenceFrameId() const { return config_.reference_frame_id; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I see the recommendation on returning const copies has changed to "don't do it"

Thanks for fixing this.

@alexmillane
Copy link
Contributor

alexmillane commented Oct 22, 2021

@victorreijgwart Should we merge this? Currently, we're not building on 20.04, and this solves matters :)

See: ethz-asl/voxblox#370 and ethz-asl/cblox#43

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants