Jump to page: 1 24  
Page
Thread overview
[Issue 16291] phobosinit fails to register encodings on individual tests
Jul 19, 2016
Ketmar Dark
Jul 19, 2016
Ketmar Dark
Jul 19, 2016
Ketmar Dark
Jul 19, 2016
Ketmar Dark
Jul 19, 2016
Ketmar Dark
Jul 19, 2016
Ketmar Dark
Jul 19, 2016
Ketmar Dark
Jul 19, 2016
Ketmar Dark
Jul 19, 2016
Jack Stouffer
Jul 19, 2016
Jack Stouffer
Jul 19, 2016
Ketmar Dark
Jul 19, 2016
Ketmar Dark
Jul 20, 2016
Ketmar Dark
Jul 28, 2016
Johannes Pfau
Aug 24, 2016
Ketmar Dark
Aug 24, 2016
Sobirari Muhomori
[Issue 16291] phobosinit never gets called
Oct 01, 2016
Martin Nowak
[Issue 16291] phobosinit never gets called (EncodingScheme.create fails)
Oct 01, 2016
Martin Nowak
July 19, 2016
https://issues.dlang.org/show_bug.cgi?id=16291

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com

--
July 19, 2016
https://issues.dlang.org/show_bug.cgi?id=16291

Ketmar Dark <ketmar@ketmar.no-ip.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ketmar@ketmar.no-ip.org

--- Comment #1 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
it seems that at least for x86 GNU/Linux ctor is called if i'm using libphobos2.so instead of libphobos2.a. i inserted printf there, and yes: with static phobos ctor is skipped, but with .so phobos it is called.

i found this not with test, but in my own app, which mysteriously stopped working when i linked it with static phobos.

--
July 19, 2016
https://issues.dlang.org/show_bug.cgi?id=16291

--- Comment #2 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
aha. i see.

the thing is that std.internal.phobosinit is not imported from anywhere, so linker simply not including it in resulting executable!

but with .so, it is included with all other phobos modules (as linker cannot throw it away from *library*), and druntime then got the list of all modules included in .so.

i.e. we have to do "import std.internal.phobosinit;" in "std.encoding". but that defeats the purpose of moving the whole thing to separate module, i guess.

what was the idea behind moving that initialization to separate module? it seems that leaving it in std.encoding is not conflicting with anything: not one of phobos modules that imports "std.encoding" has module ctors.

--
July 19, 2016
https://issues.dlang.org/show_bug.cgi?id=16291

--- Comment #3 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to Ketmar Dark from comment #2)
> aha. i see.
> 
> the thing is that std.internal.phobosinit is not imported from anywhere, so linker simply not including it in resulting executable!
> 

Well... That's weird.

I assumed module ctors would still be included, even if not imported. How the hell does vibe.d work, where everything is set up in a module ctor?

> 
> i.e. we have to do "import std.internal.phobosinit;" in "std.encoding". but that defeats the purpose of moving the whole thing to separate module, i guess.

Yes, it does. I purposely removed all imports of phobosinit (was called processinit, and std.process imported it) to avoid cycles.

> what was the idea behind moving that initialization to separate module? it seems that leaving it in std.encoding is not conflicting with anything: not one of phobos modules that imports "std.encoding" has module ctors.

There was a cycle between std.encoding and something else. The easiest thing to do was to remove the constructors.

--
July 19, 2016
https://issues.dlang.org/show_bug.cgi?id=16291

--- Comment #4 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
as nothing directly referring to module ctors, linker just doesn't know that it should not optimize 'em away. maybe we can fix that by emiting some flag for .o files, but i don't really know -- i never even read about dwarf object file format, so i'm not sure if such flag exists.

that is, linker really does it's work here: if nothing directly using the symbol from object file, that object file is not linked to the final binary. tbh, i don't know how can we solve this: if we will mark all module ctors as "include always" (if it's possible), we will have alot of unnecessary (and probably unused) code linked to the final binary. it doesn't hurt much, but still...

for now, easy fix is to move that thing back to "std.encoding", as it seems to not conflict with anything now. and probably create a new issue that documents this behavior.

maybe we can introduce some UDA to mark modules that should not be optimized away by smartlinking, or something like that, 'cause i guess that similar issues are very possible in the future, as phobos becomes more and more complex, with complex interdependencies.

--
July 19, 2016
https://issues.dlang.org/show_bug.cgi?id=16291

--- Comment #5 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
p.s. i think that vibe.d works 'cause it passes all it's files to dmd instead of creating .a library. then dmd generating single object file with everything necessary inside, including module ctors. but for .a libraries, dmd is writing alot of object files (one per function, i believe) to allow smartlinking.

so passing all phobos .d files to dmd should work as expected, but using .a isn't. and i believe that vibe.d will have the same issue if it will be complied as .a library.

--
July 19, 2016
https://issues.dlang.org/show_bug.cgi?id=16291

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|nobody@puremagic.com        |schveiguy@yahoo.com

--- Comment #6 from Steven Schveighoffer <schveiguy@yahoo.com> ---
Hm... I also realized just now that what I did is not correct. There's a chance that some other static ctor depends on std.encoding registry having been set up. But the link between encoding and phobosinit isn't there, so the ordering will be wrong.

I'll fix this in a PR.

--
July 19, 2016
https://issues.dlang.org/show_bug.cgi?id=16291

--- Comment #7 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to Ketmar Dark from comment #4)
> for now, easy fix is to move that thing back to "std.encoding", as it seems to not conflict with anything now. and probably create a new issue that documents this behavior.

This would bring back the cycle that it fixed. I should tell you that cycle detection is currently broken, see issue 16211. I was getting cycle errors with the fix for that issue until this fix was in place.

--
July 19, 2016
https://issues.dlang.org/show_bug.cgi?id=16291

--- Comment #8 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
>I should tell you that cycle detection is currently broken
yes, i know. but std.encoding seems to not import anything that may indirectly import it back, and it *seems* to not break anything for my system.

i'm not insisting on that solution, though. maybe we can use some kind of "lazy initialization" scheme instead, by writing a weird hack: on the first call to `EncodingScheme.create()` we can find std.encoding in module list, and register every encoder it has. i think this *may* work (if linker will not throw away "unused classes"). i probably will try to write such code to see if it will work.

--
July 19, 2016
https://issues.dlang.org/show_bug.cgi?id=16291

--- Comment #9 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to Ketmar Dark from comment #8)
> yes, i know. but std.encoding seems to not import anything that may indirectly import it back, and it *seems* to not break anything for my system.

That's because you don't have proper cycle detection on your system :) I manually verified the cycle did exist. I don't remember the exact cycle modules.

Note that there really aren't any cycles in terms of std.encoding requiring some other module ctor to work correctly, but the runtime doesn't do a proper job of sorting the order of the ctors, and when that is fixed, it can't solve the ordering.

My fix that I pushed was not right -- the dependency is lost, and now may result in incorrect ordering still.

--
« First   ‹ Prev
1 2 3 4