Open xparq opened 1 year ago
Both GCC and MSVC... Maybe the same double-delete bug I found in the fluent POC: the compiler generates a default copy ctor and selects that instead of the move ctor... But, unfortunately, the fix that worked there, does nothing here:
WidgetContainer(const WidgetContainer&) = delete;
WidgetContainer(WidgetContainer&&) = default;
Sigh. So... Is it the early destruction of the HBox temporary? Or related to the callback copy?
In a similar, but other, crash (due to another migration-related change in the code), this is what GDB saw:
Thread 1 received signal SIGSEGV, Segmentation fault.
0x00007ff641816336 in std::_Function_handler<void (sfw::Widget*), sfw::GUI::themeChanged()::{lambda(sfw::Widget*)#1}>::_M_invoke(std::_Any_data const&, sfw::Widget*&&) ()
(gdb)
From the experiences gathered so far (below): might be worth introducing, as a "STRICTLY EXPERIMENTAL" (and very dangerous etc.) feature (because it's so satisfying, when works), but it's way too far from replacing the old style, or even from being reasonably complete!...
And definitely REQUIRES strong setup-time diagnostics, too: it'd be tempting to reorder widgets and set them up first offline, just to be able to nicely "stream" them all into the tree afterwards, in a clean & tidy structure -- but even simple things like "named add()" won't work with free-standing containers... (It's an existing problem (#109) for the old style too, but probably less frequent there.)
TODO:
[x] Field-test my fluent POC (in a new branch): try migrating the test/demo, to see what the problems are. (See them below...)
[ ] Reconcile the method-chaining return value: ref or ptr?! Can't have both... This should of course match (and/or decide...) the overall style of the API. The thing is, neither of them quite work as yet... (See the crash with refs, and the template shadowing problem with ptrs!) This already (at least) compiles (due to
<<
accepting rhs widget pointers):But it still crashes: the returned method-chaining pointer lies: it's an lval ref to a temporary! :-o :-/ (And it's probably unfixable, without doubling all the setters, or forcing them to templates (and even then I'm not sure it could work)!) This works, but uglier (and even slightly longer) than the classic
->add(Label(...))->set(...)
style:<< (new Label(...))->set(...)
[ ] This other crash... It may be a showstopper! (See next issue comment!)
[ ] Add "stream manipulator" helper functions to support the legacy named addition! (
add(widget, "name")
) E.g.... << named(...) << ...
with the same, existing signature ofadd(...)
would be fine, I guess.[ ] Also support other custom add methods, like
Form:add("label", widget)
! Somehow... Obviously just keep the existingadd()
as a baseline, but try to improve upon that... (that's what I meant by that "somehow"). Could e.g. initializer lists work? E.g.form << { "name", widget }
? Even if yes, that would be yet another obscure stylistic element for users to learn... :-/[ ] Resolve the ptr vs ref asymmetry:
[ ] How to save a newly added item in the fluent style? I mean in a better (or at least not worse...) way than the (already pretty optimal) old style:
Examples:
This is fine (works already):
This is fine, too, but doesn't do what you might think... (indentation doesn't override op precedence! ;) ):
This would do what you wanted -- but crashes (see next issue comment!):
This works fine -- but mixing pointers and refs is sick:
Maybe committing to all pointers (like in the "classic" version anyway) and using just global
PTR << ...
overloads could work? Unfortunately, this is outright rejected by the compiler for bogus arg. types, apparently even before the templates would be considered (at least no mention of those in the -- unusually terse -- error, in either compilers):(NOTE: OK, so this is probably a case of "template shadowing", where some existing
<<
ops (instd::
?) take precedence over my templated version. I also suspected it to be an obscure case of namespace resolution with ADL, butusing namespace sfw;
didn't make a difference, so... Seems to be a lost case. :-/ )FYI, this (even more mixed-up version) also works: