Implement basic support for Objective-C methods. #4321

jacob-carlborg · 2015-01-20T20:48:19Z

Some return types does not work, those are: long double, _Complex long double and structs that are too large to fit in registers. The runtime part of this pull is in dlang/druntime#1111.

The below text is in the commit message as well. I don't know if it's too long for a commit message or if there's a better place to document this.

Basic support for classes, interfaces and instance methods.
This is implemented by adding a new linkage attribute, Objective-C,
and a compiler recognized UDA, @selector. The linkage attribute is
to be used on a class or interface. The UDA is attached to a method.

The linkage attribute tells the compiler that the class should use the
name mangling that matches the one used by Objective-C
(same as C, no mangling) and that all methods in the class
should use the Objective-C way of calling methods, see below.
The calling convention for Objective-C methods and functions is the
same as for C.

The selector UDA tells the compiler what Objective-C selector the
method should have. The selector is used in the Objective-C runtime
to find the implementation of a given method.

An Objective-C method call is implemented by making a regular C call
to the objc_msgSend function in the Objective-C runtime.
The signature of objc_msgSend looks something like this:

id objc_msgSend(id self, SEL op, ...);

The first parameter is the object (this/self pointer)
The second parameter is the selector attached to the method
The last parameter is for all the arguments that the
implementation expects

The call to objc_msgSend should not be performed as a variadic
call but instead as if it had the same signature as the method
that should be called but with the two additional parameter,
self and op, added first. The implementation of objc_msgSend
will jump to the method instead of calling it.

Because of the above, multiple versions exist of objc_msgSend.
Depending on the return type of the method that is called the correct
version need to be used. This depends on the ABI. This is a list of
functions and for which types they're used on OS X 64bit:

objc_msgSend_stret - Used for structs too large to be returned in
registries
objc_msgSend_fpret - Used for long double
objc_msgSend_fp2ret - Used for _Complex long double
objc_msgSend - Used for everything else

PetarKirov · 2015-01-21T11:30:33Z

src/objc_symbols_common.c

+    stringtable->_init();
+
+    return stringtable;
+}


You can remove this function and call StringTable::reset(..) instead, which does the same thing.

Then I don't have to new the string table?

Yes, AFAIU. The default constructor doesn't do anything.

Ok, cool, I'll update that.

PetarKirov · 2015-01-21T13:31:34Z

I think the autotester is currently failing on Windows, because you need to include objc_stubs.c to win32.mak, similar to what you have done for posix.mak.

schuetzm · 2015-01-21T15:45:48Z

src/idgen.c

@@ -353,6 +354,9 @@ Msgtable msgtable[] =
    { "basic_ostream" },
    { "basic_iostream" },
    { "char_traits" },
+
+    // Compiler recognized UDA's
+    { "udaSelector", "core.attribute.selector" },


Would it be better not to detect the special UDA by name, but have the user import e.g. core.objc, which contains a symbol selector, and let the compiler detect this special symbol? Then it wouldn't make existing @selector UDAs invalid, at the cost of needing an explicit import.

That is how it works. See https://github.com/jacob-carlborg/dmd/blob/dobjc_instance_methods/src/objc_func_selector.c#L48

jacob-carlborg · 2015-01-21T15:57:13Z

I think the autotester is currently failing on Windows, because you need to include objc_stubs.c to win32.mak, similar to what you have done for posix.mak.

Ok, I see. I'll add it to win32.mak.

PetarKirov · 2015-01-21T16:13:09Z

src/objc_func_selector.c

+                    StringExp *se = (*literal->elements)[0]->toStringExp();
+                    assert(se);
+
+                    self->objc.selector = ObjcSelector::lookup((const char *)se->toUTF8(sc)->string);


ObjcSelector::lookup(..) is a stub, so you are just delaying the error, afaics.

Yes, that's a problem.

jacob-carlborg · 2015-01-23T20:42:24Z

@ZombineDev a couple of updates:

Moved objc_FuncDeclaration_semantic_setSelector to objc_func.c and stubbed it on platforms where D_OBJC is disabled
Removed the stub for initStringTable
Updated the Windows makefile

I couldn't use stringtable->reset() cause it's a member function and the original initStringTable creates the actual object.

@WalterBright @yebblies @9rnsr please review.

yebblies · 2015-01-28T16:14:37Z

There are some parts of this that don't match the usual dmd style - eg ref parameters, the style of asserts, calling parameters self instead of the usual fd.

There also seem to be a lot of small new files. Is this because later pull requests are going to flesh them out a lot more?

Is ObjcSelectorBuilder really necessary? Can't you just use OutBuffer?

yebblies · 2015-01-28T16:15:29Z

src/objc_func.c

+                StructLiteralExp *literal = (StructLiteralExp *)e;
+                assert(literal->sd);
+
+                if (strcmp(Id::udaSelector->string, literal->sd->toPrettyChars()) == 0)


if(Id::udaSelector == literal->sd->ident) ?

The whole point of Id is to avoid doing string comparisons.

I need the fully qualified name. literal->sd->ident->string will only contain "selector".

Just throwing something random out there.
Maybe have Id::udaSelector be = "selector" and then:

if (literal->sd->ident == Id::udaSelector && (mod = literal->sd->parent->isModule()) != NULL && mod->md->packages.dim == 2 && (*mod->md->packages)[0].ident == Id::Pool("core") && (*mod->md->packages)[1].ident == Id::Pool("attribute")) { ... }

Will have to double check if that is even possible (the Package may only hold the string, not the Identifier)

I know it is a mouthful, but should be faster than toPrettyChars and have the bonus of not wasting memory.

Hehe, that is a lot of comparisons. @yebblies what do you think?

@jacob-carlborg - there could always be a new boolean method isCoreAttributeModule() or something of the sort.

Would shorten it down to:

&& (mod = literal->sd->parent->isModule()) != NULL && mod->isCoreAttributeModule())

Another option is to move this matching code to StructDeclaration::StructDeclaration, and store a reference in a static variable as is done for moduleinfo. (and lots of similar stuff for ClassDeclaration). This has the benefit of completely avoiding the check when core.attribute isn't imported.

If you did create a isCoreModule("attribute") it could be reused in other places, like detecting intrinsics.

I can do that. Wouldn't isCoreModule require doing a string comparison since the module name would be an argument?

Yeah, isCoreModule(Id::attribute) would be better.

You could still use Id::Pool(str) on the string for pointer comparisons - but yes, having the idents pre-cached and ready helps too.

You could still use Id::Pool(str) on the string for pointer comparisons

Well not really, because Identifier::idPool would have to do a string comparison.

jacob-carlborg · 2015-01-28T17:14:33Z

There are some parts of this that don't match the usual dmd style - eg ref parameters, the style of asserts, calling parameters self instead of the usual fd.

All functions prefixed with objc_ were originally inlined in some method of an existing class. Walter asked me to separate out that code in to separate files. All parameters named self is what would have been this if the code was inline in a method. Preferably I would have liked to used this as the parameter name but that's not possible. That also means at the call site of these function, the self parameter should always be this. Some other languages uses the name self instead of this, that's why I picked the name.

The reason for the ref parameters is that I wanted to avoid parameters with two pointers. But perhaps that's preferred. I also think it clearly shows which parameters will be modified. Also, this code was original inlined in some other method, using references required far less changes when it was moved to free functions.

I'm not sure that you mean with the asserts. How should I/should I not use them?

There also seem to be a lot of small new files. Is this because later pull requests are going to flesh them out a lot more?

Yes. Also, one of the GDC developers asked me to separate out the front end code from the glue code. For every (most) objc_ file there's a correspond already existing file without the prefix, where the functions are called.

Is ObjcSelectorBuilder really necessary? Can't you just use OutBuffer?

Hmm. ObjcSelectorBuilder keeps track of how many colons there are in a selector, which is used later to validate against the number of parameters the method has, which the selector is attached to.

michelf · 2015-01-28T20:08:07Z

@jacob-carlborg Is ObjcSelectorBuilder still in use somewhere? It was used initially to build the selector from the special syntax where each part was produced as an identifier by the lexer, but that syntax was replaced with a UDA taking a string parameter. (Perhaps for the auto-generated selectors which don't seem to be part of this pull request, but I'm not sure.)

yebblies · 2015-01-29T04:42:53Z

All functions prefixed with objc_ were originally inlined in some method of an existing class. Walter asked me to separate out that code in to separate files.

Yes, but including the name of the caller in the function is not super useful.

