Sg embedded sources by chsienki · Pull Request #44768 · dotnet/roslyn

chsienki · 2020-06-01T22:04:07Z

Embed generated source texts in the portable PDB. Allows stepping in etc. in the IDE as well as a dump debugging after compilation.

Also some minor code cleanup that I noticed as I was implementing.

chsienki · 2020-06-02T17:51:17Z

@dotnet/roslyn-compiler for review please

jaredpar · 2020-06-02T21:48:34Z

src/Compilers/CSharp/Test/Semantic/SourceGeneration/TestGenerators.cs

+        private readonly Action<SourceGeneratorContext> _onExecute;
+        private readonly string _sourceOpt;
+
+        public CallbackGenerator(Action<InitializationContext> onInit, Action<SourceGeneratorContext> onExecute, string sourceOpt = "")


Rather than sourceOpt why not use string? source = "" and let nullability enforce the optional ness? #Resolved

jaredpar · 2020-06-02T21:49:13Z

src/Compilers/CSharp/Test/Semantic/SourceGeneration/TestGenerators.cs

+    {
+        private readonly Action<InitializationContext> _onInit;
+        private readonly Action<SourceGeneratorContext> _onExecute;
+        private readonly string _sourceOpt;


Rather than take the source as a parameter to the type why not change _onExecute to be Func<SourceGeneratorContext, string?>? #WontFix

Most of the uses of this generator don't actually add source. We just increment a counter or whatever to check it was called. Making the callback return source just makes it noiser

In reply to: 434194957 [](ancestors = 434194957)

jaredpar · 2020-06-02T21:49:54Z

src/Compilers/CSharp/Test/Semantic/SourceGeneration/TestGenerators.cs

+        public InMemoryAdditionalText(string path, string content)
+        {
+            Path = path;
+            _content = SourceText.From(content);


In all other SourceText.From you explicitly passed Encoding.Utf8. Why is this usage different? #Resolved

Oversight. I'll fix it, but it only actually matters when we're emitting, so we don't hit this in the tests. We have an open tracking note on the board about how to simplify this.

In reply to: 434195219 [](ancestors = 434195219)

jaredpar · 2020-06-02T21:50:26Z

src/Compilers/Core/Portable/SourceGeneration/GeneratedSourceText.cs

 using System.Collections.Generic;
 using System.Text;
 using Microsoft.CodeAnalysis.Text;
+using System.IO;


This seem uneeded #Resolved

src/Compilers/Core/Portable/SourceGeneration/SourceGeneratorContext.cs

jaredpar · 2020-06-02T21:52:53Z

src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs

            // We pass it to the generators, which will realize any symbols they require. 
            compilation = RunGenerators(compilation, Arguments.ParseOptions, generators, additionalTexts, diagnostics);

-            // https://github.com/dotnet/roslyn/issues/44087 


Why did we remove this comment? Feel like this code still needs to be cleaned up once we have a way to determine if SyntaxTree instances are generated or not.

So, I started down that path, but it turns out we don't actually need it. @agocke work removing the diagnostics from the syntax tree means we won't have to parse with the config present. Because the syntax trees are deterministically ordered, we should always just be able to count beyond the length of the Arguments.SyntaxTrees to know which were added by generators?

In reply to: 434196389 [](ancestors = 434196389)

Admittedly, It feels like there is a lot of 'spaghetti' tangle in the way we handle this, various different levels of the compiler adjust the embedded texts and the config options based on their own rules and knowledge of what's coming later. I don't particularly like it, and it feels ripe for a refactor/cleanup pass but this PR at least doesn't make it worse.

In reply to: 434210846 [](ancestors = 434210846,434196389)

K. Have we checked with @jasonmalinowski on whether or not he feels the IDE team will need to be able to recognize generated SyntaxTree instances over non-generated ones?

We'll almost definitely need to do it in the future, its just not needed for this particular feature. (The IDE already works with the embedded texts and doesn't need to know they're generated)

In reply to: 434221150 [](ancestors = 434221150)

jaredpar · 2020-06-02T21:53:02Z

src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs

+                if (!sourceFileAnalyzerConfigOptions.IsDefault)
+                {
+                    sourceFileAnalyzerConfigOptions = sourceFileAnalyzerConfigOptions.AddRange(generatedSyntaxTrees.Select(f => analyzerConfigSet.GetOptionsForSourcePath(f.FilePath)));
+                }


Suggested change

}

}

``` #Resolved

jaredpar · 2020-06-02T23:01:40Z

src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs

+
+            // Clean up temp files
+            CleanupAllGeneratedFiles(src.Path);
+        }


Need to test some more cases:

Multiple generated files

Multiple generated files with identical hint values

Case where an empty file is returned

Generally do we have tests where the hint path contains known invalid file name characters?

Added more tests, but ran into #42628, which I've now addresses as part of this PR too.

We aren't currently putting these down on disk, just using them as identifiers in the PDB, so we should be good from a characters perspective. We'll need to address it when we put them to disk though (presumably some form of encoding?)

In reply to: 434220765 [](ancestors = 434220765)

jaredpar · 2020-06-02T23:02:57Z

src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs

            // We pass it to the generators, which will realize any symbols they require. 
            compilation = RunGenerators(compilation, Arguments.ParseOptions, generators, additionalTexts, diagnostics);

-            // https://github.com/dotnet/roslyn/issues/44087 


K. Have we checked with @jasonmalinowski on whether or not he feels the IDE team will need to be able to recognize generated SyntaxTree instances over non-generated ones?

…he generator name. Add more tests

jaredpar · 2020-06-03T17:30:17Z

src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs

+        [InlineData("partial class D {}", "file1.cs", "partial class D {}", "file2.cs")] // same files, different names
+        [InlineData("partial class D {}", "file1.cs", "partial class D {}", "file1.cs")] // same files, same names
+        [InlineData("partial class D {}", "file1.cs", "", "file2.cs")] // empty second file
+        [InlineData("partial class D {}", "file1.cs", "", " \\ / : * ? \" < > | ")] // illegal path name chars


Do we just silently drop the illegal path name characters?

No, they're allows as a valid hint name. We're just embedding it into the PDB not putting it to disk at the moment. We'll need to encode when we put to disk (filenames, especially on windows, look like a minefield :/).

I'll add text to the second file to make it clear we're using it.

In reply to: 434736894 [](ancestors = 434736894)

@tmat will portable PDB's handle path names with illegal characters okay?

@tmat can confirm, but the PDB itself seems fine. Interestingly VS chokes a bit when hitting a file with them: it steps in, and you can actually step over each line and see the output, but no file opens in the IDE itself (no crash or other visible errors though)

In reply to: 434767181 [](ancestors = 434767181)

I think we should file a follow up item here to resolve for 16.8. Need to take into account the following:

How does hint path impact the physical file on disk when we're in dump to disk mode

How do we deal with generators where added files hint paths differ by only illegal file system characters

How do illegal characters in the path impact other parts of the experience

Don't think we necessarily need to solve this for 16.7 though. One simple approach though is just ban illegal file system characters in hint paths because at the end of the day this is meant to represent a file name on disk 😄

Portable PDB has no requirements on path strings. Note that all characters except for \0 are valid path characters on Linux.

BTW, interesting test could be unmatched Unicode surrogates. PDB should handle them as well (and preserve their value). Not sure what happens elsewhere in the system though.

src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs

jaredpar · 2020-06-03T17:33:01Z

src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs

        }
    }
+
+    internal class SimpleGenerator2 : SimpleGenerator


Can you explain why we need this class here? It doesn't seem to add anything over SimpleGenerator

jaredpar · 2020-06-03T17:34:58Z

src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs


        internal GeneratorDriver(GeneratorDriverState state)
        {
+            Debug.Assert(state.Generators.GroupBy(s => s.GetType()).Count() == state.Generators.Length); // ensure we don't have duplicate generator types


Why do you want to restrict generator types to be unique? Consider the case where someone authors a really handy generator library and two different generators consume it. That seems like a valid case where we would have duplicate generator types.

So the rationale here was how they're actually loaded from the command line: we pass in a set of references and create the instance ourselves. That means there can only ever be one type per driver instance. That means that we can safely use the type name of the generator as a prefix for disambiguation.

Now we can of course allow multiple of the same type when constructing in tests, but it doesn't actually reflect the way user consume it from the outside. I put it in as a Debug.Assert to ensure that what we're testing is actually what happens in the real case. SourceGeneratorDriver is public and users are free to put the same type in twice if they're using it as an API.

I wasn't sure about this tbh: I'd obviously prefer that we allow multiple of the same type in the tests (it make it much easier) but doesn't reflect the real situation, and gets into weirdness with the prefix. We could make generators provide a unique prefix? But still doesn't stop collisions.

In reply to: 434739560 [](ancestors = 434739560)

Although writing that I realize that the full name of the type isn't actually enough to be unique right? I can have two types with the same full name in two different assemblies?

In reply to: 434750141 [](ancestors = 434750141,434739560)

That means there can only ever be one type per driver instance.

Consider the case where I have authored a library Generator.Utility.dll which has an accessible type SimpleGenerator that implements ISourceGenerator. Two different generators could reference this library and expose that type.

Hmmm ... Or do we scan the generator assembly directly and look for derivations of ISourceGenerator? That wouldn't have this problem but if we had a factory model we would have this issue.

We scan for a combo of derived form ISourceGenerator and marked with the SourceGeneratorAttribute. I'll have to double check, but I believe in your example we'd still only end up with one generator (we use the same mechanism as analyzer discovery, so its what ever that does)

In reply to: 434766641 [](ancestors = 434766641)

jaredpar · 2020-06-03T17:35:37Z

src/Compilers/Core/Portable/SourceGeneration/SourceGeneratorContext.cs

+
+            if (AdditionalSources.Contains(hintName))
+            {
+                throw new ArgumentException(CodeAnalysisResources.HintNameUniquePerGenerator, nameof(hintName));


Can we file a bug to track this? I don't think this is sustainable long term.

That hint names must be unique within a generator?

In reply to: 434739910 [](ancestors = 434739910)

Yes. Generators essentially represent a form of distributed development. Any aspect of the implementation which requires them to coordinate, such as not having conflicting hint names, makes authoring reliable generators difficult. The only safe approach would be to always use a GUID at which point it's not much of a hint.

No, they only have to be unique within a generator. Different generators can have the same hint name.

In reply to: 434765041 [](ancestors = 434765041)

Gotcha. Assume we have tests for that?

Yeah Generator_HintName_MustBe_Unique and Generator_HintName_Is_Appended_With_GeneratorName

In reply to: 434770864 [](ancestors = 434770864)

Ensure ordering of syntax trees

chsienki · 2020-06-04T00:06:01Z

Well, that ended up being a bigger rabbit hole than I intended to go down, but it did end up fixing a couple of extra bugs (fixes #42627 and #42593).

I've ended up going the route that we limit hint names to a specific set of characters (IdentifierName + [.,-_ []{}()]). I moved the logic into the AdditionalSourcesCollection and added a bunch of tests. That ended up surfacing issue (#42627), which was fixed by ensuring we maintain the ordering source texts are added and also the order in which generators are run. Fixing the ordering of generators in turn removed the dictionary in #42593.

chsienki · 2020-06-04T00:09:55Z

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

+                    for (int i = 0; i < _sourcesAdded.Count; i++)
+                    {
+                        // check the hashcodes, as thats the comparison we're using to put the names into _hintNames
+                        if (_sourcesAdded[i].HintName.GetHashCode() == hintName.GetHashCode())


I considered making the GeneratedSourceTexts implement equals and hashcode in terms of the hintName, but they're only equal in the context of a single generator (which is equivalent to a single AdditionalSourcesCollection)

I suppose we could use a custom comparer just in this class and check the hintName? That would allows us to get rid of the two collections and locks? EDIT: we can, but it means we don't get O(1) lookup for things in the collection, because we have to iterate over each time (and thus still need to lock anyway) so i'm not sure it's worth it?

This seems a bit complicated. Why not just use a Dictionary<string, GeneratedSourceText> instead of having parallel collections here? Could event use a ConcurrentDictionary and avoid all of the lock usage.

The reason was that Dictionary isn't ordered. This ensures that we get syntax trees back in the order in which they were added. Presumably we could use an ordered dictionary and sort by the hintName, which would at least be consistent, but not the order they were added in. Do you think it matters, as long as its deterministic?

In reply to: 435565199 [](ancestors = 435565199)

I ended up simplifying and just keeping an array builder. That way we'll keep the ordering as they're added, but don't need the locks of the second collection. It means Remove() and Contains() (and by extension Add()) are all O(N) operations now, as we have to check the whole collection for collisions. If it turns out to be a perf bottleneck we can fix it later on.

This also makes us not-thread safe as a generator could theoretically create a race condition that gets two additions into the collection with the same hintName. This could cause the compiler to crash further on (when adding to the PDBs for instance), but for now I think we're willing to live with that. I mean they could just Environment.FailFast if that was their aim.

In reply to: 435637087 [](ancestors = 435637087,435565199)

jaredpar · 2020-06-04T21:27:48Z

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

        internal AdditionalSourcesCollection()
        {
-            _sourcesAdded = PooledDictionary<string, SourceText>.GetInstance();
+            _hintNames = PooledHashSet<string>.GetInstance();


Should we provide an explicit string comparer here? #Resolved

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

jaredpar · 2020-06-04T21:29:47Z

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

@@ -13,44 +14,108 @@ namespace Microsoft.CodeAnalysis
 {
    internal sealed class AdditionalSourcesCollection


Is this a per generator collection?

Yes, we create a new one for every invocation of every generator.

In reply to: 435563892 [](ancestors = 435563892)

jaredpar · 2020-06-04T21:32:38Z

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

+                    for (int i = 0; i < _sourcesAdded.Count; i++)
+                    {
+                        // check the hashcodes, as thats the comparison we're using to put the names into _hintNames
+                        if (_sourcesAdded[i].HintName.GetHashCode() == hintName.GetHashCode())


This seems a bit complicated. Why not just use a Dictionary<string, GeneratedSourceText> instead of having parallel collections here? Could event use a ConcurrentDictionary and avoid all of the lock usage.

jaredpar · 2020-06-04T21:33:10Z

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

+                    for (int i = 0; i < _sourcesAdded.Count; i++)
+                    {
+                        // check the hashcodes, as thats the comparison we're using to put the names into _hintNames
+                        if (_sourcesAdded[i].HintName.GetHashCode() == hintName.GetHashCode())


Can't compare HashCode here because you can get false matches.

But we're comparing the hash of the string, it'll always be the same if they're considered equal, no? Otherwise the HashSet wouldn't work either?

In reply to: 435565433 [](ancestors = 435565433)

jaredpar · 2020-06-04T21:33:59Z

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

+
+        private static string AppendExtensionIfRequired(string hintName)
+        {
+            if (!Path.GetExtension(hintName).Equals(".cs", StringComparison.OrdinalIgnoreCase))


The code for comparing hint names in general is ordinal equality but the path is ignore case. Should be consistent here.

Good catch. I've made all the hint path comparison be case insensitive as we'll need them to be to support windows anyway. Added more test case theory data to cover it.

In reply to: 435565786 [](ancestors = 435565786)

- Simplify AdditionalSourceCollection - Ensure we always compare case-insensitively for hintPaths

jaredpar · 2020-06-05T04:37:53Z

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

-        private readonly PooledDictionary<string, SourceText> _sourcesAdded;
+        private readonly ArrayBuilder<GeneratedSourceText> _sourcesAdded;
+
+        private readonly StringComparer _hintNameComparer = StringComparer.OrdinalIgnoreCase;


Consider making static readonly #Resolved

jaredpar · 2020-06-05T04:39:45Z

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

+            {
+                if (_hintNameComparer.Equals(_sourcesAdded[i].HintName, hintName))
+                {
+                    _sourcesAdded.Remove(_sourcesAdded[i]);


Suggested change

_sourcesAdded.Remove(_sourcesAdded[i]);

_sourcesAdded.RemoveAt(i);

``` #Resolved

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

jaredpar · 2020-06-05T04:42:16Z

src/Compilers/Core/Portable/SourceGeneration/AdditionalSourcesCollection.cs

-            // https://github.com/dotnet/roslyn/issues/42627: This needs to be consistently ordered
-            ArrayBuilder<GeneratedSourceText> builder = ArrayBuilder<GeneratedSourceText>.GetInstance();
-            foreach (var (hintName, sourceText) in _sourcesAdded)
+            if (!Path.GetExtension(hintName).Equals(".cs", StringComparison.OrdinalIgnoreCase))


Did you consider using hintName.EndsWith(...) here? Using GetExtension means that operations like Contains end up being allocating. #Resolved

agocke · 2020-06-05T19:54:35Z

I don't understand why the tests keep passing Encoding.UTF8 to SourceText.From.

chsienki · 2020-06-05T21:15:32Z

Becuase the PDB code checks that a SourceText with a file path must have an encoding, or they'll fail at emit. We've talked about making it a default, but not done yet.

In reply to: 639759958 [](ancestors = 639759958)

agocke · 2020-06-05T21:22:36Z

Got it, so the purpose is to write out the source text as UTF-8 when embedding into the PDB? Is there a place where we validate that every incoming source text has an encoding? I couldn't easily find it in this PR.

chsienki · 2020-06-05T21:39:14Z

