Fix issue 19278 - extern(C++, "name") doesn't accept expressions #8787

TurkeyMan · 2018-10-02T06:47:08Z

Accept an expression, attempt to resolve in semantic.
Ambiguity with scoped namespace identifier can be defeated with parens.

dlang-bot · 2018-10-02T06:47:09Z

Thanks for your pull request and interest in making D better, @TurkeyMan! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	19278	enhancement	extern(C++, "name") doesn't accept expressions

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#8787"

TurkeyMan · 2018-10-02T06:48:20Z

src/dmd/dsymbolsem.d

+                return;
+            }
+            ns.ident = Identifier.idPool(ident);
+        }


Can I get guidance on this block? I'm not sure where/how it should be placed inside this function... and is there anything that should be done after the late-assignment of the symbol name that might have been missed prior?

Please spilt that loop into the else case followed by a loop over the rest of the namespaces.

So, the AST at the start looks like

NSpace foo::bar::baz +- Dsymbols members

After the end of the above it should look like (modulo s.addMember(sc, sc.scopesym))

NSpace foo +-Dsymbols members +- NSpace bar +- NSpace baz

and you want it to end up looking like

NSpace foo +- NSpace bar +- NSpace baz +- Dsymbols members

In which case instead of that I'd validate all the namespace identifiers first, and then pull the ol' switcharoo:

Enclosing Scope (e.g. module) +-NSpace foo::bar::baz +- Dsymbols members

split, do not insert into enclosing scope ->

NSpace bar::baz (parentless) Enclosing Scope +- NSpace foo +- Dsymbols members

expand ->

NSpace bar (parentless) +- NSpace baz Enclosing Scope +- NSpace foo +- Dsymbols members

rotate reassign idents ->

NSpace foo (parentless) +- NSpace bar Enclosing Scope +- NSpace baz +- Dsymbols members

pull the switcharoo (reparenting insert) ->

Enclosing Scope +- NSpace foo +- NSpace bar +-NSpace baz +- Dsymbols members

This means that the original namespace keeps all the members and the only reference to the original (because its a tree), from the enclosing scope, is overwritten. then you can addMember the intervening namespaces.

Right... see, logically it makes sense... but i have no idea what the code that implements that might look like.
Those functions addMember, setScope, importAll, and what they mean relative to the 'current' scope (which is a concept that still isn't clear), and when they should all be called... and why. I can't imagine how to implement that sequence of operations with the primitives at hand.

Not sure, try asking on slack.

Please feel free to submit a patch if you think you know how to implement this logic ;)

WalterBright · 2018-10-02T08:18:22Z

Does this mean #8781 is abandoned? Should it be Phantom Zoned, or just closed?

TurkeyMan · 2018-10-02T08:29:17Z

No, I think that is complementary; being able to compose strings this way is a lot less useful if you can't compose a string with :: in it.

I'll merge them if it's accepted, but I wanted to review the detail in this one as a self-contained patch, since it's more complex. I didn't want to muddy the waters.
If this approach is accepted, I think I'll also route the string-literal path through this same code to unify the surface-area of the feature, and move the :: handling into semantic.

TurkeyMan · 2018-10-02T08:32:13Z

I think strings with "::" in them feels more uniform with extern(C++, ns.ns2.ns3) which is accepted via the scoped path. extern(C++, "ns::ns2::ns3") feels more like the proper complement, and it also lends to simpler composition expressions without involving tuples and .expand (which are in phobos, not druntime!).

TurkeyMan · 2018-10-02T08:33:54Z

If you can comment on the work-block; is it in the right location? Ordered correctly relative to the other work? Does it need any further semantic work?

jacob-carlborg · 2018-10-02T11:47:27Z

I think strings with "::" in them feels more uniform with extern(C++, ns.ns2.ns3) which is accepted via the scoped path. extern(C++, "ns::ns2::ns3") feels more like the proper complelemt, and it also lends to simpler composition expressions without involving tuples and .expand (which are in phobos, not druntime!).

The reason I suggested several arguments/tuple was to avoid the need to decide between ”foo::bar” and ”foo.bar”. It also not necessary for the compiler to parse the string, which in my opinion feels a bit ugly.

But I didn’t think of a need to compose the string literal.

thewilsonator · 2018-10-02T12:16:52Z

src/dmd/parse.d

+                        {
+                            nextToken();
+                            cppMangleOnly = true;
+                            identExp = parseCondExp();


My parser knowledge is a bit limited, but shouldn't this be an assignExp?

How would an assign expression be valid here? I think ?: is the highest level grammar applicable (and likely to occur) in this context...

TurkeyMan · 2018-10-02T17:14:42Z

But I didn’t think of a need to compose the string literal.

Fair enough. That was one of primary motives for making it a string ;)

So, I'm feeling '::' because that's a cut & paste of the C++ namespace that it's linking against, and it also leaves no room for confusion that it might be a D identifier.

It also not necessary for the compiler to parse the string, which in my opinion feels a bit ugly.

It's one small loop; fortunately D is astonishingly good at string processing! Splitting on '::' is not trouble.

jacob-carlborg · 2018-10-02T18:25:53Z

It's one small loop; fortunately D is astonishingly good at string processing! Splitting on '::' is not trouble.

I know it's not technically difficult, it just feels wrong.

WalterBright · 2018-10-02T22:03:56Z

I know it's not technically difficult, it just feels wrong.

I agree, it's hackish to have a "language within a language".

TurkeyMan · 2018-10-02T22:28:32Z

Ummm. C++ support is definitely a language within a language to some quite ambitious extent... this expression describes a C++ namespace for mangling purposes, and that should be a string such that it may be any valid C++ namespace. The only grammatical way to express a C++ namespace in D is within a string, and as a bonus by doing so, we're not leaking any possibility for misconception that the thing is anything other than a C++ namespace to the user... I think it's perfect.
If it used '.' instead of '::', or not-a-string, it starts to look like D syntax, and it's definitely not D syntax, and shouldn't be confuse-able as such.
I really think this design has a massive clarity advantage. Users can't misunderstand what they're looking at.

wilzbach · 2018-10-02T22:50:29Z

Maybe I didn't catch the full discussion, but why is the parenthesis required?

TurkeyMan · 2018-10-02T22:57:12Z

It's not required... any expression that doesn't start with an identifier is unambiguous... it's just that i figured requiring parens was a little more explicit and less susceptible to surprise.

wilzbach · 2018-10-02T23:05:03Z

It just looks/looked like an arbitrary limitation to me and if I would look at code that uses it I would probably wonder why the author used the unnecessary parenthesis.
It just reminded me about a silliness in JavaScript's destructing rules where the parentheses are required in the last line:

{a: b} = {a: 42};
// b is 42
let obj = {};
({a: obj.b} = {a: 12}); 
// obj now has a member b with the value 12

Anyhow, AFAICT that's not the main concern here.

TurkeyMan · 2018-10-02T23:08:37Z

There's a potential ambiguity between the named scope and a string expression.
I went with parens precisely because of precedent of the kind that you just described ;)
C++ also has those same use-parens-to-break-ambiguity tricks all over the place, among others. It's pretty common, which is why I felt 'okay' about it (compared to other options).
The requirement for parens can be relaxed if named scopes are lifted into a distinct feature in the future (which I think would be ideal; named scopes should be a feature that stands in its own right if it should exist).

jacob-carlborg · 2018-10-03T08:30:30Z

If it used '.' instead of '::', or not-a-string, it starts to look like D syntax, and it's definitely not D syntax, and shouldn't be confuse-able as such.

It is D syntax. I'm mean, it is D after all.

TurkeyMan · 2018-10-05T06:35:19Z

src/dmd/dsymbolsem.d

+                    parentns.members.push(childns);
+
+                    // not sure how to fix up...
+                    // there is symtab, _scope is all wrong... etc.


I'm not sure what to write here to fix up the parent/scope pointers...

TurkeyMan · 2018-10-05T08:36:43Z

No... it's a string, that contains a C++ identifier to be used for C++ mangling. The D syntax is that it accepts a string of a valid C++ identifier for mangling. It's not valid D identifiers, and it shouldn't be possible to confuse it as such.

thewilsonator · 2018-10-05T10:25:53Z

test/fail_compilation/cppmangle.d

-fail_compilation/cppmangle.d(10): Error: invalid zero length C++ namespace
-fail_compilation/cppmangle.d(14): Error: expected valid identifer for C++ namespace but got `0num`
-fail_compilation/cppmangle.d(18): Error: string expected following `,` for C++ namespace, not `)`
+fail_compilation/cppmangle.d(10): Error: string expected following `,` for C++ namespace, not `)`


You need to update the line number and message, from the autotester:

expected: ---- fail_compilation/cppmangle.d(10): Error: string expected following `,` for C++ namespace, not `)` ---- actual: ---- fail_compilation/cppmangle.d(8): Error: found `,` when expecting `)` fail_compilation/cppmangle.d(8): Error: declaration expected, not `)` ----

so this would become

fail_compilation/cppmangle.d(9): Error: found `,` when expecting `)` fail_compilation/cppmangle.d(9): Error: declaration expected, not `)`

TurkeyMan · 2018-10-26T08:18:44Z

Walter accepted the previous PR before it was finished baking. I'm finishing it up.
Parens aren't required, they're only necessary in the case you need to disambiguate (ie, very rarely). As raw strings, this is exactly what was there's before.

jacob-carlborg · 2018-10-26T08:38:24Z

Can we deprecate extern(C++, Namespace) and only support extern(C++, "Namespace")? Then there would be a conflict. A workaround can be used until the feature is removed.

Geod24 · 2018-10-26T08:41:24Z

You lose syntax highlighting

q{} blocks are syntax highlighted. Even Github recognizes it.

you lose ability to debug

Since they are just extern definitions, the debugger doesn't care.

you lose autocomplete, you lose go-to definition

True for client code, but truly manageable for the most common std types.

I'm not attempting to block this PR, but I would prefer us to gather experience before we put in changes to the syntax that don't enable any new usage.

jacob-carlborg · 2018-10-26T08:52:23Z

q{} blocks are syntax highlighted. Even Github recognizes it.

Technically GitHub does not recognize q{} blocks. It just happens to work because it doesn't recognize the q{} and just continues to do the syntax highlighting inside q{}.

TurkeyMan · 2018-10-26T17:16:25Z

You lose syntax highlighting

q{} blocks are syntax highlighted. Even Github recognizes it.

Only one of my editors does it right.

you lose ability to debug

Since they are just extern definitions, the debugger doesn't care.

There's almost no extern in the STL bindings; that was a failed experiment.

you lose autocomplete, you lose go-to definition

True for client code, but truly manageable for the most common std types.

Client code is what matters!

And I'd argue that STL is one of the worst offenders; nobody knows what members they have. In C++, I would say the STL is my single greatest use for auto-complete assistance. Nobody knows how to work that thing, it's all weirdly named functions and iterators and rubbish.

I'm not attempting to block this PR, but I would prefer us to gather experience before we put in changes to the syntax that don't enable any new usage.

This enables one of the key cases that it was intended for, and the only case I'm aware of that's in development for druntime. I'm just finishing the patch that was already merged, I see this is a completion of that task. I never imagined for a second that string expressions wouldn't be Expressionss.

There's definitely a future where named scopes are granted their own syntax, and the disambiguation requirement ceases to exist. In 99% of cases, just string literals are fine, and no disambiguation is required. It's really just the STL, and one other private project that I've encountered where an expression is necessary. It's not pervasive.

12345swordy · 2018-10-26T18:41:03Z

Can we deprecate extern(C++, Namespace) and only support extern(C++, "Namespace")? Then there would be a conflict. A workaround can be used until the feature is removed.

We going to need a DIP for that to happen. I can write up a quick draft on it.

TurkeyMan · 2018-10-26T19:02:22Z

Can we deprecate extern(C++, Namespace) and only support extern(C++, "Namespace")? Then there would be a conflict. A workaround can be used until the feature is removed.

We going to need a DIP for that to happen. I can write up a quick draft on it.

Sure. But I don't think it's important right now. I doubt anybody in the world is waiting on that.
But I'd really like to be un-blocked here...

12345swordy · 2018-10-26T20:14:35Z

Ping @WalterBright
Is this good to go now?

rainers · 2018-10-26T19:56:49Z

src/dmd/dsymbolsem.d

+            TupleExp tup = name ? null : resolved.toTupleExp();
+            if (!tup && !name)
+            {
+                ns.error("expected string expression for namespace name, got `%s`", ns.identExp.toChars());


... or tuple

The tuple must be strings. I considered this message for a while, but felt that sticking with this message was less confusing. The message is: this expects strings. To explain that it must be a string or a tuple of strings felt too long and potentially more confusing that this message. YMMV?

Like, the precision makes sense to a compiler author, but a user just needs to know that they need to supply strings.

So maybe use the plural "string expressions"?

To explain that it must be a string or a tuple of strings felt too long and potentially more confusing that this message

In theory it could be an array of strings instead of a tuple, which is not the same thing.

rainers · 2018-10-26T19:58:38Z

src/dmd/dsymbolsem.d

+                const(char)[] ident = name.toStringz();
+                if (ident.length == 0 || !Identifier.isValidIdentifier(ident))
+                {
+                    ns.error("expected valid identifer for C++ namespace but got `%.*s`", cast(int)ident.length, ident.ptr);


As can be seen in the failure tests, this yields "namespace __anonymous" which is confusing. Better just call the global error function to omit it.

How do I do that? Just error()? That's fine to do here?

Yes, error(ns.Loc, ...) should work.

rainers · 2018-10-26T20:01:10Z

src/dmd/dsymbolsem.d

+            // resolve the namespace identifier
+            sc = sc.startCTFE();
+            Expression resolved = ns.identExp.expressionSemantic(sc);
+            resolved = resolved.expressionSemantic(sc);


Is it really necessary to call expressionSemantic again here?

Oh, good catch! This must be a cut&paste fail.

rainers · 2018-10-26T20:04:43Z

src/dmd/dsymbolsem.d

+                        return;
+                    }
+                    const(char)[] ident = name.toStringz();
+                    if (ident.length == 0 || !Identifier.isValidIdentifier(ident))


D allows more characters in identifiers as C++, but that's probably ok.

Yeah I thought about that, but I felt that writing a new validation function that's used in exactly one place was overkill.
You're limited to C++ subset in practice anyway, because the C++ code is restricted by its rules.

rainers · 2018-10-26T20:12:46Z

src/dmd/dsymbolsem.d

+                    if (i > 0)
+                    {
+                        // insert the new namespace
+                        Nspace childns = new Nspace(parentns.loc, Identifier.idPool(ident), null, parentns.members, parentns.mangleOnly);


parentns.loc and parentns.mangleOnly are constant here, maybe that's more obvious by using ns instead of parentns.

rainers · 2018-10-26T20:20:20Z

src/dmd/dsymbolsem.d

@@ -2841,6 +2902,11 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
            sc.parent = ns;
            foreach (s; *ns.members)
            {
+                if (repopulateMembers)


Is this necessary? The scopesym should not change by inserting mangleOnly namespaces and it seems dangerous to insert symbols into the symbol table again. Maybe calling s.setScope(sc) is enough.

I didn't understand this code as well as I'd like. I can try your suggestion?
I'd love an expert to fix these couple of lines, they made me a touch nervous, but the unit tests all passed, so I had moderate confidence I worked it out right...

rainers · 2018-10-26T20:34:42Z

src/dmd/parse.d

                            }
                        }
-                        else
+                        else // extern(C++, [StringExp])


The brackets don't match the comment before the function.

Mmm, WIP comment...

You might also want to add the extern(C++,struct) to the comment.

rainers · 2018-10-26T20:35:54Z

test/fail_compilation/cppmangle.d

@@ -15,9 +16,10 @@ extern(C++, "0num")
 {
 }

-extern(C++, "std", )


Is this no longer an error or can the test be kept?

It's not a meaningful test anymore. This was previously a specific case that the parser implemented manual handling for (hence the tests). Since I changed the parse code to use ParseCondExp(), this testing for no expression/fail commas is handled and tested elsewhere.
These explicit tests no longer prove anything.

Ok, but it might be interesting to verify whether the tuple is allowed to have a trailing comma or not.

Agreed with Rainers here, can you add a trailing comma somewhere in your cppmangle tests ?

Are you sure? The fail is in the parser, not semantic... so it can't be in this file with the rest of these semantic errors. I need to add a whole new file to the tester just to test that one thing, which should be tested by whatever already tests parseCondExp()...
I can do it, I just don't know why, and I have to create a whole new file just for that one test.

Hum, right, because you don't actually pass a tuple, so the point in moot. I guess when we fix this code (I see this state as temporary), we'll fix it. At the moment basically any usage of the feature needs to be surrounded by parenthesis.

There are expressions that aren't ambiguous with an identifier.
extern(C++, "hello" ~ "world") doesn't need parens for instance.

rainers · 2018-10-26T20:38:28Z

test/fail_compilation/ice17074.d

-fail_compilation/ice17074.d(29): Error: declaration expected, not `)`
---
-*/
-extern(C++, ...) void ice_token();


Why are these tests change or removed?

Same as above; they were cases that were manually handled, I replaced with ParseCondExp() and evaluate with CTFE; the testing for invalid expressions is comprehensively tested elsewhere.
I see no reason to increasing test time for arbitrary invalid expressions which are already very well tested.

Ok, shouldn't have an effect on coverage.

rainers · 2018-10-27T06:20:20Z

src/dmd/astbase.d

+         */
+        Expression identExp;
+
+        extern (D) this(const ref Loc loc, Identifier ident, Expression identExp, Dsymbols* members, bool mangleOnly)


Having both ident and identExp is redundant with mangleOnly. I wonder if it would be better to always parse expressions and determine in semantic whether there is a single fully qualified name. That would lift a couple of restrictions with respect to what expressions are detected correctly, but it's also a larger change to the existing case, so maybe left for a separate refactoring.

Geod24 · 2018-11-02T05:25:55Z

Also needs a rebase

Geod24

Could you:

rebase
squash
provide a more sensible title to the resulting commit, e.g. Fix issue 19278 - extern(C++, "name") doesn't accept expressions
Ping me when it's done

Tested it locally, it LGTM, and if it's the last thing needed for proper stdlib support, we might as well get it in at the beginning of the new release.

Geod24 · 2018-11-02T05:50:24Z

test/compilable/cppmangle.d

+{
+    void test19278_4();
+}
+version(Win64)


Can I haz POSIX tests ?

Geod24 · 2018-11-02T05:53:51Z

test/fail_compilation/cppmangle.d

@@ -15,9 +16,10 @@ extern(C++, "0num")
 {
 }

-extern(C++, "std", )


Agreed with Rainers here, can you add a trailing comma somewhere in your cppmangle tests ?

TurkeyMan · 2018-11-02T06:56:24Z

Wtf is with semaphore, and why doesn't it ever work? The error is unhelpful.

thewilsonator · 2018-11-02T06:57:25Z

Ignore it, its unrelated.

TurkeyMan · 2018-11-02T07:01:26Z

Are we done here?

thewilsonator · 2018-11-02T07:04:42Z

Just squash once more and I think thats it.

TurkeyMan commented Oct 2, 2018

View reviewed changes

TurkeyMan force-pushed the namespace_expr branch from 01a6512 to 0529127 Compare October 2, 2018 07:30

TurkeyMan requested review from ibuclaw and dnadlinger as code owners October 2, 2018 07:30

TurkeyMan force-pushed the namespace_expr branch from 0529127 to 96828a7 Compare October 2, 2018 07:37

TurkeyMan requested a review from RazvanN7 as a code owner October 2, 2018 07:37

TurkeyMan force-pushed the namespace_expr branch 2 times, most recently from 9ced37c to 45d731d Compare October 2, 2018 08:12

TurkeyMan force-pushed the namespace_expr branch from 45d731d to 583c9f3 Compare October 2, 2018 08:24

thewilsonator reviewed Oct 2, 2018

View reviewed changes

TurkeyMan commented Oct 5, 2018

View reviewed changes

TurkeyMan force-pushed the namespace_expr branch 2 times, most recently from 96e3f5b to 8aa7fcb Compare October 5, 2018 08:34

thewilsonator reviewed Oct 5, 2018

View reviewed changes

TurkeyMan mentioned this pull request Oct 26, 2018

Added std::array<> dlang/druntime#2286

Merged

rainers reviewed Oct 26, 2018

View reviewed changes

rainers reviewed Oct 27, 2018

View reviewed changes

Geod24 approved these changes Nov 2, 2018

View reviewed changes

Fix issue 19278 - extern(C++, "name") doesn't accept expressions

b596c4f

TurkeyMan force-pushed the namespace_expr branch from ee61f98 to 7e07f60 Compare November 2, 2018 06:29

dlang-bot added the Enhancement label Nov 2, 2018

Tests

db36d97

TurkeyMan force-pushed the namespace_expr branch from 7e07f60 to db36d97 Compare November 2, 2018 06:37

rainers approved these changes Nov 2, 2018

View reviewed changes

thewilsonator approved these changes Nov 2, 2018

View reviewed changes

thewilsonator changed the title ~~Attempt to support extern(C++, Expression)~~ Fix issue 19278 - extern(C++, "name") doesn't accept expressions Nov 2, 2018

thewilsonator added the 72h no objection -> merge The PR will be merged if there are no objections raised. label Nov 2, 2018

Geod24 added auto-merge and removed 72h no objection -> merge The PR will be merged if there are no objections raised. labels Nov 5, 2018

dlang-bot merged commit 1965a43 into dlang:master Nov 5, 2018

TurkeyMan deleted the namespace_expr branch November 6, 2018 06:40

Fix issue 19278 - extern(C++, "name") doesn't accept expressions #8787

Fix issue 19278 - extern(C++, "name") doesn't accept expressions #8787

Conversation

TurkeyMan commented Oct 2, 2018 • edited Loading

dlang-bot commented Oct 2, 2018 • edited Loading

Bugzilla references

Testing this PR locally

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WalterBright commented Oct 2, 2018

TurkeyMan commented Oct 2, 2018

TurkeyMan commented Oct 2, 2018 • edited Loading

TurkeyMan commented Oct 2, 2018

jacob-carlborg commented Oct 2, 2018

Choose a reason for hiding this comment

TurkeyMan Oct 2, 2018 • edited Loading

Choose a reason for hiding this comment

TurkeyMan commented Oct 2, 2018 • edited Loading

jacob-carlborg commented Oct 2, 2018

WalterBright commented Oct 2, 2018

TurkeyMan commented Oct 2, 2018

wilzbach commented Oct 2, 2018

TurkeyMan commented Oct 2, 2018

wilzbach commented Oct 2, 2018

TurkeyMan commented Oct 2, 2018 • edited Loading

jacob-carlborg commented Oct 3, 2018

Choose a reason for hiding this comment

TurkeyMan commented Oct 5, 2018

Choose a reason for hiding this comment

TurkeyMan commented Oct 26, 2018

jacob-carlborg commented Oct 26, 2018

Geod24 commented Oct 26, 2018

jacob-carlborg commented Oct 26, 2018

TurkeyMan commented Oct 26, 2018

12345swordy commented Oct 26, 2018

TurkeyMan commented Oct 26, 2018

12345swordy commented Oct 26, 2018

Choose a reason for hiding this comment

TurkeyMan Oct 26, 2018 • edited Loading

Choose a reason for hiding this comment

TurkeyMan Oct 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TurkeyMan Oct 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TurkeyMan Oct 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Geod24 commented Nov 2, 2018

Geod24 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TurkeyMan commented Oct 2, 2018 •

edited

Loading

dlang-bot commented Oct 2, 2018 •

edited

Loading

TurkeyMan commented Oct 2, 2018 •

edited

Loading

TurkeyMan Oct 2, 2018 •

edited

Loading

TurkeyMan commented Oct 2, 2018 •

edited

Loading

TurkeyMan commented Oct 2, 2018 •

edited

Loading

TurkeyMan Oct 26, 2018 •

edited

Loading

TurkeyMan Oct 26, 2018 •

edited

Loading

TurkeyMan Oct 26, 2018 •

edited

Loading

TurkeyMan Oct 26, 2018 •

edited

Loading