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

XTypes topic and key annotations, Unions as top-level types #1067

Merged
merged 167 commits into from Aug 2, 2019

Conversation

iguessthislldo
Copy link
Member

@iguessthislldo iguessthislldo commented Feb 21, 2019

Building on DOCGroup/ACE_TAO#723, this aligns opendds_idl closer to the XTypes standard by providing topic annotations such as @topic and @key as an alternative to DCPS_DATA pragma statements (addressing #366) and allowing unions to be used as topic types (addressing #486). This also makes IDL4 support the default for OpenDDS.

TODO

  • Update for DOCGroup/ACE_TAO#822 annotation API Changes
  • Fix tests/FACE/Compiler tests
  • Implement @nested and changes agreed on in DDSXTY13-69
    • For each valid type (struct or union) whether or not to generate type support is determined by these rules:
      • If @topic is applied
        • Always generate
        • If @nested is also applied and @nested.value is true
          • Warn about conflicting annotations
      • else if @nested is applied
        • Generate if @nested.value is false.
      • else inherit the parent module's default.
    • For each submodule the type support generation default is:
      • If @default_nested is applied, generate by default if @default_nested.value is false.
      • If @default_nested isn't applied, inherit the parent module's default.
    • For the root module, the ultimate parent of all modules, do not generate type support by default, unless a command line argument is passed to change that.
  • Warn (once per file, command line option can silence) for any use of #pragma DCPS_DATA_TYPE
  • Add tests somewhere in tests/DCPS/Compiler/annotations for @nested and @default_nested.
  • UnionTopic Test
  • Update CMake Module to pass --idl-version 4 --unknown-annotations ignore to tao_idl.
  • Reference Developer's Guide in warning about #pragma DCPS_DATA_TYPE.
  • Update CMake Module to optionally pass --default-nested
  • Resolve CMake Issues
  • Support Multidimensional Array Keys
  • Refractor marshal_generator.cpp
  • Have Java API work with Annotations Will be addressed in another PR

Also Added @sample_type
- @key on unions are a WIP in this commit
- Various support code for these changes
At least temporarily since it's not complete. It needs additional
marshalling functions for at least arrays, maybe other cases.
Also add example of key union nested in struct to
topic_annotations_test.idl

TODO: Add generated code for KeyLessThan in keys_generator and support
standalone unions.
Now at the point where a simple example appilcation can be compiled and
run.
Copy link
Member

@mitza-oci mitza-oci left a comment

Choose a reason for hiding this comment

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

Just a few notes on the new code... look like there are also some codacy/cppcheck issues.

dds/DCPS/Serializer.cpp Show resolved Hide resolved
dds/idl/topic_keys.cpp Outdated Show resolved Hide resolved
dds/idl/topic_keys.cpp Outdated Show resolved Hide resolved
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

How about we have cmake copy everything into cmake_integration/Messenger_X? Also for Perl dependency we can make a script that runs perl script that runs messenger without PerlDDS.

Ah! Good idea.

cmake/tao_idl_sources.cmake Show resolved Hide resolved
cmake/api_macros.cmake Show resolved Hide resolved
cmake/init.cmake Show resolved Hide resolved
@iguessthislldo
Copy link
Member Author

@mitza-oci It looks like the windows builds on Azure are working now, but Travis is being pushed over 50 minutes...

@iguessthislldo
Copy link
Member Author

If we get rid of the duplicate compiler tests for annotations, which is going to happen at some point anyway and combine this with #1211, it might bring Travis back down to under 50 minutes.

@iguessthislldo
Copy link
Member Author

@mitza-oci Do you know how the Java API generator knows that a type needs typesupport? I thought just enabling IDL4 in idl2jni_codegen would make the java tests work with annotations, but they're missing Typesupport, so it's still relying on DCPS_DATA_TYPE somehow. I looked for checks for DCPS_DATA_TYPE, but that only exists in ts_generator.cpp java generator, which is also checking for annotations, so I'm at a loss.

@mitza-oci
Copy link
Member

@mitza-oci Do you know how the Java API generator knows that a type needs typesupport? I thought just enabling IDL4 in idl2jni_codegen would make the java tests work with annotations, but they're missing Typesupport, so it's still relying on DCPS_DATA_TYPE somehow. I looked for checks for DCPS_DATA_TYPE, but that only exists in ts_generator.cpp java generator, which is also checking for annotations, so I'm at a loss.

First opendds_idl takes the user's IDL and generates FooTypeSupport.idl. Then idl2jni takes FooTypeSupport.idl and generates Java bindings. It doesn't need to know about which types are pragma'd/annotated.

The regular compiler tests now use annotations
@iguessthislldo
Copy link
Member Author

That's unfortunate, especially since it looked like at least one of them was almost done.

@mitza-oci mitza-oci changed the title Union Topics and IDL Topic Annotations a la XTypes XTypes topic and key annotations, Unions as top-level types Aug 2, 2019
@mitza-oci mitza-oci merged commit 0a3971e into OpenDDS:master Aug 2, 2019
@iguessthislldo iguessthislldo deleted the igtd/sample_annotations branch August 6, 2019 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants