Runtime capability to allow attribute editing by davidwengier · Pull Request #52986 · dotnet/roslyn

davidwengier · 2021-04-28T03:21:02Z

Adds a new runtime capability to control attribute edits. If the capability is not present, rude edits are reported. If the capability is present, semantic edits are reported.

This still needs more work done to only allow changing attributes that are stored in the CustomAttributes table.

tmat · 2021-05-08T17:21:25Z

Let's target main. I don't think we need this in 16.1x. If we need to we can then back port. In general we shouldn't make changes to -vs-deps unless absolutely necessary since it just makes harder to work in main.

tmat

🕐

…t them

davidwengier · 2021-06-03T11:55:31Z

@tmat Added the deny list for non-custom attributes, PTAL.

davidwengier · 2021-06-05T12:58:44Z

Merged and resolved conflicts. That was a pain :P
Still a couple of test failures I need to investigate in active statements, presumably something clashing between this PR and the line mapping PR just merged.

tmat · 2021-06-07T18:36:25Z

+                                continue;
+                            }
+
 #if TODO_ACCESSORS


TODO_ACCESSORS

Did I forget to delete this in my PR?

Yep. Did you mean to delete it? The linked issue is still unresolved so I left it just in case.

tmat · 2021-06-07T18:39:15Z

+
+            foreach (var parameter in newSymbol.GetParameters())
+            {
+                var oldParameter = oldSymbol?.GetParameters().SingleOrDefault(p => p.Name.Equals(parameter.Name));


SingleOrDefault

Is it possible there are parameters with duplicate names in error scenarios? We should not crash; just ignore such case.

tmat · 2021-06-07T18:41:06Z

+            if (newSymbol is INamedTypeSymbol or IFieldSymbol or IPropertySymbol)
+            {
+                var symbolKey = SymbolKey.Create(newSymbol, cancellationToken);
+                semanticEdits.Add(new SemanticEditInfo(SemanticEditKind.Update, symbolKey, syntaxMap, null, null));


null

nit: name null args

tmat · 2021-06-07T18:44:37Z

+
+            public int GetHashCode(KeyValuePair<string, TypedConstant> obj)
+                 => obj.GetHashCode();
+        }


Nit: let's move this code above the test accessor, e.g. to the Helpers region

tmat · 2021-06-07T18:44:53Z

+
+        #region Comparers
+
+        private class TypedConstantComparer : IEqualityComparer<TypedConstant>


class

nit: sealed

tmat · 2021-06-07T18:45:07Z

+                => obj.GetHashCode();
+        }
+
+        private class NamedArgumentComparer : IEqualityComparer<KeyValuePair<string, TypedConstant>>


private

nit: sealed

tmat · 2021-06-07T18:46:02Z

+        NewTypeDefinition = 1 << 4,
+
+        /// <summary>
+        /// Adding, updating and deleting of custom attributes (as distinct from attributes that effect metadata)


attributes that effect metadata

pseudo-custom attributes

tmat · 2021-06-07T18:46:56Z

+        /// <summary>
+        /// Adding, updating and deleting of custom attributes (as distinct from attributes that effect metadata)
+        /// </summary>
+        UpdateCustomAttributes = 1 << 5,


UpdateCustomAttributes

Perhaps ChangeCustomAttributes would be better since update might mean just updating existing values (this is how we usually use the word in EnC).

tmat · 2021-06-07T18:49:09Z

                    case SyntaxKind.AttributeList:
-                        ReportError(RudeEditKind.Insert);
+                        // To allow inserting of attributes we need to check if the inserted attribute
+                        // is a pseudo-custom attribute that CLR allows us to change, or if it is a compiler well-know attribute


allows

"does not allow"?

tmat

ghost added the Area-Interactive label Apr 28, 2021

davidwengier force-pushed the EnCAttributeEdits branch from 2eb4a43 to 5a39d3d Compare April 28, 2021 05:24

davidwengier changed the base branch from release/dev16.10 to release/dev16.10-vs-deps April 28, 2021 05:24

davidwengier force-pushed the EnCAttributeEdits branch from 881f943 to 2ef2257 Compare April 30, 2021 11:07

davidwengier marked this pull request as ready for review May 4, 2021 00:30

davidwengier requested a review from a team as a code owner May 4, 2021 00:30