All parameters named self is what would have been this if the code was inline in a method. Preferably I would have liked to used this as the parameter name but that's not possible. That also means at the call site of these function, the self parameter should always be this. Some other languages uses the name self instead of this, that's why I picked the name.

There is a huge amount of code already in dmd that has been moved out of member functions (eg into visitors) and none of it uses self. IMO using self is referencing the history of the code and is not useful. ie The fact that it used to be in a member function is not important to somebody reading the code.

The reason for the ref parameters is that I wanted to avoid parameters with two pointers. But perhaps that's preferred. I also think it clearly shows which parameters will be modified. Also, this code was original inlined in some other method, using references required far less changes when it was moved to free functions.

There is no need to avoid parameters with two pointers. Another option is to return the modified elem, it looks like these functions currently return void.

I'm not sure that you mean with the asserts. How should I/should I not use them?

Current code uses printf and assert(0), you've used assert(false && msg). It's a new style for dmd and potentially problematic because of issue 4733.

Yes. Also, one of the GDC developers asked me to separate out the front end code from the glue code. For every (most) objc_ file there's a correspond already existing file without the prefix, where the functions are called.

Keeping the frontend/glue stuff split is important, but maintaining a 1-1 mapping from objc_ files to the original is not. If some of the files can be logically combined they should be, within reason. eg objc_mars.c probably shouldn't exist, it's too small. Unless later pull requests are going to make it worthwhile. Again, the history of where these functions used to be doesn't matter.

Hmm. ObjcSelectorBuilder keeps track of how many colons there are in a selector, which is used later to validate against the number of parameters the method has, which the selector is attached to.

Another option is using Identifiers and appending idents to it. Or just scan the buffer and count the colons.

jacob-carlborg · 2015-01-29T07:45:46Z

@yebblies @michelf it doesn't look like ObjcSelectorBuilder is used anymore. I thought it was, I can remove it.

jacob-carlborg · 2015-01-29T08:03:28Z

Yes, but including the name of the caller in the function is not super useful.

It's kind of hard to come up with names for all these small functions. The final version will have 93 of these free functions.

There is a huge amount of code already in dmd that has been moved out of member functions (eg into visitors) and none of it uses self. IMO using self is referencing the history of the code and is not useful. ie The fact that it used to be in a member function is not important to somebody reading the code.

Ok.

There is no need to avoid parameters with two pointers. Another option is to return the modified elem, it looks like these functions currently return void.

Ok, I'll change to two pointers. Other functions will modify multiple parameters, so for those it won't work. If I mix the styles I'm pretty certain someone will complain about that.

Current code uses printf and assert(0), you've used assert(false && msg). It's a new style for dmd and potentially problematic because of issue 4733.

I can change that. I'm not sure what issue 4733 has to do with this, or are you thinking about ddmd?

Keeping the frontend/glue stuff split is important, but maintaining a 1-1 mapping from objc_ files to the original is not. If some of the files can be logically combined they should be, within reason. eg objc_mars.c probably shouldn't exist, it's too small. Unless later pull requests are going to make it worthwhile.

Could you please tell how I should organize the code, it would be much easier then me guessing. The total line count of all objc_ files in the final implementation is around 4000.

Again, the history of where these functions used to be doesn't matter.

I'm just explaining why the code looks like it does.

Another option is using Identifiers and appending idents to it. Or just scan the buffer and count the colons.

Seems like it's not used anymore. I don't even know if that validation is necessary.

ibuclaw · 2015-01-29T09:24:22Z

src/declaration.h

@@ -534,6 +535,7 @@ class FuncDeclaration : public Declaration
                                        // scopes from having the same name
    VarDeclaration *vthis;              // 'this' parameter (member and nested)
    VarDeclaration *v_arguments;        // '_arguments' parameter
+    Ojbc_FuncDeclaration objc;


Ojbc_FuncDeclaration looks like a typo (should it be Objc_FuncDeclaration?)

Is the 'n' unicode or something?

No, Ojbc vs Objc.

Why didn't the 'n' show up in the email? Weird.

Yes, it's a typo.

Nearly ready, I'll probably make a PR tomorrow. I'm just not really happy with the overhead of introducing a new type.

I'll let the testing with gdc do the talking. :)

Has that branch anything to do with my pull request? Or are you just taking any opportunity to communicate 😃

