Module name too close to IDL keyword ?

[neoliant]

Hello,

With default OpenDDS-3.15 and 3.16 installation, when I try to compile the following IDL file :

module ATTRIBUTE
{
  @topic
  struct TopicA {
    string a;
  };
}; // module ATTRIBUTE

with the following commands :

tao_idl --idl-version 4 -I/usr/local/include --unknown-annotations ignore -Cw test.idl
opendds_idl -Cw test.idl
tao_idl --idl-version 4 -I/usr/local/include --unknown-annotations ignore -Cw testTypeSupport.idl

I have the following error with the last command Error - tao_idl: "testTypeSupport.idl", line 38: error in lookup of symbol: ATTRIBUTE::TopicA

The issue doesn't appear with a different module name. I understand this is closely related to the IDL keyword, but with a different case.

Am I doing something silly here? Any hint? Is this a bug? Help and thanks in advance ;-)

[mitza-oci]

The IDL language spec doesn't allow identifiers that collide with keywords case-insensitively.

[neoliant]

So what is the purpose of providing a flag that changes it from error to warning ?

The OMG Specification for the OPC UA - DDS Gateway comes with a normative IDL file (dds-opcua_services.idl) that contains such a module name. Who do you think should be contacted in that case ?

I guess prepending the module name with "_" is all that is needed.

[jwillemsen]

You should report a formal issue to the OMG at https://issues.omg.org/issues/create-new-issue

[neoliant]

I understand and I will do it.

Meanwhile, I tried using "module _ATTRIBUTE {" (like it is described in the IDL specs) and can pass the opendds_idl and tao_idl code generation steps. But latter on it doesn't compile.

test.idl :

module Foo {
module _ATTRIBUTE
{
  @topic
  struct TopicA {
    string a;
  };
}; // module ATTRIBUTE
}; // module Foo

main.cpp :

#include "testTypeSupportImpl.h"

int main (int argc, char *argv[])
{
   Foo::ATTRIBUTE::TopicA a;

   return 0;
}

CMakeLists.txt :

cmake_minimum_required(VERSION 3.9.4)
project(testATTRIBUTE CXX)
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(OPENDDS_CMAKE_VERBOSE ON)
set(CMAKE_PREFIX_PATH "/usr/local")

add_compile_definitions(USE_OPENDDS)

find_package(OpenDDS REQUIRED)
set(CMAKE_CXX_COMPILER ${OPENDDS_COMPILER})
find_package(ICU 63.0 COMPONENTS uc data REQUIRED)

add_executable(testATTRIBUTE main.cpp)

set_target_properties(testATTRIBUTE PROPERTIES OUTPUT_NAME testATTRIBUTE)
OPENDDS_TARGET_SOURCES(testATTRIBUTE
    test.idl
)

target_compile_features(testATTRIBUTE PUBLIC cxx_std_17)
target_link_libraries(testATTRIBUTE OpenDDS::OpenDDS)

Error :

testTypeSupportImpl.h:50:23: error: variable or field ‘set_default’ declared void
   50 | void set_default(Foo::_ATTRIBUTE::TopicA& stru);
      |                       ^~~~~~~~~~
/home/xavier/testATTRIBUTE/build/testTypeSupportImpl.h:50:23: error: ‘Foo::_ATTRIBUTE’ has not been declared

Any idea ?

[jwillemsen]

Looks you found a small issue in the code generator, maybe you can extend one of the unit tests under https://github.com/objectcomputing/OpenDDS/tree/master/tests/DCPS/Compiler as reproducer and make a pull request with the extension?

[neoliant]

Done. This presently breaks compilation, which I guess is what is looked after.

Shouldn't the issue be re-opened then?

[jwillemsen]

I reported the issue to the OMG earlier today.

[jwillemsen]

When you can create a fix please do so and add it to the PR

[neoliant]

Thanks for reporting to OMG. I'm afraid fixing the issue is beyond my knowledge. Any hint or doc I should focus on?

[iguessthislldo]

Shouldn't the issue be re-opened then?

Yes, because you should be able to use a keyword if it's escaped.

I'm afraid fixing the issue is beyond my knowledge. Any hint or doc I should focus on?

That's okay, because I think I fixed it.

https://github.com/objectcomputing/OpenDDS/blob/98edd335e2c5b2401193747678ab99634b384721/dds/idl/dds_generator.cpp#L63-L67

This function always adds an escaping underscore, but the C++ name doesn't have an underscore if it's escaped. It only needs one if when we're writing the *TypeSupport.idl file. I will try to push the fix into your PR in a little bit.

[neoliant]

I had found the same place, but then was wondering : what if somebody chooses a name different from an IDL keyword, and starting with an '_' ? Should we compare the string with a list of keywords?

Aren't there other functions that call the same function but require the escaping '_' ?

I'm really unfamiliar with all this and glad you came by!

[iguessthislldo]

what if somebody chooses a name different from an IDL keyword, and starting with an '_' ? Should we compare the string with a list of keywords?

It looks like taoidl always takes the `` away unless it's a C++ keyword. So we don't have to check for IDL keywords, but thanks for bringing this up because we do have to make sure it's not adding underscore to escaped C++ keywords, which get a _cxx_ prefix.

Aren't there other functions that call the same function but require the escaping '_' ?

Like I said I had to make a special case for the generated *TypeSupport.idl because it's IDL, but I think that's the only one.

[neoliant]

Got it. Thank you so much !

[iguessthislldo]

So the fun part about this is a C++ keyword topic name like class goes from class in the input IDL to _cxx_class in the *TypeSupport.idl to cxx_class in the *TypeSupportC.h.

[jwillemsen]

I had found the same place, but then was wondering : what if somebody chooses a name different from an IDL keyword, and starting with an '_' ? Should we compare the string with a list of keywords?

No, we shouldn't check for keywords, IDL could be extended with other keywords in the future which we don't know of at this point, a leading underscore in IDL should always be removed.