The actual check happens here in Compilation http://sourceroslyn.io/#Microsoft.CodeAnalysis/Compilation/Compilation.cs,2076

We have another issue about making this better, but it's not actually directly related to the embedded requirement. We're thinking we'll just make SourceText.From use UTF8 by default if it's not specified.

In reply to: 639827040 [](ancestors = 639827040)

agocke · 2020-06-05T21:45:23Z

We're thinking we'll just make SourceText.From use UTF8 by default if it's not specified

That's fine. But what we should avoid is only giving a diagnostic when the sources are embedded as we don't want generators to function when embedding is turned off, then start to break when it's suddenly turned on.

agocke

LGTM

chsienki · 2020-06-05T21:48:39Z

We're thinking we'll just make SourceText.From use UTF8 by default if it's not specified

That's fine. But what we should avoid is only giving a diagnostic when the sources are embedded as we don't want generators to function when embedding is turned off, then start to break when it's suddenly turned on.

Nah, it was doing it before anyway, so no change there. Thanks for the review!

chsienki added 7 commits June 1, 2020 14:59

Append '.cs' to hint name if missing

dc6ab31

Remove suppression

9b17480

Split out test generators into a seperate file

4e9bf20

Embed generated texts and add test to cover

0765298

Fix ide test

7655c8c

Whitespace

025c089

Cleanup duplicate code

3d57a69

chsienki added Area-Compilers Feature - Source Generators Source Generators labels Jun 2, 2020

chsienki added this to the 16.7.P3 milestone Jun 2, 2020

chsienki marked this pull request as ready for review June 2, 2020 17:51

chsienki requested review from a team as code owners June 2, 2020 17:51

jaredpar reviewed Jun 2, 2020

View reviewed changes

chsienki added 2 commits June 2, 2020 15:28

PR Feedback

ad709c1

derp.

661a339

jaredpar reviewed Jun 2, 2020

View reviewed changes

chsienki added 2 commits June 2, 2020 18:50

Ensure uniqueness of hintnames per generator, and prefix names with t…

a7ab4e3

…he generator name. Add more tests

Fix tests

86707c1

jaredpar reviewed Jun 3, 2020

View reviewed changes

Restrict hintNames to filepath characters.

4359337

Ensure ordering of syntax trees

chsienki commented Jun 4, 2020

View reviewed changes

chsienki added 5 commits June 3, 2020 18:26

Derp.

ed4940a

Include the MVID in the filenames to ensure uniqueness

29290c2

FIx cmdline tests

7b5be90

Fix tests on linux/unix

05755fa

Fix linux/macos round 2

86306ef

jaredpar reviewed Jun 4, 2020

View reviewed changes

PR Feedback:

d910455

- Simplify AdditionalSourceCollection - Ensure we always compare case-insensitively for hintPaths

jaredpar reviewed Jun 5, 2020

View reviewed changes

PR Feedback

550efa2

chsienki mentioned this pull request Jun 5, 2020

Sg editorconfig access #44875

Merged

jaredpar approved these changes Jun 5, 2020

View reviewed changes

agocke approved these changes Jun 5, 2020

View reviewed changes

chsienki merged commit e6afc95 into dotnet:master Jun 5, 2020

ghost modified the milestones: 16.7.P3, Next Jun 5, 2020

RikkiGibson modified the milestones: Next, 16.7.P3 Jun 8, 2020

-                }
+                }
+``` #Resolved

		@@ -13,44 +14,108 @@ namespace Microsoft.CodeAnalysis
		{
		internal sealed class AdditionalSourcesCollection

	_sourcesAdded.Remove(_sourcesAdded[i]);
	_sourcesAdded.RemoveAt(i);
	``` #Resolved

Conversation

chsienki commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chsienki commented Jun 2, 2020

Uh oh!

jaredpar Jun 2, 2020 • edited by chsienki Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaredpar Jun 2, 2020 • edited by chsienki Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaredpar Jun 2, 2020 • edited by chsienki Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaredpar Jun 2, 2020 • edited by chsienki Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaredpar Jun 2, 2020 • edited by chsienki Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chsienki Jun 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmat Jun 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chsienki Jun 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chsienki commented Jun 1, 2020 •

edited

Loading

jaredpar Jun 2, 2020 •

edited by chsienki

Loading

jaredpar Jun 2, 2020 •

edited by chsienki

Loading

jaredpar Jun 2, 2020 •

edited by chsienki

Loading

jaredpar Jun 2, 2020 •

edited by chsienki

Loading

jaredpar Jun 2, 2020 •

edited by chsienki

Loading

chsienki Jun 3, 2020 •

edited

Loading

tmat Jun 3, 2020 •

edited

Loading

chsienki Jun 3, 2020 •

edited

Loading

chsienki commented Jun 4, 2020 •

edited

Loading

chsienki Jun 4, 2020 •

edited

Loading

jaredpar Jun 4, 2020 •

edited by chsienki

Loading