No comment. :P

Nothing to do with the pull request, sorry.

yebblies · 2015-01-29T10:47:57Z

I can change that. I'm not sure what issue 4733 has to do with this, or are you thinking about ddmd?

Yes, issue 4733 + ddmd.

michelf · 2015-01-29T14:00:09Z

@jacob-carlborg

I don't even know if that validation is necessary.

Validating the number of colons isn't technically necessary, but it's a cheap way to catch some typos at compile time instead of at runtime. Think what would happen declaring memcpy with 1 argument instead of 3 and then calling it. This is the kind of mistake we're catching since it's cheap to check. Colons from the UDA string are counted by ObjcSelector::lookup(const char *s) and later validated against the parameter count in objc_FuncDeclaration_semantic_validateSelector.

jacob-carlborg · 2015-01-29T16:56:44Z

Colons from the UDA string are counted by ObjcSelector::lookup(const char *s) and later validated against the parameter count in objc_FuncDeclaration_semantic_validateSelector

Right, then it's all good 😃, ObjcSelectorBuilder can be removed.

jacob-carlborg · 2015-01-29T19:43:50Z

@yebblies about the file structure, how about this: one file for the glue code, one for the stubs and one for the rest?

yebblies · 2015-01-30T03:46:56Z

@yebblies about the file structure, how about this: one file for the glue code, one for the stubs and one for the rest?

That sounds better. 4000 lines of code isn't actually that much compared to some of dmd's files.

jacob-carlborg · 2015-07-09T08:14:14Z

Anyone know how I should modify the windows makefile to make the test pass? @yebblies is this related to magicport?

yebblies · 2015-07-09T08:43:00Z

Yes, you might need to add any new types to the lists or make sure that the new header/source files are scanned. http://wiki.dlang.org/Updating_Pull_Requests_for_DDMD

iain-buclaw-sociomantic · 2015-07-09T13:07:29Z

So, any plans to make all those horrid flat functions first class citizens?

jacob-carlborg · 2015-07-10T08:18:04Z

@yebblies Could you please verify this commit: jacob-carlborg@a003913. It's the changes I needed to make to make magicport work.

BTW, doesn't the auto tester run magicport by default on Posix systems?

yebblies · 2015-07-10T09:06:01Z

The Objc_FuncDeclaration ctor changes shouldn't be necessary IIRC, and you should be following the existing formatting in the json file.

The auto tester runs magicport and builds ddmd on all platforms.

jacob-carlborg · 2015-07-10T12:20:05Z

The Objc_FuncDeclaration ctor changes shouldn't be necessary IIRC

Before that change I got this error:

func.d(439): Error: this.Declaration is not an lvalue

It failed to rewrite the call to the super constructor. It looks like this:

this.Declaration = id;

and you should be following the existing formatting in the json file.

Yeah, I completely missed that.

The auto tester runs magicport and builds ddmd on all platforms.

It doesn't look like it does. It passes on all platforms expect for Windows, and I haven't made any changes to make it work for magicport (that are pushed). Also, when explicitly run the "ddmd" make target locally I got errors that didn't show up in the auto tester.

jacob-carlborg · 2015-07-10T12:53:44Z

@yebblies Now I noticed that this doesn't work either. I need the default constructor in C++ after I moved the initialization of objc in to the constructor of FuncDeclaration. But this will generate a default constructor in D, which is not allowed for structs.

I don't think magicport supports initializing variables in the initialization list for a constructor, because DMD never uses that, until now. Before this, DMD always used pointers to all classes.

yebblies · 2015-07-10T13:06:56Z

Magicport does support both super calls and member initialization in the ctor init list, unless this is a weird corner case I missed. Other classes in DMD do use this. Aaaaaand it looks like it only supports the 'super' re-write if there is only one expression in the init list, so that explains that. The code is in dprinter.d:253 if you feel like adding support for both together.

It looks like the posix ddmd build was stopped at some point due to makefile changes. Hopefully it still builds.

You can have a struct with a default ctor, but you will need to make sure that the default init D generates is valid (all members must be initialized to T.init, ctor does no real processing etc) and add it to the dropdefaultctor list in typenames.d. I never got around to moving that list into the json file.

jacob-carlborg · 2015-07-10T14:27:34Z

