Add support for equality operators via new 'requires' keyword for interfaces

[AndrewAtAvenza]

This is the proof-of-concept for adding the new interface keyword requires(). While this keyword will currently accept both eq and ord, it only generates code for eq. Also, it currently only works for Java -> C++ (not even the reverse).

Currently, eq means defining both 'equals()andhashCode()in Java and directing them tooperator==(const & other) constandint32_t hashCode() const` respectively. The former should be fine, but the latter could clash with a user-defined method name. Maybe this is an acceptable trade-off though?

[a4z]

you should be able to fix the formatCheck CI by running sbt scalafmtAll
see https://djinni.xlcpp.dev/djinni-generator/developer-guide/#code-formatting

if that stopped working, please let me know

[AndrewAtAvenza]

you should be able to fix the formatCheck CI by running sbt scalafmtAll see https://djinni.xlcpp.dev/djinni-generator/developer-guide/#code-formatting

if that stopped working, please let me know

Much obliged! I took a look at that on Monday but thought I had to install it and could figure out how to do that. Glad it's just there, that's really handy.

Sorry it's been quiet the last couple of days. I'm landing some big branches and blocking a release. Should be back to this tomorrow or Friday. I took a quick look at how it might look publishing Java to C++ and I've got some thoughts I'll post to the discussion thread later.

[a4z]

Sorry it's been quiet the last couple of days. I'm landing some big branches and blocking a release. Should be back to this tomorrow or Friday. I took a quick look at how it might look publishing Java to C++ and I've got some thoughts I'll post to the discussion thread later.

No worries, take your time. This an OS project, so contributing shall also make fun. No stress

[AndrewAtAvenza]

I finally added the integration test but it reminded of one thing that's missing before I can offer it for review: I'm basically hardcoding in the C++ method names in the Operators sub-class as equals() and hashCode() but they should probably be subject to the naming conventions specified when djinni was run, right? So hashCode() should really be hash_code() in the IT because it uses the default settings, but it's still hashCode().

Obviously there is logic somewhere that takes a method name like hash_code and spits out HashCode() or hashCode() or hash_code() depending on the settings. I can't add these methods to the method list obviously, since they need to live in the Operators() sub-class namespace, so I'm hoping that the logic applied to each entry in the method list is available to be invoked to 'mangle' a name; or if not, can it be altered to be available on demand?

[a4z]

yes, there is some 'magic' that does naming tricks:
https://djinni.xlcpp.dev/djinni-generator/cli-usage/#identifier-style

I assume you mean that.

I was never a big fan of this. Hard to test and awful to maintain.
I am not even sure it makes sense; maybe the only reason it is there is that the original authors had to please several teams with their different naming conventions. Who knows.

The question is, could you have access to the various *IdentStyle and get the names from it,
or is that a problem?
If it works hardcoded, we probably are okay with that. For this special case, user options are limited, and we put that into the docs. (would that work, or did I understand the problem wrong?).

[AndrewAtAvenza]

The question is, could you have access to the various *IdentStyle and get the names from it, or is that a problem? If it works hardcoded, we probably are okay with that. For this special case, user options are limited, and we put that into the docs. (would that work, or did I understand the problem wrong?).

Yeah, that is exactly what I was talking about. I figured if it was possible to access it, better to conform. If it's not possible though and you're cool with hard-coding it, then I guess it is ready? Well, once I re-apply the formatting I forgot last push. equals() and hashCode() OK or should I go with equals() and hash_code()?

[a4z]

The various ident styles are accessible via the Spec class, and this spec is part of every generator or in the Marshall classes via properties. Here is an example where the function name is resolved
I guess you could do the same if you want

[AndrewAtAvenza]

The various ident styles are accessible via the Spec class, and this spec is part of every generator or in the Marshall classes via properties. Here is an example where the function name is resolved I guess you could do the same if you want

I'll give it a shot, might as well be consistent if I can. If not, well, we can leave it hard-coded.'

Thanks!

[AndrewAtAvenza]

OK, equality should be good to go. The only thing I haven't done is finish implementing the compare() mapping, but I can do that shortly -- I just don't have an easy go-to test case so I'll have to rig one up on my project.

It looks like the idCpp.method() worked perfectly so I pushed that in.

[AndrewAtAvenza]

Sorry this has taken so long, but I think it's finally ready for review!

[a4z]

Cool!

I am confused to see the .github files in the diff.
You merged the main branch, strange.

For the rest, I trust in your expertise because, after this work, you are more into the topic of djinni generators than I now.

So we will keep this here open for a while so others can check, and then merge after a while

[AndrewAtAvenza]

I am confused to see the .github files in the diff. You merged the main branch, strange.

That confuses me as well. If you want, I can rebuild the branch fairly easily off the new HEAD to be sure, it'd just mean copy & pasting a dozen files.

[a4z]

You worked enough; I am going to approve that now

[a4z]

I will leave that open for a day or two, then I merge it (if nobody else does it, who can, feel free to do it)

Once merged, we will have the 'latest' release. I will send out some announcements so people can test before we tag a new release.

[AndrewAtAvenza]

Just had a thought: is there documentation that needs to be updated as part of this PR? e.g. explaining how to use the feature or how its limited to Java?

[a4z]

Just had a thought: is there documentation that needs to be updated as part of this PR? e.g. explaining how to use the feature or how its limited to Java?

Excellent thought; thank you so much!

This is very likely the best place to add the new option:
https://github.com/cross-language-cpp/djinni-generator/blob/main/docs/idl.md

The docs using mkdocs , the part can be previews local by just using this repo,
but the final homepage is done via this repository https://github.com/cross-language-cpp/cross-language-cpp.github.io

For local, if you have a python even, pip install and mkdocs serve should give you the preview of the pages for the generator

[AndrewAtAvenza]

Excellent thought; thank you so much!

This is very likely the best place to add the new option: https://github.com/cross-language-cpp/djinni-generator/blob/main/docs/idl.md

The docs using mkdocs , the part can be previews local by just using this repo, but the final homepage is done via this repository https://github.com/cross-language-cpp/cross-language-cpp.github.io

For local, if you have a python even, pip install and mkdocs serve should give you the preview of the pages for the generator

Sounds good, I'll find time this afternoon to write something up. Might as well be complete!

[AndrewAtAvenza]

Sorry for the delay, work stuff came up!

I tried to run the documentation locally but ran into a problem: doesn't seem to work on Windows! Fortunately, the changes I'm making are pretty minor, the styling is minimal and GItHub has pretty good MD previews. I think it should be okay, but please let me know if I should change anything. I mostly used the Deriving section as a template, but as a sub-section of Interfaces.

[a4z]

Thanks for trying!
Yes, Windows can be a problem. I run containers for mkdoc works; I should add that, but not now.

Please add the docs. If there should be something to fix, I can do that.

[AndrewAtAvenza]

Thanks for trying! Yes, Windows can be a problem. I run containers for mkdoc works; I should add that, but not now.

Please add the docs. If there should be something to fix, I can do that.

I pushed the additions when I made the comment -- updating the idl.md file (using GitHub's preview feature). There wasn't a whole lot to add so it didn't really need much in the way of previewing, more proofing!

[a4z]

Looks good, lets give the PR two more days, in case anyone wants to test it, and the I will merge it