Remove spaces from sim3 type names

[iillyyaa]

Discovered while experimenting with g2o cli utility. Spaces introduced by clang-format in a70b6d7 prevent these types from being parsable from a file.

Several similar fixes were already applied to other types (e.g. e28160d and 25e393e), but types/sim were missed on those occasions.

---

My recommendation for a more robust fix would be to change the type registration macro thus:

diff --git a/g2o/core/factory.h b/g2o/core/factory.h
index d7b48f22..cd5e5e2d 100644
--- a/g2o/core/factory.h
+++ b/g2o/core/factory.h
@@ -145,11 +145,11 @@ class RegisterTypeProxy {
 #endif
 
 // These macros are used to automate registering types and forcing linkage
 #define G2O_REGISTER_TYPE(name, classname)                         \
   extern "C" void G2O_FACTORY_EXPORT g2o_type_##classname(void) {} \
-  static g2o::RegisterTypeProxy<classname> g_type_proxy_##classname(#name);
+  static g2o::RegisterTypeProxy<classname> g_type_proxy_##classname(name);
 
 #define G2O_USE_TYPE_BY_CLASS_NAME(classname)                    \
   extern "C" void G2O_FACTORY_IMPORT g2o_type_##classname(void); \
   static g2o::ForceLinker proxy_##classname(g2o_type_##classname);
 

... and provide character literals at the type registration site:

diff --git a/g2o/types/sim3/types_seven_dof_expmap.cpp b/g2o/types/sim3/types_seven_dof_expmap.cpp
index d5e50dcd..1faebcc2 100644
--- a/g2o/types/sim3/types_seven_dof_expmap.cpp
+++ b/g2o/types/sim3/types_seven_dof_expmap.cpp
@@ -32,14 +32,14 @@
 namespace g2o {
 
 G2O_USE_TYPE_GROUP(sba);
 G2O_REGISTER_TYPE_GROUP(sim3);
 
-G2O_REGISTER_TYPE(VERTEX_SIM3:EXPMAP, VertexSim3Expmap);
-G2O_REGISTER_TYPE(EDGE_SIM3:EXPMAP, EdgeSim3);
-G2O_REGISTER_TYPE(EDGE_PROJECT_SIM3_XYZ:EXPMAP, EdgeSim3ProjectXYZ);
-G2O_REGISTER_TYPE(EDGE_PROJECT_INVERSE_SIM3_XYZ:EXPMAP,
+G2O_REGISTER_TYPE("VERTEX_SIM3:EXPMAP", VertexSim3Expmap);
+G2O_REGISTER_TYPE("EDGE_SIM3:EXPMAP", EdgeSim3);
+G2O_REGISTER_TYPE("EDGE_PROJECT_SIM3_XYZ:EXPMAP", EdgeSim3ProjectXYZ);
+G2O_REGISTER_TYPE("EDGE_PROJECT_INVERSE_SIM3_XYZ:EXPMAP",
                   EdgeInverseSim3ProjectXYZ);
 
 VertexSim3Expmap::VertexSim3Expmap() : BaseVertex<7, Sim3>() {
   _marginalized = false;
   _fix_scale = false;

This would prevent clang-format from interfering with those values in the future. I have a commit with this fix already ready, and if you wish, can provide it in addition to or instead of this PR.

This would of course not be backward-compatible with other projects that might use g2o and register their own types. If backward-compatibility were desired, a new G2O_REGISTER_TYPE_NAME macro could be introduced that expects a character literal, and G2O_REGISTER_TYPE macro could remain (deprecated) purely for backward-compatibility.

Additionally or alternatively, run-time checking of type names could be added in RegisterTypeProxy constructor, and flag or replace characters that could impede parsing. In C++20 a template-based solution could probably be used to validate characters at compile time, but I digress.

@RainerKuemmerle, I am offering to implement, at your discretion, either the straight-forward change, or the backward-compatible one. Just let me know in this PR which one you prefer, if any, and I can enter an issue with the chosen proposal and develop the fix.

[RainerKuemmerle]

@iillyyaa thanks for spotting and providing a fix.

I like the idea of G2O_REGISTER_TYPE_NAME macro that expects a string literal. We can probably keep it backwards compatible by sharing implementation of the new macro and the old one. This would prevent clang-format issues in the future with those macros.

I'd appreciate a PR for this backward compatible change.

Furthermore, I think an improvement to prevent problems can be a static_assert. std::string_view in C++17 offers us constexpr c'tor and find functions to implement this.

#define G2O_REGISTER_TYPE(name, classname)                              \
  extern "C" void G2O_FACTORY_EXPORT g2o_type_##classname(void) {}      \
  static_assert(std::string_view(#name).find_first_of(" \t\n\f\r\v") == \
                    std::string_view::npos,                             \
                "name contains whitespace");                            \
  static g2o::RegisterTypeProxy<classname> g_type_proxy_##classname(#name);

The static_assert can also be part of the new macro to prevent later problems with the g2o file format when saving.
What do you think?

If I apply this on master before your fix, we obtain

/home/goki/workspace/g2o/g2o/types/sim3/types_seven_dof_expmap.cpp:37:1: error: static assertion failed due to requirement 'std::basic_string_view<char, std::char_traits<char>>("VERTEX_SIM3 : EXPMAP").find_first_of(" \t\n\f\r\v") == std::basic_string_view<char, std::char_traits<char>>::npos': name contains whitespace
   37 | G2O_REGISTER_TYPE(VERTEX_SIM3 : EXPMAP, VertexSim3Expmap);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/goki/workspace/g2o/g2o/core/factory.h:151:17: note: expanded from macro 'G2O_REGISTER_TYPE'
  151 |   static_assert(std::string_view(#name).find_first_of(" \t\n\f\r\v") == \
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  152 |                     std::string_view::npos,                             \
      |                     ~~~~~~~~~~~~~~~~~~~~~~
/home/goki/workspace/g2o/g2o/types/sim3/types_seven_dof_expmap.cpp:37:1: note: expression evaluates to '11 == 18446744073709551615'
   37 | G2O_REGISTER_TYPE(VERTEX_SIM3 : EXPMAP, VertexSim3Expmap);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/goki/workspace/g2o/g2o/core/factory.h:151:70: note: expanded from macro 'G2O_REGISTER_TYPE'
  151 |   static_assert(std::string_view(#name).find_first_of(" \t\n\f\r\v") == \
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
  152 |                     std::string_view::npos,                             \
      |                     ~~~~~~~~~~~~~~~~~~~~~~

[iillyyaa]

@RainerKuemmerle, great idea on using string_view - I forgot that it was already constexpr in C++17.

I will put together another PR on top of this branch, and (assuming this merges soon) will file it separately.

I have a couple of questions as I make these changes:

1. Would you like me to remove // clang-format on/off annotations around these type registration blocks?
2. If so, then would you like me to clang-format those blocks? A few of the type declarations will get line breaks, since Google style defaults to 80 columns, and _TYPE and "" together add 7 characters.

For now, I proceeded assuming the answer is "yes" to both. Here's a new pull request: #832