Move to the use site tracking for the purpose of recording used assemblies by AlekseyTs · Pull Request #40560 · dotnet/roslyn

AlekseyTs · 2019-12-23T07:19:11Z

The basic idea is that dependencies should be tracked along with use-site diagnostics and should be collected together with regular diagnostic messages and bubble up to the top. The final consumer of the combined diagnostics and dependencies is responsible for “registering” dependencies with the compilation or ignore them (due to the context, etc.). I believe this is a very robust approach and would require little maintenance going forward, because every time a symbol is used, a use-site diagnostics should be reported. We just need to keep doing that for every new feature and dependencies will be tracked automatically. Also, the code performing binding no longer needs to be context aware (should dependencies be tracked at this moment or not), they should always be tracked and collected along with the regular diagnostics and it is up to the consumer to decide what to do with them, if anything. For example, when SemanticModel performs binding, dependencies are not registered, etc.

- Merged extern aliases - Embeddable attributes - Usage of canonical definitions for NoPia embedded types - nameof of a method - Deconstruction - Collection initializers Related to dotnet#37768.

…blies

AlekseyTs · 2019-12-23T17:21:11Z

@dotnet/roslyn-compiler Please review.

AlekseyTs · 2019-12-26T17:37:56Z

@dotnet/roslyn-compiler Please review.

333fred · 2019-12-26T21:53:42Z

/// This is base class for a bag used to accumulate various information as binding is performed.

Idiomatic English would be either "various pieces of information" or just "information" #Resolved

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:15 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

gafter · 2019-12-26T22:00:02Z

    private bool _haveErrors;

_hasErrors #Closed

Refers to: src/Compilers/Core/Portable/Binding/UseSiteInfo.cs:93 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-26T22:05:25Z

        Debug.Assert(DiagnosticBag is object);

Why assert DiagnosticBag is non-null then ?. it? #Resolved

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:45 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

gafter · 2019-12-26T22:05:29Z

It would be really nice to have a measure of the performance impact of this approach, if any. #Closed

333fred · 2019-12-26T22:05:55Z

        Debug.Assert(DiagnosticBag is object);

Also, consider using RoslynDebug.Assert, which has been nullable annotated.

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

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:45 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-26T22:15:00Z

    internal void Free()

Should we only free if we were allocated to use the pool? #Resolved

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:81 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-26T22:21:13Z

    internal void AddRangeAndFree(BindingDiagnosticBag<TAssemblySymbol> other)

Is this a common pattern we have already? If not, I'd suggest AddAndFreeRange as a better name. AddRangeAndFree implies, to me, that the current range will be freed, not the range passed in. By putting Range last, you better imply that both actions will apply to the given range. #Closed

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:99 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-26T22:27:55Z

        if (dependency is object && DependenciesBag is object)

Consider either making this a ?. or using this pattern in the method right above. #WontFix

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:147 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-26T22:43:28Z

        return Diagnostics.GetHashCode();

Why not include Dependencies in the hash code? #WontFix

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:298 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

gafter · 2019-12-26T22:59:46Z

internal class BindingDiagnosticBag

We need a better name for this. Having a binding-diagnostic-bag (typically named diagnostic-bag) that contains a diagnostic-bag named diagnostic-bag is just too much. #Closed

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:18 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

gafter · 2019-12-26T23:00:51Z

src/Compilers/CSharp/Portable/Binder/BindingDiagnosticBag.cs

+
+namespace Microsoft.CodeAnalysis.CSharp
+{
+    internal static class BindingDiagnosticBagExtensions


BindingDiagnosticBagExtensions [](start = 26, length = 30)

File is incorrectly named given this class name. #Closed

File is incorrectly named given this class name.

The BindingDiagnosticBag class is below

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

Please place the class BindingDiagnosticBagExtensions in a file BindingDiagnosticBagExtensions.cs.

In reply to: 361549090 [](ancestors = 361549090,361546561)

Added a PROTOTYPE comment in #40640

In reply to: 361554376 [](ancestors = 361554376,361549090,361546561)

AlekseyTs · 2019-12-26T23:01:06Z

/// This is base class for a bag used to accumulate various information as binding is performed.

Idiomatic English would be either "various pieces of information" or just "information"

Would this sound better: "This is base class for a bag used to accumulate various information while binding is performed"?

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

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:15 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-26T23:01:46Z

internal class BindingDiagnosticBag

We need a better name for this. Having a binding-diagnostic-bag (typically named diagnostic-bag) that contains a diagnostic-bag named diagnostic-bag is just too much.

I am open to suggestions.

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

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:18 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

gafter · 2019-12-26T23:03:09Z

    public abstract void ReportInvalidAttributeArgument(DiagnosticBag diagnostics, SyntaxNode attributeSyntax, int parameterIndex, AttributeData attribute);

If this is only invoked in the method below, it should probably be changed to protected. Do you know somewhere it is used outside this type? Same for the other abstract methods below. #Resolved

Refers to: src/Compilers/Core/Portable/Diagnostic/CommonMessageProvider.cs:244 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-26T23:03:50Z

    internal void Free()

Should we only free if we were allocated to use the pool?

Not necessary. We should free when it is appropriate to free. I.e. constituent pieces are either null or allocated from a pool.

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

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:81 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-26T23:06:44Z

        Debug.Assert(DiagnosticBag is object);

Why assert DiagnosticBag is non-null then ?. it?

Even though it is safe to call this method any time. It is likely not the right thing to do when we are not collecting diagnostics. In other words, this assert is not about null-safety.

In reply to: 569140729 [](ancestors = 569140729,569140668)

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:45 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-26T23:08:33Z

    internal void AddRangeAndFree(BindingDiagnosticBag<TAssemblySymbol> other)

Is this a common pattern we have already? If not, I'd suggest AddAndFreeRange as a better name. AddRangeAndFree implies, to me, that the current range will be freed, not the range passed in. By putting Range last, you better imply that both actions will apply to the given range.

Yes, this is a common pattern with DiagnosticBag, keeping the same name avoids significant amount of code changes.

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

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:99 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

gafter · 2019-12-26T23:09:26Z

Assert that reference != null? Or maybe #nullable enable the whole file? #WontFix

Refers to: src/Compilers/Core/Portable/ReferenceManager/MergedAliases.cs:33 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-26T23:09:38Z

    internal void AddRangeAndFree(BindingDiagnosticBag<TAssemblySymbol> other)

Yes, this is a common pattern with DiagnosticBag,

As far as I can tell, we have no methods named this currently.

In reply to: 569147655 [](ancestors = 569147655,569142395)

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:99 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-26T23:10:26Z

/// This is base class for a bag used to accumulate various information as binding is performed.

No. The problem is various information.

In reply to: 569146820 [](ancestors = 569146820,569139255)

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:15 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-26T23:10:56Z

    public abstract void ReportInvalidAttributeArgument(DiagnosticBag diagnostics, SyntaxNode attributeSyntax, int parameterIndex, AttributeData attribute);

If this is only invoked in the method below, it should probably be changed to protected. Do you know somewhere it is used outside this type? Same for the other abstract methods below.

I see nothing wrong in calling these methods directly, now or in the future. Keeping original accessibility.

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

Refers to: src/Compilers/Core/Portable/Diagnostic/CommonMessageProvider.cs:244 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-26T23:17:09Z

/// This is base class for a bag used to accumulate various information as binding is performed.

Specifically, various is an adjective that implies the noun it's modifying is countable, while information is not a countable noun. So it needs to either add a countable thing (pieces of) or be removed.

In reply to: 569147835 [](ancestors = 569147835,569146820,569139255)

Refers to: src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs:15 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

gafter · 2019-12-30T23:22:28Z

    public abstract void ReportInvalidAttributeArgument(DiagnosticBag diagnostics, SyntaxNode attributeSyntax, int parameterIndex, AttributeData attribute);

This is a violation of good software engineering principles (e.g. principle of least privilege). Please restrict access.

In reply to: 569147877 [](ancestors = 569147877,569147070)

Refers to: src/Compilers/Core/Portable/Diagnostic/CommonMessageProvider.cs:244 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

gafter · 2019-12-30T23:24:37Z

Finished review pass (Iteration 4). The only issue remaining that concerns me is the access of ReportInvalidAttributeArgument and similar methods. #Closed

333fred · 2019-12-31T02:20:52Z

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

-        private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModuleBuilder moduleBeingBuilt, bool hasDeclarationErrors, bool emitMethodBodies, DiagnosticBag diagnostics, CancellationToken cancellationToken)
+        private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModuleBuilder moduleBeingBuilt, bool hasDeclarationErrors, bool emitMethodBodies, BindingDiagnosticBag diagnostics, CancellationToken cancellationToken)
        {
+            Debug.Assert(diagnostics.DiagnosticBag != null);


Consider using either is object, or RoslynDebug.Assert. This is a pure null check in an unannotated method, and will cause warnings when nullable is eventually enabled. A simple change now makes it easier later. #WontFix

Consider using either is object, or RoslynDebug.Assert. This is a pure null check in an unannotated method, and will cause warnings when nullable is eventually enabled. A simple change now makes it easier later.

This file has lots of asserts that will have to be changed when nullable is enabled here, for now I keep the same stile as used throughout the file.

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

333fred · 2019-12-31T02:21:26Z

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

-        private void CompileSynthesizedMethods(ImmutableArray<NamedTypeSymbol> additionalTypes, DiagnosticBag diagnostics)
+        private void CompileSynthesizedMethods(ImmutableArray<NamedTypeSymbol> additionalTypes, BindingDiagnosticBag diagnostics)
        {
+            Debug.Assert(diagnostics.DiagnosticBag != null);


Same comment here. #WontFix

333fred · 2019-12-31T02:21:59Z

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

        {
            Debug.Assert(compilation != null);
            Debug.Assert(diagnostics != null);
+            Debug.Assert(diagnostics.DiagnosticBag != null);


Consider using either is object or RoslynDebug.Assert #WontFix

333fred · 2019-12-31T02:24:33Z

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

                Debug.Assert(!discardedDiagnostics.HasAnyErrors());
-                discardedDiagnostics.Free();
+                discardedDiagnostics.DiagnosticBag.Clear();
+                _diagnostics.AddRangeAndFree(discardedDiagnostics);


Consider using a method that is explicit about adding Dependencies only. Calling clear on the line before this is confusing at first read. #Resolved

Consider using a method that is explicit about adding Dependencies only. Calling clear on the line before this is confusing at first read.

This is addressed in #40640

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

333fred · 2019-12-31T02:35:28Z

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncRewriter.cs

+            if (!hasErrors)
            {
-                diagnostics.AddRange(bag);
+                bag.DiagnosticBag.Clear();


Perhaps add a clarifying comment that we still want to record assembly usage in the presence of errors. #Resolved

Perhaps add a clarifying comment that we still want to record assembly usage in the presence of errors.

Moving to using AddDependencies API instead

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

This is addressed in #40640

In reply to: 362138345 [](ancestors = 362138345,362137536)

333fred · 2019-12-31T02:37:06Z

src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorRewriter.cs

+            if (!hasErrors)
            {
-                diagnostics.AddRange(bag);
+                bag.DiagnosticBag.Clear();


Same clarifying comment would be useful. #Resolved

Same clarifying comment would be useful.

Moving to using AddDependencies API instead

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

This is addressed in #40640

In reply to: 362138975 [](ancestors = 362138975,362137708)

333fred · 2019-12-31T02:50:03Z

        else if (!_binder.IsSemanticModelBinder &&

I really do like the overall approach much better here. This is the type of code that was worrying me about needing to be spread everywhere, and now it just comes along for the ride. #Resolved

Refers to: src/Compilers/CSharp/Portable/Symbols/AliasSymbol.cs:318 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = True)

333fred · 2019-12-31T03:04:33Z

                    GetBoundReferenceManager().MergedAssemblyReferencesMap.TryGetValue(reference, out ImmutableArray<MetadataReference> merged))

Should we consider saving this off to a temp? #Resolved

Refers to: src/Compilers/CSharp/Portable/Symbols/Compilation_UsedAssemblies.cs:40 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-31T03:06:06Z

    public abstract void ReportInvalidAttributeArgument(DiagnosticBag diagnostics, SyntaxNode attributeSyntax, int parameterIndex, AttributeData attribute);

This is a violation of good software engineering principles (e.g. principle of least privilege). Please restrict access.

I do not agree with this “hardcore” application of the principle in this particular case. We are dealing with internal utility type, addition of the convenience wrappers doesn’t make the methods being wrapped less useful or safe. However, it feels like you are not open to having any discussion around the subject and are willing to block PR on this issue alone. For no other reason, but to get about a week worth of work moving forward, I will adjust accessibility.

In reply to: 569825083 [](ancestors = 569825083,569147877,569147070)

Refers to: src/Compilers/Core/Portable/Diagnostic/CommonMessageProvider.cs:244 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-31T03:08:36Z

                    GetBoundReferenceManager().MergedAssemblyReferencesMap.TryGetValue(reference, out ImmutableArray<MetadataReference> merged))

Should we consider saving this off to a temp?

Could you clarify please, which part?

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

Refers to: src/Compilers/CSharp/Portable/Symbols/Compilation_UsedAssemblies.cs:40 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-31T03:10:51Z

                    GetBoundReferenceManager().MergedAssemblyReferencesMap.TryGetValue(reference, out ImmutableArray<MetadataReference> merged))

Is this about MergedAssemblyReferencesMap?

In reply to: 569854489 [](ancestors = 569854489,569854107)

Refers to: src/Compilers/CSharp/Portable/Symbols/Compilation_UsedAssemblies.cs:40 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-31T03:13:44Z

                                                                          useSiteInfo.Dependencies.Count == 1 ?

Odd formatting here. Please align with the first parameter above. #WontFix

Refers to: src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs:988 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-31T03:14:43Z

        static ArrayBuilder<TypeParameterDiagnosticInfo> ensureUseSiteDiagnosticsBuilder(ref ArrayBuilder<TypeParameterDiagnosticInfo> useSiteDiagnosticsBuilder)

It doesn't look like this return is being used. #Resolved

Refers to: src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs:1015 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-31T03:15:08Z

                    GetBoundReferenceManager().MergedAssemblyReferencesMap.TryGetValue(reference, out ImmutableArray<MetadataReference> merged))

Extracting MergedAssemblyReferencesMap into a local. Please reactivate if this will not address the suggestion.

In reply to: 569854683 [](ancestors = 569854683,569854489,569854107)

Refers to: src/Compilers/CSharp/Portable/Symbols/Compilation_UsedAssemblies.cs:40 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-31T03:17:21Z

                                                                          useSiteInfo.Dependencies.Count == 1 ?

Odd formatting here. Please align with the first parameter above.

I believe this doesn't violate formatting guidelines

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

Refers to: src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs:988 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-31T03:17:39Z

                result = result.AdjustDiagnosticInfo(diagnosticInfo);

Consider doing this part unconditionally, and returning the result of the above conditional (saving said conditional off to a temp). #WontFix

Refers to: src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs:904 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-31T03:18:15Z

                result = result.AdjustDiagnosticInfo(diagnosticInfo);

Consider doing this part unconditionally, and returning the result of the above conditional (saving said conditional off to a temp). #WontFix

Refers to: src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs:354 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-31T03:18:44Z

        static ArrayBuilder<TypeParameterDiagnosticInfo> ensureUseSiteDiagnosticsBuilder(ref ArrayBuilder<TypeParameterDiagnosticInfo> useSiteDiagnosticsBuilder)

It doesn't look like this return is being used.

Looks used to me: ensureUseSiteDiagnosticsBuilder(ref useSiteDiagnosticsBuilder).Add

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

Refers to: src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs:1015 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-31T03:20:14Z

                                                                          useSiteInfo.Dependencies.Count == 1 ?

This is an argument for new TypeParameterDiagnosticInfo(, aligning it with typeParameter argument would move code to far to the right.

In reply to: 569855345 [](ancestors = 569855345,569854959)

Refers to: src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs:988 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred · 2019-12-31T03:21:05Z

                result = result.AdjustDiagnosticInfo(diagnosticInfo);

And here. #WontFix

Refers to: src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs:373 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-31T04:07:39Z

    public abstract void ReportInvalidAttributeArgument(DiagnosticBag diagnostics, SyntaxNode attributeSyntax, int parameterIndex, AttributeData attribute);

This is addressed in #40640

In reply to: 569854263 [](ancestors = 569854263,569825083,569147877,569147070)

Refers to: src/Compilers/Core/Portable/Diagnostic/CommonMessageProvider.cs:244 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

AlekseyTs · 2019-12-31T04:10:25Z

                    GetBoundReferenceManager().MergedAssemblyReferencesMap.TryGetValue(reference, out ImmutableArray<MetadataReference> merged))

This is addressed in #40640

In reply to: 569855110 [](ancestors = 569855110,569854683,569854489,569854107)

Refers to: src/Compilers/CSharp/Portable/Symbols/Compilation_UsedAssemblies.cs:40 in dce0ee8. [](commit_id = dce0ee8, deletion_comment = False)

333fred

LGTM (commit 4) with followup PR

AlekseyTs · 2019-12-31T04:46:19Z

@gafter I believe the issue that you had indicated as blocking has been addressed.

gafter

; LGTM based on changes in #40640

…blies in VB (#40640) This change is continuing work started in #40560. It makes similar changes on VB side.

AlekseyTs added 4 commits December 12, 2019 20:45

Record used assembly references for more scenarios.

f6ac474

- Merged extern aliases - Embeddable attributes - Usage of canonical definitions for NoPia embedded types - nameof of a method - Deconstruction - Collection initializers Related to dotnet#37768.

Minor adjustment

a376e99

Fix tests

3f488c6

Move to the use site tracking for the purpose of recording used assem…

dce0ee8

…blies

AlekseyTs added the Area-Compilers label Dec 23, 2019

AlekseyTs requested a review from a team as a code owner December 23, 2019 07:19

gafter reviewed Dec 26, 2019

View reviewed changes

333fred reviewed Dec 31, 2019

View reviewed changes

333fred approved these changes Dec 31, 2019

View reviewed changes

gafter approved these changes Dec 31, 2019

View reviewed changes

AlekseyTs merged commit bb8f2df into dotnet:features/UsedAssemblyReferences Dec 31, 2019

AlekseyTs added a commit that referenced this pull request Jan 4, 2020

Move to the use site tracking for the purpose of recording used assem…

d6a5d04

…blies in VB (#40640) This change is continuing work started in #40560. It makes similar changes on VB side.

Conversation

AlekseyTs commented Dec 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs commented Dec 23, 2019

Uh oh!

AlekseyTs commented Dec 26, 2019

Uh oh!

333fred commented Dec 26, 2019 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gafter commented Dec 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

333fred commented Dec 26, 2019 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gafter commented Dec 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

333fred commented Dec 26, 2019

Uh oh!

333fred commented Dec 26, 2019 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

333fred commented Dec 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

333fred commented Dec 26, 2019 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

333fred commented Dec 26, 2019 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gafter commented Dec 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gafter Dec 26, 2019 • 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!

AlekseyTs commented Dec 26, 2019

Uh oh!

AlekseyTs commented Dec 26, 2019

Uh oh!

gafter commented Dec 26, 2019 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs commented Dec 26, 2019

Uh oh!

AlekseyTs commented Dec 26, 2019

Uh oh!

AlekseyTs commented Dec 26, 2019

Uh oh!

gafter commented Dec 26, 2019 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

333fred commented Dec 26, 2019

Uh oh!

333fred commented Dec 26, 2019

Uh oh!

AlekseyTs commented Dec 26, 2019

Uh oh!

333fred commented Dec 26, 2019

Uh oh!

gafter commented Dec 30, 2019

Uh oh!

gafter commented Dec 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

333fred Dec 31, 2019 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs commented Dec 23, 2019 •

edited

Loading

333fred commented Dec 26, 2019 •

edited by AlekseyTs

Loading

gafter commented Dec 26, 2019 •

edited

Loading

333fred commented Dec 26, 2019 •

edited by AlekseyTs

Loading

gafter commented Dec 26, 2019 •

edited

Loading

333fred commented Dec 26, 2019 •

edited by AlekseyTs

Loading

333fred commented Dec 26, 2019 •

edited

Loading

333fred commented Dec 26, 2019 •

edited by AlekseyTs

Loading

333fred commented Dec 26, 2019 •

edited by AlekseyTs

Loading

gafter commented Dec 26, 2019 •

edited

Loading

gafter Dec 26, 2019 •

edited

Loading

gafter commented Dec 26, 2019 •

edited by AlekseyTs

Loading

gafter commented Dec 26, 2019 •

edited by AlekseyTs

Loading

gafter commented Dec 30, 2019 •

edited

Loading

333fred Dec 31, 2019 •

edited by AlekseyTs

Loading

333fred Dec 31, 2019 •

edited by AlekseyTs

Loading

333fred Dec 31, 2019 •

edited by AlekseyTs

Loading

333fred Dec 31, 2019 •

edited by AlekseyTs

Loading

333fred Dec 31, 2019 •

edited by AlekseyTs

Loading

333fred Dec 31, 2019 •

edited by AlekseyTs

Loading

333fred commented Dec 31, 2019 •

edited by AlekseyTs

Loading

333fred commented Dec 31, 2019 •

edited by AlekseyTs

Loading

333fred commented Dec 31, 2019 •

edited by AlekseyTs

Loading

333fred commented Dec 31, 2019 •

edited by AlekseyTs

Loading