Adding cli option to remove interfaces constraints on records.

[jferreyra-sc]

Context

The motivation for this change is that often you want to pass a set of interfaces in a record instead of different parameters in a function.
This helps for the code to be more scalable/maintainable as we can separate the creation of the parameters from the calling and handling of them.

Example:

class Instance /* ... */

struct Callbacks {
   std::nn_shared_pointer<TasteDescriptor> tasteDescriptor;
   std::nn_shared_pointer<CostDescriptor> costsDescriptor;
   std::nn_shared_pointer<LocalizationDescriptor> localizationDescriptor;      
   std::shared_pointer<LogDescriptor> logDescriptor;      /* optional */
};

class SDK {
    std::shared_ptr<Instance> createInstance( const Callbacks& callbacks );
};

Then the caller can do:

auto instance = SDK::createInstance( createAndConfigureCallbacks() );

Please notice that you were able to pass optional interfaces using the optional<> wrapper (which was properly interpreted and generated), and some users made used of a notnull<> extension to pass not null interfaces as a workaround. However, the extension workaround has a few weakness that are better to avoided (more boilerplate code, less readable code, some tricky implementation of this notnull extension).

Changes

This PR adds an option to remove the constraint, though, we should consider to directly always enable it as I see no reason to disallow them. My rationale is the following (please check for any wrong assumptions):

Records only go from one language to another during function calls, function calls can have parameters or return values that are interfaces. There is no environments where records are transmitted in a situation where interfaces are not supported.

Remote invocations are not a problem, for the same reasons explained above.

I imagine that if we use records for serialization to permanent storage, then interfaces wouldn't fly, but I believe the correct behavior would be to only restrict interfaces for this specific case, and not for the other cases.
On the same line, I guess DataRefs disallowed serialization to permanent storage already, so this case should not be concerning.

Admins, please let me know if you prefer the change as it is or if you prefer to always relax it.

Testing

I have tested the changes by extending the examples to use records holding interfaces instead of passing them directly. You can compare the diff to this branch here.

Both Android and iOS examples built and executed correctly.

[arekmula]

Hey @li-feng-sc

Is there a reason this one hasn't been merged? I guess there must've been a reason in the past to have that constraint, and these changes are pretty simple 🤔

[li-feng-sc]

Hey @li-feng-sc

Is there a reason this one hasn't been merged? I guess there must've been a reason in the past to have that constraint, and these changes are pretty simple 🤔

"there must've been a reason in the past to have that constraint" exactly, and I don't completely understand what was that reason.

[li-feng-sc]

Maybe @artwyman could answer that question.

[artwyman]

"there must've been a reason in the past to have that constraint" exactly, and I don't completely understand what was that reason.

I don't remember a specific reason. I know we always had a pretty strong separation of records being code passed by value and interfaces being objects passed by reference. I remember explaining that enough times that I probably knew a reason at some point, but I can't recall now. My instinct is that there wouldn't be any fundamental difference in complexity to passing an interface across the language boundary as an argument or return, vs. passing it in a record. I could imagine some additional risk of creating circular references, perhaps, but that's something you can already do if you're not careful.

I could be wrong here, but instinct is also that implementing such a feature would take some work beyond what's in this PR. I see code to disable exceptions in certain combinations, but I'd be surprised if the code generated was correct after simply removing the exceptions. I'd assume that previously-inaccessible codepaths are probably not going to work without modification. I could be surprised here, though. I'm curious how much has been tested, and would encourage adding some new unit tests to confirm how the newly-allowed cases might behave.

It's certainly possible to implement the Callbacks in your example above as an interface with getters and setters. I remember suggesting people do similar things when they asked about combining record+interface on the same object (i.e. an object with public data and also virtual methods), which was another aspect of the code/data separation argument. As another workaround, I'd also bet it's possible to do some template metaprogramming to return your callbacks in a tuple then automatically pass them into separate arguments to SDK::createInstance().

I realize you (original author, or new commenter) might not find such a workarounds satisfying in this case, but they should work with a small amount of effort. Making the broader feature work for all use cases might be harder, or might be easier than I think. Newer more flexible features seem good, but I'm not deep enough in the code to know the risks a prior anymore, and I'm not the maintainer who'd have to maintain the result anymore.