Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be able to compile using upstream compiler #9

Closed
6 of 7 tasks
leandro-lucarella-sociomantic opened this issue Feb 8, 2017 · 28 comments
Closed
6 of 7 tasks

Be able to compile using upstream compiler #9

leandro-lucarella-sociomantic opened this issue Feb 8, 2017 · 28 comments

Comments

@leandro-lucarella-sociomantic
Copy link
Contributor

leandro-lucarella-sociomantic commented Feb 8, 2017

For now Ocean for D2 is stuck at 2.070.x, but even then there are custom modifications that are needed to be able to build / use it (this compiler is built as part of Ocean automated testing by applying patches to the upstream compiler.

These are the issues:

After all the issues are solved upstream, we still need to upgrade to an upstream DMD that have all the fixes, which might take a while too.

@mihails-strasuns-sociomantic
Copy link
Contributor

There are various issue that have already been fixed in later versions of upstream compiler but have to be picked for 2.070 base we use. For example:

  1. GC.stats (Expose gc_stats as core.memory.GC.stats dlang/druntime#1610), available starting with 2.072
  2. 5.1 GCC fixup (Fix test suite C++ test with GCC 5.1 dual ABI dlang/dmd#5686), available starting with 2.072
  3. Extra C bindings, available starting with 2.073

Those changes provide most of patches in https://github.com/sociomantic-tsunami/ocean/tree/v2.4.x/docker/dmd-transitional/patches (I will provide PR that rearranges patches in more clear way later).

Throwable.message is the only issue which is still unresolved. Intended way to workaround it is to version away for upstream compiler all tests that rely on specific message content. Some practical issue examples it causes would be broken NamedTest formatting and parts of application framework that have "empty error message" special case.

@leandro-lucarella-sociomantic
Copy link
Contributor Author

Thanks, updated the summary with a TODO list.

@mihails-strasuns-sociomantic
Copy link
Contributor

Thanks, updated the summary with a TODO list.

Do you need a full list of fixes in later versions with PR links? I only picked some as an example.

@leandro-lucarella-sociomantic
Copy link
Contributor Author

Just for the records, we are pushing for DIP1007 to add a feature to declare @future symbols so we can add Throwable.message with a non-breaking migration path.

@nemanja-boric-sociomantic
Copy link
Contributor

nemanja-boric-sociomantic commented Sep 7, 2017

Throwable.message is now in upstream's druntime master 🎉 dlang/druntime#1895

@gavin-norman-sociomantic

Crazy days.

@leandro-lucarella-sociomantic
Copy link
Contributor Author

This didn't made it to 2.076, but it should be in 2.077, right?

@mihails-strasuns-sociomantic do you know how much problems are expected to be found when moving from v2.071 to v2.076? We shouldn't expect huge breakage after the fixing of imports, right?

@nemanja-boric-sociomantic
Copy link
Contributor

This didn't made it to 2.076, but it should be in 2.077, right?

Correct.

@mathias-lang-sociomantic
Copy link
Contributor

do you know how much problems are expected to be found when moving from v2.071 to v2.076?

Not much. A couple deprecations, and most likely some scope related regression, although we might get lucky as we are not using @safe. Oh and the damn cyclic import thing but that's trivial to work around.
I am already using some part of ocean for my personal projects, and they use upstream.

@mihails-strasuns-sociomantic
Copy link
Contributor

Cyclic import problem is probably the painful one (workaround are simple, but solving it properly often requires restructuring of modules). Other than that I don't expect much trouble but it is one of lowest priority things so I haven't put any research into it.

@nemanja-boric-sociomantic
Copy link
Contributor

Trying to compile with dmd v2.076, this is the current set of issues:

nemanjaboric@labs-129:/home/nemanjaboric/work/ocean  git:(v3.x.x*) $ DC=dmd make DVER=2
   rdmd build/devel/tmp/allunittests
./src/ocean/text/convert/Layout_tango.d(825): Error: undefined identifier TypeInfo_Typedef
./src/ocean/text/convert/Layout_tango.d(826): Error: undefined identifier TypeInfo_Typedef
./src/ocean/text/convert/Layout_tango.d(1022): Error: template instance ocean.text.convert.Layout_tango.Layout!char error instantiating
./src/ocean/io/Stdout.d(88): Error: template instance ocean.io.Stdout.TerminalOutput!char error instantiating
./src/ocean/io/Stdout.d(121): Error: template instance ocean.io.stream.Format.FormatOutput!char error instantiating
./src/ocean/io/Stdout.d(88):        instantiated from here: TerminalOutput!char
./src/ocean/io/console/AppStatus.d(121): Error: template instance ocean.text.convert.Layout.StringLayout!char error instantiating

Look who removed it: dlang/druntime@bec52ee

:D

@mathias-lang-sociomantic
Copy link
Contributor

Damn....

@mathias-lang-sociomantic
Copy link
Contributor

We're not going to upgrade anytime soon anyway, are we ? 👼

@nemanja-boric-sociomantic
Copy link
Contributor

The other set of errors is coming from druntime's binding that are adjusted for DIP1000:

./src/ocean/io/select/selector/TimeoutSelectedKeysHandler.d(140): Error: function core.stdc.stdlib.qsort (scope void* base, ulong nmemb, ulong size, extern (C) int function(scope const(void*), scope const(void*)) @system compar) is not callable using argument types (ISelectClient*, ulong, ulong, extern (C) int function(const(void*) a_, const(void*) b_) pure nothrow @nogc @system)
./src/ocean/io/select/selector/TimeoutSelectedKeysHandler.d(149): Error: function core.stdc.stdlib.bsearch (scope const(void*) key, scope inout(void)* base, ulong nmemb, ulong size, extern (C) int function(scope const(void*), scope const(void*)) @system compar) is not callable using argument types (void*, ISelectClient*, ulong, ulong, extern (C) int function(const(void*) a_, const(void*) b_) pure nothrow @nogc @system)

@leandro-lucarella-sociomantic
Copy link
Contributor Author

How bad is this, is it something we can change at our end? I would really really want 2.077 to be the DMD upstream version we can start finally using.

@mathias-lang-sociomantic
Copy link
Contributor

I wasn't aware of this plan. Is there any reason why ? The removal can be worked around trivially (this case is never hit in D2). For the scope adjustement, those ones in particular should be simple to fix, as contravariance seems semi-implemented. E.g. the following compiles:

void foo (scope void delegate (void* ptr))
{

}

void main ()
{
    scope void delegate (scope void* ptr) oops = (scope void* ptr) {};
    foo(oops);
}

With our dmd-transitional, so we can add the scope annotation without breaking other code I think.

@leandro-lucarella-sociomantic
Copy link
Contributor Author

I wasn't aware of this plan.

It's been more than 2 years now that we are trying to fix the differences of what we need and upstream to be able to use upstream. Is not that v2.077 is special, is just that we finally got to solve the infamous Throwable.message issue that was the only left reason why we couldn't use upstream so far.

Is there any reason why ?

Because we don't want to maintain a fork of the compiler any more. dmd-transitional MUST DIE.

@mathias-lang-sociomantic
Copy link
Contributor

mathias-lang-sociomantic commented Sep 18, 2017

dmd-transitional MUST DIE.

Amen

@stefan-koch-sociomantic
Copy link
Contributor

We should able to do that soon afaics.
version out Typedef_Typeinfo and we should be good to go.

@mihails-strasuns-sociomantic
Copy link
Contributor

Because we don't want to maintain a fork of the compiler any more.

Before everyone gets excited, I should say this isn't very realistic goal (for anything but just ocean) right now. Going to discuss this with @leandro-lucarella-sociomantic and @mathias-lang-sociomantic but please hold the party a bit :)

@Geod24
Copy link
Contributor

Geod24 commented Oct 10, 2017

Updated ocean on my personal project, and had to convert it to D2 again.
Here's the issues I had:

@leandro-lucarella-sociomantic
Copy link
Contributor Author

What does this mean? I don't get the implications. Is this to update ocean to the latest DMD?

@mathias-lang-sociomantic
Copy link
Contributor

@leandro-lucarella-sociomantic : Basically I'm using ocean in my private projects, which use upstream's compiler. It works fine thanks to rdmd, but since I have to d1to2fix + git push on every release I tested it, and those are the things I found. There's also:
./src/ocean/util/log/PeriodicTrace.d(190): Error: undefined identifier __va_argsave, did you mean struct __va_argsave_t?

And we use this identifier a lot, but it should die fairly soon (v4.x.x hopefully).

@leandro-lucarella-sociomantic
Copy link
Contributor Author

Usual scope thing with different callback type as mentioned here.

Is there a dedicated issue about this?

@mathias-lang-sociomantic
Copy link
Contributor

There's a P.R. to fix it.

@mathias-lang-sociomantic
Copy link
Contributor

Can we assign this to v4.0.0 to make matters clearer ? (CC @mihails-strasuns-sociomantic )

@mihails-strasuns-sociomantic mihails-strasuns-sociomantic added this to the v4.0.0 milestone Oct 23, 2017
@mihails-strasuns-sociomantic
Copy link
Contributor

Sounds good. I hoped to fit it into v3.x.x but doesn't seem like circular import issue can be resolved with strict backwards compatibility.

@mihails-strasuns-sociomantic
Copy link
Contributor

v4.x.x HEAD is now tested with upstream compiler, thus this can be closed as done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants