Created a branded type for identifier-escaped strings #16915

weswigham · 2017-07-03T20:59:39Z

Then flowed it throughout the compiler, finding and fixing a handful of bugs relating to underscore-prefixed identifiers in the process. Includes a test for two (new) cases noticed - diagnostics from conflicting symbols from export *'s, and enums with underscore prefixed member emit.

I mentioned this while talking among the team last week. This can be done in place of/alongside #16868.

Additionally, this fixes #15334, #14268, #11902, and #3268. (Each should also now exist as a test case in this PR, too)

The shape of this brand is also rather unique compared to others we've used. Instead of just an intersection of a string and an object, it is that union-ed with an intersection of void and an object. This makes it wholly incompatible with a normal string (which is good, it cannot be misused on assignment or on usage), while still being comparable with a normal string via === (also good) and castable from a string.

Summary:

__String is used to represent a string whose leading underscore have been
escaped (if present) or a string representing an internal compiler symbol name,
such as "__call". To escape a string in this way, call escapeLeadingUnderscores.
To unescape such a string, call unescapeLeadingUnderscores. Strings of this kind
are used to represent symbol names and identifier text, to allow for internal symbol
names to cohabitate the same symbol table as normal members without being in conflict.
The brand on this type is structured such that it is castable and comparable with
normal strings, but a normal string cannot be used in its place and it can not
be used in place of a normal string (to prevent escaped text from leaking out via
diagnostic messages and the like). The union member enabling this is the member intersected
with void, which makes it incompatible with anything asking for a string.

Then flowed it throughout the compiler, finding and fixing a handful of bugs relating to underscore-prefixed identifiers in the process. Includes a test for two cases noticed - diagnostics from conflicting symbols from export *'s, and enum with underscore prefixed member emit.

weswigham · 2017-07-03T21:16:06Z

cc @sandersn and @andy-ms - you're both assigned to some of the issues this PR fixes.

ghost · 2017-07-05T15:01:37Z

src/compiler/binder.ts

@@ -147,7 +147,7 @@ namespace ts {
            options = opts;
            languageVersion = getEmitScriptTarget(options);
            inStrictMode = bindInStrictMode(file, opts);
-            classifiableNames = createMap<string>();
+            classifiableNames = createMap<EscapedIdentifier>() as EscapedIdentifierMap<EscapedIdentifier>;


Could use createEscapedMap as this occurs many times.

ghost · 2017-07-05T15:01:56Z

src/compiler/types.ts

@@ -3005,7 +3005,24 @@ namespace ts {
        isRestParameter?: boolean;
    }

-    export type SymbolTable = Map<Symbol>;
+    export type EscapedIdentifier = (string & { __escapedIdentifier: void }) | (void & { __escapedIdentifier: void });


What's the union for?
Also, would name this EscapedString since it's not an escaped version of Identifier.

Mmmm, the name's actually a little hard, since a plain escaped string I would expect to be a string where " has been replaced with \" to escape it (which we do! elsewhere!). I called it EscapedIdentifier at first since Identifier was the only documented place where we stored strings escaped this way - except that was by no means true. From identifiers, it leaked into symbol names (which was desirable, based on use), and from there, even string literals were (sometimes and inconsistently) escaped to be compatible with symbol names. All the
inconsistency is gone now, but the name definitely no longer makes sense. What about UnderscoreEscapedString?

Also, re: the shape:

The shape of this brand is also rather unique compared to others we've used. Instead of just an intersection of a string and an object, it is that union-ed with an intersection of void and an object. This makes it wholly incompatible with a normal string (which is good, it cannot be misused on assignment or on usage), while still being comparable with a normal string via === (also good) and castable from a string.

Could you move that to a doc comment?

UnderscoreEscapedString is a bit verbose for such a common type. EscapedName might be better.

ghost · 2017-07-05T15:06:45Z

src/compiler/binder.ts

            const name = getNameOfDeclaration(node);
            if (name) {
                if (isAmbientModule(node)) {
-                    return isGlobalScopeAugmentation(<ModuleDeclaration>node) ? "__global" : `"${(<LiteralExpression>name).text}"`;
+                    const moduleName = name.kind === SyntaxKind.Identifier ? unescapeIdentifier((<Identifier>name).text) : (<LiteralExpression>name).text;


If isAmbientModule, the name should always be a string literal.

ghost · 2017-07-05T15:07:13Z

src/compiler/binder.ts

                }
                if (name.kind === SyntaxKind.ComputedPropertyName) {
                    const nameExpression = (<ComputedPropertyName>name).expression;
                    // treat computed property names where expression is string/numeric literal as just string/numeric literal
                    if (isStringOrNumericLiteral(nameExpression)) {
-                        return nameExpression.text;
+                        return nameExpression.text && escapeIdentifier(nameExpression.text);


text shouldn't be optional on a literal, right?

ghost · 2017-07-05T15:13:57Z

src/compiler/binder.ts

@@ -1028,7 +1029,7 @@ namespace ts {
        function bindBreakOrContinueStatement(node: BreakOrContinueStatement): void {
            bind(node.label);
            if (node.label) {
-                const activeLabel = findActiveLabel(node.label.text);
+                const activeLabel = findActiveLabel(unescapeIdentifier(node.label.text));


Might be better to just always use an escaped name in an ActiveLabel.

ghost · 2017-07-05T16:07:59Z

src/services/navigateTo.ts

@@ -85,11 +85,13 @@ namespace ts.NavigateTo {

        function getTextOfIdentifierOrLiteral(node: Node) {
            if (node) {
-                if (node.kind === SyntaxKind.Identifier ||
-                    node.kind === SyntaxKind.StringLiteral ||
+                if (node.kind === SyntaxKind.Identifier) {


This pattern has occurred a few times, could use a helper function.

ghost · 2017-07-05T16:08:37Z

src/services/navigationBar.ts

@@ -580,12 +580,12 @@ namespace ts.NavigationBar {
        // Otherwise, we need to aggregate each identifier to build up the qualified name.
        const result: string[] = [];

-        result.push(moduleDeclaration.name.text);
+        result.push(moduleDeclaration.name.kind === SyntaxKind.Identifier ? unescapeIdentifier(moduleDeclaration.name.text) : moduleDeclaration.name.text);


This pattern has occurred a few times, could use a helper function.

ghost · 2017-07-05T16:09:07Z

src/services/services.ts

@@ -609,11 +609,13 @@ namespace ts {

            function getTextOfIdentifierOrLiteral(node: Node) {


Ah, here is a helper function!

ghost · 2017-07-05T16:10:15Z

src/services/services.ts

@@ -2111,7 +2113,7 @@ namespace ts {
                        node.parent.kind === SyntaxKind.ExternalModuleReference ||
                        isArgumentOfElementAccessExpression(node) ||
                        isLiteralComputedPropertyDeclarationName(node)) {
-                        setNameTable((<LiteralExpression>node).text, node);
+                        setNameTable((<LiteralExpression>node).text as EscapedIdentifier, node); // Literal expressions are escaped


parseLiteralLikeNode doesn't seem to escape?

Correct! When I started it did (as it does right now), and that leaked escaped strings to wholly too many places. This probably surfaces as a bug.

ghost · 2017-07-05T16:10:42Z

src/services/utilities.ts

@@ -1272,7 +1272,7 @@ namespace ts {
        // If this is an export or import specifier it could have been renamed using the 'as' syntax.
        // If so we want to search for whatever is under the cursor.
        if (isImportOrExportSpecifierName(location) || isStringOrNumericLiteral(location) && location.parent.kind === SyntaxKind.ComputedPropertyName) {
-            return location.text;
+            return location.kind === SyntaxKind.Identifier ? unescapeIdentifier((location as Identifier).text) : (location as LiteralLikeNode).text;


Use a helper function

ghost · 2017-07-05T16:16:34Z

Could use a string enum for special identifiers such as "__call", and include it in the EscapedIdentifier type. Would cut down on casting and help us document what things escaping is used for.
It might make more sense to only escape symbol tables, not every identifier. But that could be left until later.

weswigham · 2017-07-05T18:32:28Z

@andy-ms

Could use a string enum for special identifiers such as "__call", and include it in the EscapedIdentifier type. Would cut down on casting and help us document what things escaping is used for.

Would "default" and "export=" and "args" be included in this list? Should all the global symbols we look up for our diagnostics be included (Iterator, Iterable, etc)? Or should it just be the ones with leading underscores?

ghost · 2017-07-05T21:02:33Z

I think if it's a case where a string literal just happens to not need escaping (e.g. Iterator) it shouldn't be in the enum.

default would fall under that since the module namespace object would actually have a property named default.

export= is an unusual case, since it's a special name, not meant to be accessed by a property, but doesn't begin with underscores. (Maybe it should.) I think it should be in the enum because it's a special name and not accessible as a normal name.

Don't know about "args" since this is the first I've seen it.

weswigham · 2017-07-05T21:05:53Z

"args" is a synthetic symbol we make when checking using jsdoc. I don't think it's supposed to actually exist inside any function bodies or other lexical scopes (but it might be simulated so it exists within the type annotation).

…on casting

weswigham · 2017-07-06T17:18:47Z

@andy-ms I think I've responded to all your feedback and I've implemented the list of internal symbol names as part of the brand type declaration (which, IMO, is almost more useful as a source of documentation than anything else).

ghost · 2017-07-06T17:39:18Z

src/compiler/parser.ts

@@ -1227,7 +1226,9 @@ namespace ts {

        function parsePropertyNameWorker(allowComputedPropertyNames: boolean): PropertyName {
            if (token() === SyntaxKind.StringLiteral || token() === SyntaxKind.NumericLiteral) {
-                return <StringLiteral | NumericLiteral>parseLiteralNode(/*internName*/ true);
+                const node = <StringLiteral | NumericLiteral>parseLiteralNode();
+                internIdentifier(node.text);


internIdentifier won't have the desired effect if used as a statement; it returns the interned string, but it won't set node.text to it. I like the way it was before.

I'll change it, but according to my knowledge of strings in V8 (which, admitted, is gleaned from stackoverflow answers), reassignment of the value from the map isn't required to make the runtime intern the string.

It does this:

let identifier = identifiers.get(text); if (identifier === undefined) { identifiers.set(text, identifier = text); } return identifier;

Its primary use (now that it is no longer handling escaping things) in our code is setting up the identifiers list which is used in the emitter and declaration emitter for knowing what identifiers it needs to avoid to generate names. Secondarily, this will cause the string to be interned in V8, since it's used as a property name (or map key in ES6) - a reassignment of the version from the map should not be required (since the same string primitive that's passed in is what gets interned (thus is already assigned to the object) or is already interned if already in the map).

TBQH I feel like the function should be renamed, since interning is not its primary use. Could probably reduce the space used by it a little bit by making it a Map<true>, too.

ghost · 2017-07-06T17:39:48Z

src/compiler/parser.ts

@@ -4098,7 +4099,7 @@ namespace ts {
                        indexedAccess.argumentExpression = allowInAnd(parseExpression);
                        if (indexedAccess.argumentExpression.kind === SyntaxKind.StringLiteral || indexedAccess.argumentExpression.kind === SyntaxKind.NumericLiteral) {
                            const literal = <LiteralExpression>indexedAccess.argumentExpression;
-                            literal.text = internIdentifier(literal.text);
+                            internIdentifier(literal.text);


Same, don't use this as a statement

ghost · 2017-07-06T17:39:52Z

src/compiler/parser.ts

@@ -5624,7 +5625,8 @@ namespace ts {
                node.flags |= NodeFlags.GlobalAugmentation;
            }
            else {
-                node.name = <StringLiteral>parseLiteralNode(/*internName*/ true);
+                node.name = <StringLiteral>parseLiteralNode();
+                internIdentifier(node.name.text);


ghost · 2017-07-06T17:42:13Z

src/compiler/types.ts

@@ -3005,7 +3005,42 @@ namespace ts {
        isRestParameter?: boolean;
    }

-    export type SymbolTable = Map<Symbol>;
+    export type InternalSymbolName =


I think this would be better as a real string enum. Also, could you add the comment to the code about why (void & { __escapedIdentifier: void }) is present?

sandersn

First round of comments. Still looking.

sandersn · 2017-07-06T17:44:45Z

src/compiler/types.ts

+    export type UnderscoreEscapedString = (string & { __escapedIdentifier: void }) | (void & { __escapedIdentifier: void }) | InternalSymbolName;
+
+    /** EscapedStringMap based on ES6 Map interface. */
+    export interface UnderscoreEscapedMap<T> {


stupid question, but why can't this just be type EscapedNameMap<T> = Map<T, EscapedName> ?

We originally didn't make the key of Map generic, so it's hardcoded as string. Although maybe a better way to do this would be to define ESMap<K, V> and Map<V> = ESMap<string, V> and UnderscoreEscapedMap<V> = ESMap<UnderscoreEscapedString, V>.

sandersn · 2017-07-06T17:46:52Z

src/compiler/utilities.ts

        switch (name.kind) {
            case SyntaxKind.Identifier:
                return (<Identifier>name).text;
            case SyntaxKind.StringLiteral:
            case SyntaxKind.NumericLiteral:
-                return (<LiteralExpression>name).text;
+                return (<LiteralExpression>name).text as UnderscoreEscapedString;


why isn't text always UnderscoreEscapedString, at least for LiteralExpression?

actually, this might just be an unsafe cast. Should it be escaped?

I think you're right, this is probably unsafe, given where this is used. I'll add the escapes, rather than casts.

sandersn · 2017-07-06T17:55:32Z

src/compiler/utilities.ts

+    export function isIntrinsicJsxName(name: UnderscoreEscapedString | string) {
+        // An escaped identifier had a leading underscore prior to being escaped, which would return true
+        // The escape adds an extra underscore which does not change the result
+        const ch = (name as string).substr(0, 1);


aside: How does this code tell us that a string is an intrinsic JSX name? Its first character is lowercase? notAnInstrinsic seems like an obvious counterexample.

Actually, this is probably a fine predicate, just one that shouldn't be promoted to utilities.ts

It was already here? It's used within the checker and the jsx transform. Also, a JSX intrinsic is just any tag that starts with a lowercase letter (ie, body), indicating that its type needs to be looked up globally rather than locally, right?

sandersn · 2017-07-06T17:57:48Z

src/compiler/utilities.ts

-    export function unescapeIdentifier(identifier: string): string {
-        return identifier.length >= 3 && identifier.charCodeAt(0) === CharacterCodes._ && identifier.charCodeAt(1) === CharacterCodes._ && identifier.charCodeAt(2) === CharacterCodes._ ? identifier.substr(1) : identifier;
+    export function unescapeLeadingUnderscores(identifier: UnderscoreEscapedString): string {
+        return (identifier as string).length >= 3 && (identifier as string).charCodeAt(0) === CharacterCodes._ && (identifier as string).charCodeAt(1) === CharacterCodes._ && (identifier as string).charCodeAt(2) === CharacterCodes._ ? (identifier as string).substr(1) : identifier as string;


formatting: could you create a temp here to make the line shorter? (Maybe name it id. :)

sandersn · 2017-07-06T18:03:26Z

src/compiler/parser.ts

@@ -1203,7 +1202,7 @@ namespace ts {
                if (token() !== SyntaxKind.Identifier) {
                    node.originalKeywordKind = token();
                }
-                node.text = internIdentifier(scanner.getTokenValue());
+                node.text = escapeLeadingUnderscores(internIdentifier(scanner.getTokenValue()));


internIdentifier used to escape before interning. Now the code interns, then escapes. Is this correct?

This is correct for how we use the identifiers map the internIdentifier function uses (namely, looking for collisions in generated identifiers).

sandersn · 2017-07-06T18:17:53Z

tests/baselines/reference/doubleUnderscoreSymbolsAndEmit.errors.txt

@@ -0,0 +1,60 @@
+tests/cases/compiler/index.tsx(33,1): error TS2308: Module "./b" has already exported a member named '__foo'. Consider explicitly re-exporting to resolve the ambiguity.


I didn't expect a test named doubleUnderscoreSymbolsAndEmit to have errors. Is this expected?

Yep. I wanted the js output of the enum (which was bugged) and this diagnostic (which used to have an extra underscore) - I could split it into two tests if you'd prefer.

Maybe just change the name to omit 'Symbols'

Too late - already split this file into about 4 unique tests which test different things with double underscore prefixes. 🐱

ajafff · 2017-07-06T19:15:41Z

src/compiler/utilities.ts

@@ -3974,8 +4009,8 @@ namespace ts {
     * @param identifier The escaped identifier text.
     * @returns The unescaped identifier text.
     */
-    export function unescapeIdentifier(identifier: string): string {
-        return identifier.length >= 3 && identifier.charCodeAt(0) === CharacterCodes._ && identifier.charCodeAt(1) === CharacterCodes._ && identifier.charCodeAt(2) === CharacterCodes._ ? identifier.substr(1) : identifier;
+    export function unescapeLeadingUnderscores(identifier: UnderscoreEscapedString): string {


this should definitely be documented as breaking change

I'll add an alias to the old name with an @deprecated jsdoc.

This fact that identifier.text is no longer a string is a breaking change and should have an entry here. See #17020.

…enum usages

sandersn

Two comments:

It's really scary that you fixed tens of bugs, especially in the language service, and didn't see any changes in the tests.
I don't have a good understanding of which type to use when I'm writing new code in the checker or binder.

To fix (2), please add to the PR summary. And maybe to the wiki or to https://sandersn.github.io/manual/Typescript-compiler-implementation.html for future reference.

To fix (1), we'd need to write tons of tests for code that's now checked by the compiler anyway. So it's probably not needed.

sandersn · 2017-07-06T18:29:38Z

src/services/completions.ts

@@ -1505,7 +1506,8 @@ namespace ts.Completions {
                    // TODO(jfreeman): Account for computed property name


These comments look old and probably out of date. Can you update them? (Just remove Jason's name from the TODO if it still applies, I guess.)

I've removed Jason's name, but I think computed property names might still not be handled specially - if a computed property is returned from getNameOfDeclaration, getEscapedTextOfIdentifierOrLiteral will quietly return undefined.

sandersn · 2017-07-06T19:55:00Z

src/compiler/types.ts

+        | "export=" // Export assignment symbol
+        | "default"; // Default export symbol (technically not wholly internal, but included here for usability)
+
+    export type UnderscoreEscapedString = (string & { __escapedIdentifier: void }) | (void & { __escapedIdentifier: void }) | InternalSymbolName;


A short name is __String. But it's pretty (too?) cute.

sandersn · 2017-07-06T20:15:57Z

src/compiler/binder.ts

@@ -258,7 +259,7 @@ namespace ts {
                case SyntaxKind.ExportDeclaration:
                    return "__export";
                case SyntaxKind.ExportAssignment:
-                    return (<ExportAssignment>node).isExportEquals ? "export=" : "default";
+                    return ((<ExportAssignment>node).isExportEquals ? "export=" : "default");


looks like some extra parens got left in after their associated casts were removed.

See [here](microsoft/TypeScript#16915 (comment))

weswigham added 3 commits July 3, 2017 12:32

Correctly double underscores WRT mapped types

ce4018c

Add fourslash tests for other fixed issues

342ec75

msftclas added the cla-already-signed label Jul 3, 2017

Merge branch 'master' into escaped-string-type

770010e

weswigham changed the title ~~Created a branded type for escaped strings~~ Created a branded type for identifier-escaped strings Jul 3, 2017

ghost reviewed Jul 5, 2017

View reviewed changes

weswigham added 8 commits July 5, 2017 10:27

use function call over cast

1f55366

Update forEachEntry type accuracy

4411889

Just use escaped names for ActiveLabel

47f7fbd

Remove casts from getPropertyNameForPropertyNameNode

7f22f39

This pattern has occurred a few times, could use a helper function.

f88f21a

Remove duplicated helper

af30041

Remove unneeded check, use helper

5d9ea3a

Identifiers list is no longer escaped strings

352c13d

weswigham added 2 commits July 5, 2017 12:45

Extract repeated string-getting code into helper

b371e7a

Rename type and associated functions

ba0071e

weswigham added 3 commits July 6, 2017 09:54

Make getName() return UnderscoreEscapedString, add getUnescapedName()

e0c7dcc

Add list of internal symbol names to escaped string type to cut back …

50a8c04

…on casting

Remove outdated comments

3398d41

ghost reviewed Jul 6, 2017

View reviewed changes

sandersn reviewed Jul 6, 2017

View reviewed changes

ajafff reviewed Jul 6, 2017

View reviewed changes

weswigham added 6 commits July 6, 2017 12:58

Reassign interned values to nodes, just in case

a4bd2ab

Swap to string enum

0967927

Add deprecated aliases to escapeIdentifier and unescapeIdentifier

e99b679

Add temp var

3383d8a

Remove unsafe casts

5eca8a3

Rename escaped string type as per @sandersn's suggestion, fix string …

9424f14

…enum usages

weswigham force-pushed the escaped-string-type branch from f15d2ef to 9424f14 Compare July 6, 2017 20:37

sandersn approved these changes Jul 6, 2017

View reviewed changes

weswigham added 2 commits July 6, 2017 13:54

Reorganize double underscore tests

23032d4

Remove jfreeman from TODO

008532d

weswigham added a commit to weswigham/manual that referenced this pull request Jul 6, 2017

Document branded strings in the compiler

537db10

See [here](microsoft/TypeScript#16915 (comment))

weswigham mentioned this pull request Jul 6, 2017

Document branded strings in the compiler sandersn/manual#1

Merged

Remove unneeded parenthesis

5e6b69a

weswigham merged commit 4e6b2f3 into microsoft:master Jul 6, 2017

This was referenced Jul 6, 2017

Type indexing with string literals starting with __ does not work #14268

Closed

TSServer: Extra underscores in properties #11902

Closed

Cannot rename LHS of import specifier when its name is prefixed with two underscores #3268

Closed

ghost mentioned this pull request Jul 7, 2017

Unescape identifiers in booleanTriviaRule #17020

Closed

weswigham deleted the escaped-string-type branch August 17, 2017 23:47

meyer mentioned this pull request Nov 12, 2017

Update JSX intrinsic element test to match babel’s #19946

Merged

microsoft locked and limited conversation to collaborators Jun 14, 2018

		@@ -609,11 +609,13 @@ namespace ts {

		function getTextOfIdentifierOrLiteral(node: Node) {

		@@ -0,0 +1,60 @@
		tests/cases/compiler/index.tsx(33,1): error TS2308: Module "./b" has already exported a member named '__foo'. Consider explicitly re-exporting to resolve the ambiguity.

		@@ -1505,7 +1506,8 @@ namespace ts.Completions {
		// TODO(jfreeman): Account for computed property name

Created a branded type for identifier-escaped strings #16915

Created a branded type for identifier-escaped strings #16915

Conversation

weswigham commented Jul 3, 2017 • edited Loading

weswigham commented Jul 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weswigham Jul 5, 2017 • 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

ghost commented Jul 5, 2017

weswigham commented Jul 5, 2017

ghost commented Jul 5, 2017

weswigham commented Jul 5, 2017

weswigham commented Jul 6, 2017

ghost Jul 6, 2017 • edited by ghost Loading

Choose a reason for hiding this comment

weswigham Jul 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Jul 6, 2017 • edited by ghost Loading

Choose a reason for hiding this comment

sandersn left a 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

weswigham Jul 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Jul 7, 2017 • edited by ghost Loading

Choose a reason for hiding this comment

sandersn left a 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

weswigham commented Jul 3, 2017 •

edited

Loading

weswigham Jul 5, 2017 •

edited

Loading

ghost Jul 6, 2017 •

edited by ghost

Loading

weswigham Jul 6, 2017 •

edited

Loading

ghost Jul 6, 2017 •

edited by ghost

Loading

weswigham Jul 6, 2017 •

edited

Loading

ghost Jul 7, 2017 •

edited by ghost

Loading