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

Canonicalize NiceTypeName and add XML-friendly version #461

Merged
merged 9 commits into from
Jan 22, 2016

Conversation

sherm1
Copy link
Member

@sherm1 sherm1 commented Jan 21, 2016

For serializing arbitrary user-defined templated types as is necessary to resolve issue #305 (State serialization), we need a unique, persistent, platform-independent, human-readable, XML-compatible string to represent the type. That string can then be written to the XML file, and then used as a key during deserialization to find the correct registered deserializer for that type.

Simbody's NiceTypeName<T> class is the right idea, but was not producing canonicalized names. This PR modifies its namestr() method to produce cleaned-up names, and adds an xmlstr() method that processes the clean name to replace XML-unfriendly angle brackets with curly braces; that is then suited as the registration key.

There are some SimTK namespace free functions introduced for doing the string processing, which requires the C++11 <regex> facility. That in turn requires at least gcc 4.9, so this PR modifies the Travis installation to use that. This will affect the minimum supported Ubuntu level for Simbody except for users who are willing to upgrade compilers.

Known limitations: types that are in an anonymous namespace or are function-local still produce ugly names that are not platform-independent. These must not be used as types for serialization.

@klshrinidhi, @chrisdembia please let me know if you can find flaws with this -- the State serializer will be counting on these type names to be reliable registration keys.

@sherm1
Copy link
Member Author

sherm1 commented Jan 21, 2016

@chrisdembia, my attempt to upgrade Travis to gcc 4.9 worked for gcc but it broke clang. Looks like clang is trying to use the gcc 4.9 headers but is missing something. Maybe that needs an upgrade too? Can you tell what's wrong?

@chrisdembia
Copy link
Member

...replace XML-unfriendly angle brackets with curly braces; that is then suited as the registration key.

Are curly braces valid in XML element names? If so we should probably do the same thing in OpenSim. @aseth, curly braces might look better for Connector types.

@chrisdembia
Copy link
Member

The NiceTypeName test failed on OSX with clang (no gcc to worry about):

Test failed due to exception: SimTK Exception thrown at TestNiceTypeName.cpp:264:
  Internal bug detected: Test condition failed.
  (Assertion '((NiceTypeName<std::vector<int>>::namestr() == "std::vector<int,std::allocator<int>>"))' failed).

@chrisdembia
Copy link
Member

I have a few ideas for trying to fix the clang/gcc issue. I'll look into it. However, @klshrinidhi probably just solved this same problem.

@sherm1
Copy link
Member Author

sherm1 commented Jan 21, 2016

Are curly braces valid in XML element names?

No. Element names allow only a few special characters. But I am only using these type names as the value of a "type" attribute, which is just a quoted string. Angle brackets cause trouble even inside a quoted string because XML requires they be replaced with &lt; and so on which I deemed too ugly for use.

@sherm1
Copy link
Member Author

sherm1 commented Jan 21, 2016

The NiceTypeName test failed on OSX with clang

Yes, I saw that and investigated. Apparently Apple's stl library throws in extra secret namespaces with names like __1::. I'll modify the collection of regular expressions that I use to process the names to remove namespaces that begin with __. Cleaning up these names is always going to be an imperfect heuristic since there is no language standard for it. But the worst case would be that a name is different on some platform in which case saved State trajectories won't be portable between that platform and others without some manual file cleanup.

@sherm1
Copy link
Member Author

sherm1 commented Jan 21, 2016

I have a few ideas for trying to fix the clang/gcc issue. I'll look into it.

Thanks. It also occurred to me that asking Travis to use "trusty" (14.04) might work since that would presumably come with an updated clang. (Would still need to install gcc 4.9 since 4.8 didn't have regex.) I pushed an attempt at that to this PR to see what happens.


/** Obtain human-readable and XML-usable names for arbitrarily-complicated
C++ types. Three methods `name()`, `namestr()`, and `xmlstr()` are provided
giving respectively the raw, compiler-dependent output from `typeid(T).%name()`,
Copy link
Member

Choose a reason for hiding this comment

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

The comma between raw and compiler is not necessary and makes it seem like there are 4 items in the list.

@chrisdembia
Copy link
Member

I just looked through all the changes. This stuff is a little bit beyond me; so I can't really comment.

Here's a page from boost docs about type names, though they do NOT attempt to obtain platform-independent names: http://www.boost.org/doc/libs/1_58_0/doc/html/boost_typeindex/examples.html

- Better handling of anonymous namespace to match boost.
- Now leaves spaces if they separate any two words, not just ones we recognize.
- Fixed a comment.
@sherm1
Copy link
Member Author

sherm1 commented Jan 22, 2016

Thanks, Chris -- very helpful. I fixed the comment and made two other changes:

  • The boost link your provided showed that they produce {anonymous}:: for anonymous namespace. I liked that so now I'm mapping the various anonymous namespace descriptions into that form.
  • I improved and simplified blank compression. Now I have a single pattern that keeps any space that separates two words, meaning it is surrounded on both sides with an alphanumeric or underscore character. This improved the output for function-local classes though those can't be made usable as reliable keys.

@klshrinidhi, any comments on this?

@sherm1
Copy link
Member Author

sherm1 commented Jan 22, 2016

This is building cleanly now using Ubuntu 14.04 (trusty) on Travis, using the default CMake, but with gcc upgraded to 4.9.

sherm1 added a commit that referenced this pull request Jan 22, 2016
Canonicalize NiceTypeName and add XML-friendly version
@sherm1 sherm1 merged commit 4d1b1ce into master Jan 22, 2016
@sherm1 sherm1 deleted the improve-NiceTypeName-for-XML branch January 22, 2016 17:34
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.

None yet

2 participants