yeah, this is fine.

i'll let @cedric review the eo file updates, mostly looks good to me besides the inline comment i've come across to

I added a check in f796a21d72c47d7661349f14e6533299f750654a, but currently our eo files fail with that, so it's behind an env var.

i noticed that part will need more rewriting anyway, i'll look into it tomorrow.

Yes, definitely. I've already been working on this.

I also like the idea of an IRC meeting. Next or this weekend is good :)

Switching this to TODO, as the immediate issue is gone, but we might perhaps need some more work on constness that is yet to be discussed in order to make things right.

Do we really still want this? As I see it, there is fairly little point in polluting public API with wrappers over callback_call, considering callback_call is mostly meant to be called from within implementation in the first place, i.e. leaking it into public API is probably bad.

Yeah, that'd help.

Please give this a try after b8f28e2a99535a8e9780f9ea15572693bf0f7148

yes, I was thinking this could potentially allow an array of extra checks we couldn't do before to happen.

I was thinking Eolian tests would be a good place to run the "global" check pass. It wouldn't run directly during build, but we generally make sure make check keeps passing.

yeah, I think it's okay

It hasn't started yet. I've come to a conclusion that it is not possible to detect name clashes upon generation no matter what changes we make. I think perhaps we could take the middle ground and do checks only in stuff that is parsed at the time, and for global checking we could perhaps have another tool (sort of an Eolian static analyzer). But that's just a thought atm

This has long been fixed/irrelevant.

I think we could possibly do this now that we have units, but gotta investigate how. It will probably depend on T2251.

Yes, I noticed that as well. I thought we could perhaps make them const in the static funcs, but I'll have to investigate how much it breaks first.

I'm looking into this, but I fail to see why this would be necessary. The object is always const on getters, i.e. it's implied and there shouldn't be a need to set it manually. It doesn't make sense on properties in general, because a setter by definition changes the object.

Additionally, to point out one of @segfaultxavi's points:

I really dislike the two-hierarchies thing. I think it makes everything unnecessarily more complicated, besides, C-only information in eo files should be kept to a minimum only for interfacing into C; most APIs are fine as they are, and where different name for C is necessary, Eolian already provides all needed features.

Yeah, I understand what you mean, I'm just not sure if I like the solution. I can't really think of anything else either, though.

I'm not against the idea of generating the wrappers, but I'm not quite sure about the "empty" idea, it seems pretty hacky and ugly API-wise...

I committed and pushed the changes, but dropped the JS. It's better to keep it in its current state, because doing stuff like passing NULL in those calls as first param is wrong and will fail, but it will still build - so let's make it easier for the person who ends up fixing it.

the klass_def.hpp change is definitely wrong, you can cast state to unit, but not the other way around - besides, there is no reason not to use eolian_unit_class_by_name_get anyway.

I think this would be a good fit for a Lua script.

yeah, a wizard option is a good idea

IMO it'd be best to actually remove those because most people I've seen use E were annoyed by those and removed them immediately; the maximizing behavior would be much more useful and expected for most.

I applied your patch.

yeah, I don't think we can fix this with autotools. Maybe meson would help though, @bu5hm4n any ideas?

yeah, you're probably right; the current legacy field was never even meant to be a proper feature, more like a temporary hack to get enums for legacy APIs to behave as we want them to...

thanks

I'd unlink it from T5826 but leave it open...

i guess i'll just close this since part information is already done since a few days ago...

Since I won't have time for the proper solution in near future (because of other work elsewhere) I'll let @ajwillia.ms proceed with his workaround for the time being. A real solution will be implemented later as time allows, but not bound by this project.

This should be fixed by the maintainer.

We'll definitely need this, but I'm not sure whether processing some sort of XML from Doxygen is the best way to go at it... since this is just C we're dealing with, it's fairly simple as far as the language in headers goes; perhaps we could parse it manually (it shouldn't really be a big deal) which would allow for simpler integration into dokuwiki (doxygen carries a lot of clutter we'd have to strip off into the result). But I will need to investigate this to be able to tell for sure.

i expect to have it done by tomorrow evening...

alright, let's get it in during this week

Yeah, the test was wrong and so was the generator...

just FYI, keep in mind that if you "pass" the defined function as an argument in Eolian, it will actually generate *three* arguments in C; data, the actual funcptr, and a "free" funcptr (Eina_Free_Cb) - this is necessary to track lifetimes.

you're right, that should be fixed.

yeah, the generation for this should probably be changed; it's just a tad difficult because there is no real indication of what the group should be... OTOH, this issue should not be very relevant anymore, because the old Doxygen documentation is getting deprecated anyway...

I implemented a fix for the bytecode confusion in 67d1c0e51c01ba159f88adc6446cc31cee79c808 so similar cases should not happen again.

Done in 4a16e1d86253ce336464f26ee9244f25511cb8bb

this is what happens when people push into eolian without letting me review first... i didn't feel like calling out the responsible person at first because it was a change that needed doing anyway but i guess i will have to:

The right way really is to remove the bytecode if it fails loading and try loading the source file... so I guess I will do just that

looks like leftover cached bytecode made with previous version of luajit...

we should also figure out how to integrate the doxygen docs and the dokuwiki, because having two separate systems is just bad...

yeah, i think we can just proceed with this, if there are no further objections

Yeah, if the function becomes three parameters, it's technically possible to implement. I just fear that this is 1) not really necessary 2) rather ugly API-wise... Also, the way Eolian currently works doesn't assume multiple things can be generated from one, so that'd have to be updated as well...

I don't think eolian_gen is the right place to put this at all. It should go into the validator in the library. If you want me to implement this, I can...

yeah, will be a pretty simple fix

That sounds potentially sensible to me. EDD is coming up soon so we can also discuss the details more there.

I guess it's possible in theory, though it's more work than you think and I'm not sure if I like the idea yet. Also it doesn't properly handle types and other dependent things (which are shared between classes and multiple translation units) and I could probably think up a billion other potential issues that could get in the way, so the extra complexity is by no means small. Having to care about where things are put in the binary sounds super nasty and I hate it, too. So this is definitely not something to think about for this release cycle, if at all.

In T5577#88995, @barbieri wrote:

what I wonder (actually I should know, but I never remember) and maybe @q66 can clarify is if we have that information available in runtime and how/where?

@jpeg they won't work unless we figure out lifetime and memory management tracking... if you have ideas, feel free to suggest

The change 7007c3314d31d1c80d1670500d899ed9d7350a79 is a pretty bad idea as it seems to break build. It's never good to deifne a thing in two places like this, because it *will* result in conflict in Eolian for a good reason; the solution would be either putting the undefined type in the .eot file so it's in a single place, or just revert the commit for now.

yeah, _EXTRA_OPS should work for this purpose, even though it wasn't really written with this report in mind.

This is also something we could look into, however I've been doing some prototyping in my local tree and still not sure how I'd approach this. We should probably discuss this more and find some reasonable way for 1.20.

Yeah, I agree with @cedric here. This is something to look into. The parts syntax is probably good enough. I don't like the @part variant.