Hopefully it still builds.

At least it builds for me locally.

ibuclaw · 2015-07-10T14:48:38Z

src/mars.c

@@ -1193,6 +1194,7 @@ Language changes listed by -transition=id:\n\
        VersionCondition::addPredefinedGlobalIdent("D_NoBoundsChecks");

    VersionCondition::addPredefinedGlobalIdent("D_HardFloat");
+    objc_tryMain_dObjc();


What is tryMain and why must I be forced to call this if I wish to merge support into GDC?

That objc_tryMain_dObjc function just adds a predefined version identifier in case you want to use version (D_ObjectiveC) to detect whether Objective-C features are available at compile-time. (If your backend does not support emitting Objective-C code, then you probably shouldn't add the 'D_ObjectiveC' version identifier.)

For me it's not a question of whether or not GCC supports, it's a question of why you are poisoning my glue interface with names like this?

Jacob did this after Walter asked for Objective-C related things to be put in separate functions. Personally, I find it more confusing than useful. Those functions could have more meaning full names I guess, but all those parts weren't originally meant to be standalone functions in the first place.

If it was only me, I'd just call VersionCondition::addPredefinedGlobalIdent("D_ObjectiveC") here and delete objc_tryMain_dObjc (it was like this in the original). But with the only advice from Walter being that things must be kept segregated in functions, I'm not sure what Jacob can do to make keep this patch acceptable. Perhaps a more verbose name such as objc_addObjectiveCPredefinedGlobalIdent would make the situation better.

You can segregate things properly by implementing a namespace (via a struct). Look at port.c or target.c as an example. The header is the shared implementation between front-ends, and the source is managed by each compiler vendor.

jacob-carlborg · 2015-07-10T15:12:40Z

Anyone know what's going on with the linker error on Windows?

jacob-carlborg · 2015-07-11T10:33:52Z

@yebblies I have some trouble with magicport. I have three new files: objc.c, objc.h and objc_stubs.c. objc.h is always used, objc.c contains the implementation of objc.h when Objective-C support is enabled. objc_stubs.c contains the implementation when Objective-C support is disable. I don't know how to represent this in magicport. To elaborate, when two declarations that have the same name and both have a body magicport fails with an assertion.

yebblies · 2015-07-11T11:04:32Z

Yeah, magicport doesn't support that. One option is to handport one of the files.

jacob-carlborg · 2015-07-11T11:06:10Z

Is it possible to do some trick with the makefile to first port one of them and then the other one?

yebblies · 2015-07-11T11:39:34Z

Possible, but unlikely to be worth it. Just write a D version of objc_stubs.c by hand?

jacob-carlborg · 2015-07-11T12:18:25Z

Sure, that's no problem. Just wanted to make sure there's no easier way.

Basic support for classes, interfaces and instance methods. This is implemented by adding a new linkage attribute, `Objective-C`, and a compiler recognized UDA, `@selector`. The linkage attribute is to be used on a class or interface. The UDA is attached to a method. The linkage attribute tells the compiler that the class should use the name mangling that matches the one used by Objective-C (same as C, no mangling) and that all methods in the class should use the Objective-C way of calling methods, see below. The calling convention for Objective-C methods and functions is the same as for C. The selector UDA tells the compiler what Objective-C selector the method should have. The selector is used in the Objective-C runtime to find the implementation of a given method. An Objective-C method call is implemented by making a regular C call to the `objc_msgSend` function in the Objective-C runtime. The signature of `objc_msgSend` looks something like this: `id objc_msgSend(id self, SEL op, ...);` * The first parameter is the object (this/self pointer) * The second parameter is the selector attached to the method * The last parameter is for all the arguments that the implementation expects The call to `objc_msgSend` should not be performed as a variadic call but instead as if it had the same signature as the method that should be called but with the two additional parameter, `self` and `op`, added first. The implementation of `objc_msgSend` will jump to the method instead of calling it. Because of the above, multiple versions exist of `objc_msgSend`. Depending on the return type of the method that is called the correct version need to be used. This depends on the ABI. This is a list of functions and for which types they're used on OS X 64bit: * objc_msgSend_stret - Used for structs too large to be returned in registries * objc_msgSend_fpret - Used for `long double` * objc_msgSend_fp2ret - Used for `_Complex long double` * objc_msgSend - Used for everything else

Implement basic support for Objective-C methods.

rainers · 2015-07-12T07:39:56Z

src/objc_stubs.c

+ */
+
+#include "arraytypes.h"
+#include "class.c"


Is this deliberate? It is causing duplicate symbols when building in VS (not generating a library for the front end). Including aggregate.h and scope.h instead seems to work, too.

It should be aggregate.h

It's a mistake, I'll fix it.

jacob-carlborg mentioned this pull request Jan 20, 2015

Add UDA for specifying selector for Objective-C methods. dlang/druntime#1111

Merged

jacob-carlborg force-pushed the dobjc_instance_methods branch from c08211f to fc5605e Compare January 20, 2015 22:07

PetarKirov reviewed Jan 21, 2015
View reviewed changes

schuetzm reviewed Jan 21, 2015
View reviewed changes

PetarKirov reviewed Jan 21, 2015
View reviewed changes

jacob-carlborg force-pushed the dobjc_instance_methods branch 2 times, most recently from 83912cf to 09b3980 Compare January 23, 2015 20:36

yebblies reviewed Jan 28, 2015
View reviewed changes

ibuclaw reviewed Jan 29, 2015
View reviewed changes

jacob-carlborg force-pushed the dobjc_instance_methods branch 2 times, most recently from 0313f74 to 21f9f7b Compare July 9, 2015 07:47

jacob-carlborg force-pushed the dobjc_instance_methods branch from 21f9f7b to 25ad571 Compare July 9, 2015 20:22

jacob-carlborg force-pushed the dobjc_instance_methods branch from 25ad571 to 6e217bc Compare July 10, 2015 14:27

ibuclaw reviewed Jul 10, 2015
View reviewed changes

jacob-carlborg force-pushed the dobjc_instance_methods branch from 6e217bc to e5cc03f Compare July 10, 2015 15:14

jacob-carlborg force-pushed the dobjc_instance_methods branch from e5cc03f to 94027ed Compare July 11, 2015 19:29

michelf and others added 2 commits July 11, 2015 21:36

Add tests for the various objc_msgSend_* functions.

929b56c

jacob-carlborg force-pushed the dobjc_instance_methods branch from 94027ed to 929b56c Compare July 11, 2015 19:37

WalterBright added a commit that referenced this pull request Jul 11, 2015

Merge pull request #4321 from jacob-carlborg/dobjc_instance_methods

941808d

Implement basic support for Objective-C methods.

WalterBright merged commit 941808d into dlang:master Jul 11, 2015

rainers reviewed Jul 12, 2015
View reviewed changes

ibuclaw mentioned this pull request Jul 12, 2015

Set linkage against the wrong scopesym #4812

Merged

jacob-carlborg deleted the dobjc_instance_methods branch October 4, 2015 19:00

Implement basic support for Objective-C methods. #4321

Implement basic support for Objective-C methods. #4321

Conversation

jacob-carlborg commented Jan 20, 2015

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

PetarKirov commented Jan 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacob-carlborg commented Jan 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacob-carlborg commented Jan 23, 2015

yebblies commented Jan 28, 2015

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

jacob-carlborg commented Jan 28, 2015

michelf commented Jan 28, 2015

yebblies commented Jan 29, 2015

jacob-carlborg commented Jan 29, 2015

jacob-carlborg commented Jan 29, 2015

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

yebblies commented Jan 29, 2015

michelf commented Jan 29, 2015

jacob-carlborg commented Jan 29, 2015

jacob-carlborg commented Jan 29, 2015

yebblies commented Jan 30, 2015

jacob-carlborg commented Jul 9, 2015

yebblies commented Jul 9, 2015

iain-buclaw-sociomantic commented Jul 9, 2015

jacob-carlborg commented Jul 10, 2015

yebblies commented Jul 10, 2015

jacob-carlborg commented Jul 10, 2015

jacob-carlborg commented Jul 10, 2015

yebblies commented Jul 10, 2015

jacob-carlborg commented Jul 10, 2015

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

jacob-carlborg commented Jul 10, 2015

jacob-carlborg commented Jul 11, 2015

yebblies commented Jul 11, 2015

jacob-carlborg commented Jul 11, 2015

yebblies commented Jul 11, 2015

jacob-carlborg commented Jul 11, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment