Overly-Restrictive Check in IDL Compiler

[allenh1]

I was doing some work with annotations the other day, and ran into this message.

Translation unit contains both annotations and #pragma keylist directives, use only one of these or use the -f keylist command line option to force using only #pragma keylist and ignore annotations

I was very confused when I saw this, as I was not using #pragma keylist in addition to the @key annotation, so I was surprised the generator exited with error an error. Digging into it a bit more, I think that this exception will throw if ANY annotation is present with #pragma keylist.

https://github.com/eclipse-cyclonedds/cyclonedds/blob/e54f6eeaa2bc5d7860975507ef60802246ffbb46/src/tools/idlc/src/idlc.c#L377-L382

Was this intentional? I believe the check should be

else if (pstate->keylists && pstate->annotations)  {
  for (idl_annotation_appl_t *a = annotation_appls; a; a = idl_next(a)) {
    if (a->annotation && (0 == strcmp(a->annotation->name, "key"))) {
      /* produce error and exit */
    }
}

[eboasson]

Hi @allenh1 !

What is doing is according to our intent: the #pragma keylist syntax is the ancient, pre-XTypes, OpenSplice solution that (unfortunately) was inherited by Cyclone, and we really want to get rid of it. Of course we can't just stop accepting code that uses it and so we decided on requiring one to use either the old way or the new way using annotations, but no mixing of them.

If you just replace all the #pragma keylists with @key annotations, the only DDS implementations you can't use the new IDL files with are (as far as I know) OpenSplice and Cyclone DDS pre-0.8.0. And who would want to go there?

But as always, we make mistakes just as often as everyone else. If there is good reason to be less strict, then perhaps we should weaken the check like you suggest.

[allenh1]

@eboasson thanks for the response.

When I ran into this print message, it was a result of having the pragma and an unrelated annotation on the same struct. We did remove the pragma from the message (as it was not even being used), but I figured I would ask, as I don't see how this use case was invalid.

[thijsmie]

While I think the warning could be a bit better, I actually agree with the current implementation. The only reason #pragma keylist exists in this codebase at all is so OpenSplice users can use their idl files unchanged. If you want to use anything new and annotation related you should switch over the pragmas too. Perhaps we should even lock away the pragmas behind a cli switch now and fully remove them in a version or two.