autowrap icon indicating copy to clipboard operation
autowrap copied to clipboard

Handle all storage classes gracefully

Open Laeeth opened this issue 4 years ago • 11 comments

At the moment csharp fails at build. I don't know about others.

For SIL const ref and immutable have often been problematic.

Ideally return a tuple of the return type of any and values returned as out. For ref do the obvious. For const ref that too.

Laeeth avatar Aug 22 '19 02:08 Laeeth

At the moment csharp fails at build

When storage classes are used?

I don't know about others.

Python has tests to make sure const and immutable member functions can be used at the very least.

For SIL const ref and immutable have often been problematic.

SIL uses autowrap to reflect. Everything else is SIL code.

Ideally return a tuple of the return type of any and values returned as out

I don't see this being needed in Python or Excel. I'd have to research on C#. Mutating arguments is easy and should work in the former two.

atilaneves avatar Aug 22 '19 09:08 atilaneves

Yes in at least some cases for example with out parameters.

On SIL yes I know, because I wrote the first version of autowrap for SIL and I tried to get changes to reflection to address the problems I found and some of those were made but not all. Until John introduced a regression, mostly on the SIL side autowrap would just skip it and if there was an error that made it impossible to build then it would usually come from autowrap reflection.

Still had problems trying to wrap Xenon pure D version last time I tried (on reflection side) So I think we should have a test for reflection that involves trying to wrap something larger as part of the tests. I honestly don't see anything wrong with trying to wrap all of phobos and all of mir.

Laeeth avatar Aug 23 '19 10:08 Laeeth

Atila would you perhaps consider arguing for your position just as a habit. Just a few words more. You cannot see it being needed but if you are autowrapping xenon for python and excel how can doing something like this fail to be needed? How is the number gonna get into excel? The logical implication is I just don't want to bother autowrapping a code base with lots of out parameters. You're on your own. But we have code like that (I disapprove but it was too late) and the implication is then ultimately we have to write manual shims or fork our own project! It is not what I expected to be arguing in favour of code generation over manually written boilerplate.

I see just now part of the reason is you are assuming the code is returning a slice by reference. The examples I have in mind return doubles by reference. Grep for out parameters in Xenon.

Laeeth avatar Aug 23 '19 10:08 Laeeth

I tried to get changes to reflection to address the problems I found and some of those were made but not all

Do you have a list of what wasn't done? Or better yet, what didn't work?

I honestly don't see anything wrong with trying to wrap all of phobos and all of mir.

I agree, they should both work.

The logical implication is I just don't want to bother autowrapping a code base with lots of out parameters

If functions with out parameters currently can't be wrapped that needs to be fixed. I confess I hadn't considered them, probably because I don't usually write them.

ultimately we have to write manual shims or fork our own project!

I definitely don't want us to do that. Things should just work.

atilaneves avatar Aug 24 '19 15:08 atilaneves

I'm a bit short of time this second but briefly. No, I don't have a list of what wasn't done because sometimes developers have tunnel vision. I tried for example to have xenon wrapping introduced into SIL and it was dropped without trying to solve the problem of making a beginning on a bigger problem an alternative way. It's not like the choices we're limited to let my work rot and not do anything to advance the project. The problem is that it's not obvious what exactly the problem is because of complexity of original code. The falling test is for example that pick a code base - xenon or phobos and try and wrap it. If I had more time I could spend time writing it up, but I don't. And if I don't do it myself, basic things that are a pain in the neck just don't get done. So to make breakthroughs I need to do them, but then it's most unfortunate that they get blocked because somebody doesn't see why. It's impossible to do proper code review without actually understanding the problem being solved and the way to do that if I say storage classes arent properly handled is to try it and see or read the code. In an ideal world I would write a beautiful and complete report, but if we lived in that world there might be much less interesting work to do so it's a package deal.

I don't write them either, but see std socket and see Xenon. Try const ref and ref too.

Csharp tests fails to build. So your insistence on not adopting a workaround to dub that's not that bad leads to failing the Joel test and more importantly deterring people from using autowrap to do quite critical things. Leading to manually written marshaling code in case of OT and stuff just not being done in another case.

I tried to show Walter how to use it on day trip to London and I couldn't even figure out how to run the tests in the fifteen minutes I had to show him...

Laeeth avatar Aug 26 '19 00:08 Laeeth

Duplicate of #42

atilaneves avatar Aug 28 '19 11:08 atilaneves

This isn't a duplicate. It's broader than the previous issue - all storage classes. On the other hand I guess const is a type qualifier not a storage class. So the right thing to do is write up the union of both issues

Laeeth avatar Aug 29 '19 01:08 Laeeth

@atilaneves

Laeeth avatar Aug 29 '19 01:08 Laeeth

#42 is tracking in, scope const and ref parameters and const ref returns. This issue tracks the remaining D storage classes.

atilaneves avatar Sep 02 '19 11:09 atilaneves

Thanks. L

Laeeth avatar Sep 03 '19 23:09 Laeeth

Encountered this problem today:

class Foo {
  immutable string BAR = "bar";
}

when build .dll, autowrap will report:

ERROR! Autowrap could not wrap aggregate mydll.Foo for Python

change immutable to const has the same issue, I have to remove both.

mw66 avatar Jun 25 '22 22:06 mw66