diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs index 42ba6a4ad4214e4bf0c3d4fa3bf58f76ec1032f5..09aa09b6b97dfb435ef400d4a52ae47fc96e51ea 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs @@ -34,7 +34,7 @@ internal partial class Binder // Create a map from type parameter name to ordinal. // No need to report duplicate names since duplicates // are reported when the type parameters are bound. - var names = new Dictionary(n); + var names = new Dictionary(n, StringOrdinalComparer.Instance); foreach (var typeParameter in typeParameters) { var name = typeParameter.Name; diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs index 6f8ff506b8cd9b88dbad8b481b2f00a276cf8008..85cb52e70a51bf88a980dbc54d271601e341d2e1 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs @@ -8,6 +8,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; +using Microsoft.CodeAnalysis.Collections; /* SPEC: @@ -2224,122 +2225,129 @@ private bool Fix(int iParam, ref HashSet useSiteDiagnostics) return false; } - var candidates = new HashSet(); + var candidates = PooledHashSet.GetInstance(); + try + { - // Optimization: if we have one exact bound then we need not add any - // inexact bounds; we're just going to remove them anyway. + // Optimization: if we have one exact bound then we need not add any + // inexact bounds; we're just going to remove them anyway. - if (exact == null) - { - if (lower != null) + if (exact == null) { - foreach (var lowerBound in lower) + if (lower != null) { - candidates.Add(lowerBound); + foreach (var lowerBound in lower) + { + candidates.Add(lowerBound); + } } - } - if (upper != null) - { - foreach (var upperBound in upper) + if (upper != null) { - candidates.Add(upperBound); + foreach (var upperBound in upper) + { + candidates.Add(upperBound); + } } } - } - else - { - candidates.Add(exact.First()); - } + else + { + candidates.Add(exact.First()); + } - if (candidates.Count == 0) - { - return false; - } + if (candidates.Count == 0) + { + return false; + } - // Don't mutate the collection as we're iterating it. - var initialCandidates = candidates.ToList(); + // Don't mutate the collection as we're iterating it. + var initialCandidates = candidates.ToList(); - // SPEC: For each lower bound U of Xi all types to which there is not an - // SPEC: implicit conversion from U are removed from the candidate set. + // SPEC: For each lower bound U of Xi all types to which there is not an + // SPEC: implicit conversion from U are removed from the candidate set. - if (lower != null) - { - foreach (var bound in lower) + if (lower != null) { - // Make a copy; don't modify the collection as we're iterating it. - foreach (var candidate in initialCandidates) + foreach (var bound in lower) { - if (bound != candidate && !ImplicitConversionExists(bound, candidate, ref useSiteDiagnostics)) + // Make a copy; don't modify the collection as we're iterating it. + foreach (var candidate in initialCandidates) { - candidates.Remove(candidate); + if (bound != candidate && !ImplicitConversionExists(bound, candidate, ref useSiteDiagnostics)) + { + candidates.Remove(candidate); + } } } } - } - // SPEC: For each upper bound U of Xi all types from which there is not an - // SPEC: implicit conversion to U are removed from the candidate set. + // SPEC: For each upper bound U of Xi all types from which there is not an + // SPEC: implicit conversion to U are removed from the candidate set. - if (upper != null) - { - foreach (var bound in upper) + if (upper != null) { - foreach (var candidate in initialCandidates) + foreach (var bound in upper) { - if (bound != candidate && !ImplicitConversionExists(candidate, bound, ref useSiteDiagnostics)) + foreach (var candidate in initialCandidates) { - candidates.Remove(candidate); + if (bound != candidate && !ImplicitConversionExists(candidate, bound, ref useSiteDiagnostics)) + { + candidates.Remove(candidate); + } } } } - } - // SPEC: * If among the remaining candidate types there is a unique type V to - // SPEC: which there is an implicit conversion from all the other candidate - // SPEC: types, then the parameter is fixed to V. + // SPEC: * If among the remaining candidate types there is a unique type V to + // SPEC: which there is an implicit conversion from all the other candidate + // SPEC: types, then the parameter is fixed to V. - // SPEC: 4.7 The Dynamic Type - // Type inference (7.5.2) will prefer dynamic over object if both are candidates. - // This rule doesn't have to be implemented explicitly due to special handling of - // conversions from dynamic in ImplicitConversionExists helper. + // SPEC: 4.7 The Dynamic Type + // Type inference (7.5.2) will prefer dynamic over object if both are candidates. + // This rule doesn't have to be implemented explicitly due to special handling of + // conversions from dynamic in ImplicitConversionExists helper. - TypeSymbol best = null; - foreach (var candidate in candidates) - { - foreach (var candidate2 in candidates) + TypeSymbol best = null; + foreach (var candidate in candidates) { - if (candidate != candidate2 && !ImplicitConversionExists(candidate2, candidate, ref useSiteDiagnostics)) + foreach (var candidate2 in candidates) { - goto OuterBreak; + if (candidate != candidate2 && !ImplicitConversionExists(candidate2, candidate, ref useSiteDiagnostics)) + { + goto OuterBreak; + } + } + + if ((object)best == null) + { + best = candidate; } + else if (!(best.IsDynamic() && candidate.IsObjectType())) + { + Debug.Assert(!(best.IsObjectType() && candidate.IsDynamic())); + Debug.Assert(!(best.IsDynamic() && candidate.IsObjectType())); + + // best candidate is not unique + return false; + } + + OuterBreak: + ; } if ((object)best == null) { - best = candidate; - } - else if (!(best.IsDynamic() && candidate.IsObjectType())) - { - Debug.Assert(!(best.IsObjectType() && candidate.IsDynamic())); - Debug.Assert(!(best.IsDynamic() && candidate.IsObjectType())); - - // best candidate is not unique + // no best candidate return false; } - OuterBreak: - ; + _fixedResults[iParam] = best; + UpdateDependenciesAfterFix(iParam); + return true; } - - if ((object)best == null) + finally { - // no best candidate - return false; + candidates.Free(); } - - _fixedResults[iParam] = best; - UpdateDependenciesAfterFix(iParam); - return true; } private bool ImplicitConversionExists(TypeSymbol source, TypeSymbol destination, ref HashSet useSiteDiagnostics) diff --git a/src/Compilers/CSharp/Portable/Declarations/MergedNamespaceDeclaration.cs b/src/Compilers/CSharp/Portable/Declarations/MergedNamespaceDeclaration.cs index b7c4bb8df7f05c7a676acb8c20e38d111f2fa479..fbef92d86b98359f5aad20abb321479ba8ad543a 100644 --- a/src/Compilers/CSharp/Portable/Declarations/MergedNamespaceDeclaration.cs +++ b/src/Compilers/CSharp/Portable/Declarations/MergedNamespaceDeclaration.cs @@ -2,6 +2,7 @@ using System.Collections.Immutable; using Microsoft.CodeAnalysis.CSharp.Symbols; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp { @@ -148,7 +149,7 @@ private ImmutableArray MakeChildren() } else { - var namespaceGroups = namespaces.ToDictionary(n => n.Name); + var namespaceGroups = namespaces.ToDictionary(n => n.Name, StringOrdinalComparer.Instance); namespaces.Free(); foreach (var namespaceGroup in namespaceGroups.Values) diff --git a/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs b/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs index 931ad582ea633e46d43676ddbec32aa542f65b15..b748b80b106be50c940316f60f5e2dc3806a8d76 100644 --- a/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs +++ b/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs @@ -16,8 +16,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Emit { internal sealed class CSharpSymbolMatcher : SymbolMatcher { - private static readonly StringComparer s_nameComparer = StringComparer.Ordinal; - private readonly MatchDefs _defs; private readonly MatchSymbols _symbols; @@ -113,7 +111,7 @@ private Cci.IDefinition VisitDefInternal(Cci.IDefinition def) return null; } - return this.VisitTypeMembers(otherContainer, nestedType, GetNestedTypes, (a, b) => s_nameComparer.Equals(a.Name, b.Name)); + return this.VisitTypeMembers(otherContainer, nestedType, GetNestedTypes, (a, b) => StringOrdinalComparer.Equals(a.Name, b.Name)); } var member = def as Cci.ITypeDefinitionMember; @@ -128,7 +126,7 @@ private Cci.IDefinition VisitDefInternal(Cci.IDefinition def) var field = def as Cci.IFieldDefinition; if (field != null) { - return this.VisitTypeMembers(otherContainer, field, GetFields, (a, b) => s_nameComparer.Equals(a.Name, b.Name)); + return this.VisitTypeMembers(otherContainer, field, GetFields, (a, b) => StringOrdinalComparer.Equals(a.Name, b.Name)); } } @@ -161,7 +159,7 @@ private Cci.INamespaceTypeDefinition VisitNamespaceType(Cci.INamespaceTypeDefini { if (_lazyTopLevelTypes == null) { - var typesByName = new Dictionary(s_nameComparer); + var typesByName = new Dictionary(StringOrdinalComparer.Instance); foreach (var type in this.GetTopLevelTypes()) { // All generated top-level types are assumed to be in the global namespace. @@ -667,19 +665,19 @@ private bool AreArrayTypesEqual(ArrayTypeSymbol type, ArrayTypeSymbol other) private bool AreEventsEqual(EventSymbol @event, EventSymbol other) { - Debug.Assert(s_nameComparer.Equals(@event.Name, other.Name)); + Debug.Assert(StringOrdinalComparer.Equals(@event.Name, other.Name)); return _comparer.Equals(@event.Type, other.Type); } private bool AreFieldsEqual(FieldSymbol field, FieldSymbol other) { - Debug.Assert(s_nameComparer.Equals(field.Name, other.Name)); + Debug.Assert(StringOrdinalComparer.Equals(field.Name, other.Name)); return _comparer.Equals(field.Type, other.Type); } private bool AreMethodsEqual(MethodSymbol method, MethodSymbol other) { - Debug.Assert(s_nameComparer.Equals(method.Name, other.Name)); + Debug.Assert(StringOrdinalComparer.Equals(method.Name, other.Name)); Debug.Assert(method.IsDefinition); Debug.Assert(other.IsDefinition); @@ -708,7 +706,7 @@ private static MethodSymbol SubstituteTypeParameters(MethodSymbol method) private bool AreNamedTypesEqual(NamedTypeSymbol type, NamedTypeSymbol other) { - Debug.Assert(s_nameComparer.Equals(type.Name, other.Name)); + Debug.Assert(StringOrdinalComparer.Equals(type.Name, other.Name)); // TODO: Test with overloads (from PE base class?) that have modifiers. Debug.Assert(!type.HasTypeArgumentsCustomModifiers); Debug.Assert(!other.HasTypeArgumentsCustomModifiers); @@ -718,7 +716,7 @@ private bool AreNamedTypesEqual(NamedTypeSymbol type, NamedTypeSymbol other) private bool AreParametersEqual(ParameterSymbol parameter, ParameterSymbol other) { Debug.Assert(parameter.Ordinal == other.Ordinal); - return s_nameComparer.Equals(parameter.Name, other.Name) && + return StringOrdinalComparer.Equals(parameter.Name, other.Name) && (parameter.RefKind == other.RefKind) && _comparer.Equals(parameter.Type, other.Type); } @@ -734,7 +732,7 @@ private bool ArePointerTypesEqual(PointerTypeSymbol type, PointerTypeSymbol othe private bool ArePropertiesEqual(PropertySymbol property, PropertySymbol other) { - Debug.Assert(s_nameComparer.Equals(property.Name, other.Name)); + Debug.Assert(StringOrdinalComparer.Equals(property.Name, other.Name)); return _comparer.Equals(property.Type, other.Type) && property.Parameters.SequenceEqual(other.Parameters, AreParametersEqual); } @@ -742,7 +740,7 @@ private bool ArePropertiesEqual(PropertySymbol property, PropertySymbol other) private bool AreTypeParametersEqual(TypeParameterSymbol type, TypeParameterSymbol other) { Debug.Assert(type.Ordinal == other.Ordinal); - Debug.Assert(s_nameComparer.Equals(type.Name, other.Name)); + Debug.Assert(StringOrdinalComparer.Equals(type.Name, other.Name)); // Comparing constraints is unnecessary: two methods cannot differ by // constraints alone and changing the signature of a method is a rude // edit. Furthermore, comparing constraint types might lead to a cycle. @@ -796,7 +794,7 @@ private bool AreTypesEqual(TypeSymbol type, TypeSymbol other) members.AddRange(synthesizedMembers); } - var result = members.ToDictionary(s => ((Symbol)s).Name, s_nameComparer); + var result = members.ToDictionary(s => ((Symbol)s).Name, StringOrdinalComparer.Instance); members.Free(); return result; } diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/NamedTypeSymbolAdapter.cs b/src/Compilers/CSharp/Portable/Emitter/Model/NamedTypeSymbolAdapter.cs index cc2d0c5bed7f0a002d7a096f5b7d4c9282752eca..df07b66320ffe173f518f384da9e42ab914d872c 100644 --- a/src/Compilers/CSharp/Portable/Emitter/Model/NamedTypeSymbolAdapter.cs +++ b/src/Compilers/CSharp/Portable/Emitter/Model/NamedTypeSymbolAdapter.cs @@ -826,8 +826,9 @@ string Cci.INamespaceTypeReference.NamespaceName // INamespaceTypeReference is a type contained in a namespace // if this method is called for a nested type, we are in big trouble. Debug.Assert(((Cci.ITypeReference)this).AsNamespaceTypeReference != null); - return this.ContainingSymbol.ToDisplayString(SymbolDisplayFormat.QualifiedNameOnlyFormat); - } + + return this.ContainingNamespace.QualifiedName; + } } bool Cci.INamespaceTypeDefinition.IsPublic diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs b/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs index 9e69df61ea69d510cb83ee6e2d3d4b4a9843fbfb..ad87de06b9f08290c2998f5ce8500e88fc7f7252 100644 --- a/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs +++ b/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs @@ -472,7 +472,7 @@ public override IEnumerable GetExportedTypes(EmitContext con if (_lazyExportedTypes.Length > 0) { - var exportedNamesMap = new Dictionary(); + var exportedNamesMap = new Dictionary(StringOrdinalComparer.Instance); // Report name collisions. foreach (var exportedType in _lazyExportedTypes) diff --git a/src/Compilers/CSharp/Portable/Parser/DocumentationCommentParser.cs b/src/Compilers/CSharp/Portable/Parser/DocumentationCommentParser.cs index 5ab38f0930598c777a3b9ab5cb1765e4c597fecc..3e1ca4b9f5a8e3de0a2083c3d295443bcdf0eb5b 100644 --- a/src/Compilers/CSharp/Portable/Parser/DocumentationCommentParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/DocumentationCommentParser.cs @@ -244,7 +244,7 @@ private XmlNodeSyntax ParseXmlElement() endName = this.WithXmlParseError(endName, XmlParseErrorCode.XML_InvalidWhitespace); } - if (!endName.IsMissing && name.ToString() != endName.ToString()) + if (!endName.IsMissing && !MatchingXmlNames(name, endName)) { endName = this.WithXmlParseError(endName, XmlParseErrorCode.XML_ElementTypeMatch, endName.ToString(), name.ToString()); } @@ -288,6 +288,30 @@ private XmlNodeSyntax ParseXmlElement() _pool.Free(attrs); } } + + private static bool MatchingXmlNames(XmlNameSyntax name, XmlNameSyntax endName) + { + // PERF: because of deduplication we often get the same name for name and endName, + // so we will check for such case first before materializing text for entire nodes + // and comparing that. + if (name == endName) + { + return true; + } + + // before doing ToString, check if + // all nodes contributing to ToString are recursively the same + // NOTE: leading and trailing trivia do not contribute to ToString + if (!name.HasLeadingTrivia && + !endName.HasTrailingTrivia && + name.IsEquivalentTo(endName)) + { + return true; + } + + return name.ToString() == endName.ToString(); + } + // assuming this is not used concurrently private readonly HashSet _attributesSeen = new HashSet(); diff --git a/src/Compilers/CSharp/Portable/Parser/QuickScanner.cs b/src/Compilers/CSharp/Portable/Parser/QuickScanner.cs index 4dde5a47e7eb2631d7ae0dff1fec2d4525dcf93f..016232d811dd87288cb5e4a6dcb044511bcff3de 100644 --- a/src/Compilers/CSharp/Portable/Parser/QuickScanner.cs +++ b/src/Compilers/CSharp/Portable/Parser/QuickScanner.cs @@ -28,8 +28,11 @@ private enum QuickScanState : byte Dot, CompoundPunctStart, DoneAfterNext, + // we are relying on Bad state immediately following Done + // to be able to detect exiting conditions in one "state >= Done" test. + // And we are also relying on this to be the last item in the enum. Done, - Bad + Bad = Done + 1 } private enum CharFlags : byte @@ -209,7 +212,13 @@ private SyntaxToken QuickScanSyntaxToken() var flags = uc < charPropLength ? (CharFlags)s_charProperties[uc] : CharFlags.Complex; state = (QuickScanState)s_stateTransitions[(int)state, (int)flags]; - if (state == QuickScanState.Done || state == QuickScanState.Bad) + // NOTE: that Bad > Done and it is the only state like that + // as a result, we will exit the loop on either Bad or Done. + // the assert below will validate that these are the only states on which we exit + // Also note that we must exit on Done or Bad + // since the state machine does not have transitions for these states + // and will promptly fail if we do not exit. + if (state >= QuickScanState.Done) { goto exitWhile; } @@ -221,7 +230,7 @@ private SyntaxToken QuickScanSyntaxToken() exitWhile: TextWindow.AdvanceChar(i - TextWindow.Offset); - Debug.Assert(state == QuickScanState.Bad || state == QuickScanState.Done); + Debug.Assert(state == QuickScanState.Bad || state == QuickScanState.Done, "can only exit with Bad or Done"); if (state == QuickScanState.Done) { diff --git a/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs b/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs index 5ce1bb3be6ee60f9e917a438be02f585a20ed631..dc9c1ad3f1c64b28ead5d9b0a5fe8cd858d9c79f 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs @@ -8,6 +8,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; +using Microsoft.CodeAnalysis.Collections; namespace Microsoft.CodeAnalysis.CSharp.Symbols { @@ -496,16 +497,18 @@ private static bool HasDuplicateInterfaces(NamedTypeSymbol type, ConsList(ReferenceEqualityComparer.Instance); + var set = PooledHashSet.GetInstance(); foreach (var i in array) { if (!set.Add(i.OriginalDefinition)) { + set.Free(); goto hasRelatedInterfaces; } } // all interfaces are unrelated + set.Free(); return false; } diff --git a/src/Compilers/CSharp/Portable/Symbols/LabelSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/LabelSymbol.cs index 76330056758439ac2e768f52dd5c575cf360df94..7ce2df69bd6771434c92e30a98291968dae380cd 100644 --- a/src/Compilers/CSharp/Portable/Symbols/LabelSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/LabelSymbol.cs @@ -13,24 +13,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols /// internal abstract class LabelSymbol : Symbol, ILabelSymbol { - internal LabelSymbol(string name) - { - _name = name; - } - - private readonly string _name; - - /// - /// Gets the name of this label - /// - public override string Name - { - get - { - return _name; - } - } - /// /// Returns false because label can't be defined externally. /// diff --git a/src/Compilers/CSharp/Portable/Symbols/MergedNamespaceSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/MergedNamespaceSymbol.cs index 915deb280e15520e9ed905ab23937e788446f520..8a5677639ee5c77125b9f839f567efd46d02f3d9 100644 --- a/src/Compilers/CSharp/Portable/Symbols/MergedNamespaceSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/MergedNamespaceSymbol.cs @@ -18,7 +18,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols /// namespaces. Any sub-namespaces with the same names are also merged if they have two or more /// instances. /// - /// Merged namespaces are used to merged the symbols from multiple metadata modules and the + /// Merged namespaces are used to merge the symbols from multiple metadata modules and the /// source "module" into a single symbol tree that represents all the available symbols. The /// compiler resolves names against this merged set of symbols. /// diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs index 56a9cdd41b19c84861ce834bba245b3f7268b04b..315b92623789930ba03cc1179873f95edb05f8a8 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs @@ -22,7 +22,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE /// internal abstract class PENamedTypeSymbol : NamedTypeSymbol { - private static readonly Dictionary> s_emptyNestedTypes = new Dictionary>(); + private static readonly Dictionary> s_emptyNestedTypes = new Dictionary>(EmptyComparer.Instance); private readonly NamespaceOrTypeSymbol _container; private readonly TypeDefinitionHandle _handle; @@ -1585,10 +1585,10 @@ public override TypeKind TypeKind { get { - if (_lazyKind == TypeKind.Unknown) - { - TypeKind result; + TypeKind result = _lazyKind; + if (result == TypeKind.Unknown) + { if (_flags.IsInterface()) { result = TypeKind.Interface; @@ -1603,22 +1603,26 @@ public override TypeKind TypeKind { SpecialType baseCorTypeId = @base.SpecialType; - // Code is cloned from MetaImport::DoImportBaseAndImplements() - if (baseCorTypeId == SpecialType.System_Enum) - { - // Enum - result = TypeKind.Enum; - } - else if (baseCorTypeId == SpecialType.System_MulticastDelegate) + switch (baseCorTypeId) { - // Delegate - result = TypeKind.Delegate; - } - else if (baseCorTypeId == SpecialType.System_ValueType && - this.SpecialType != SpecialType.System_Enum) - { - // Struct - result = TypeKind.Struct; + case SpecialType.System_Enum: + // Enum + result = TypeKind.Enum; + break; + + case SpecialType.System_MulticastDelegate: + // Delegate + result = TypeKind.Delegate; + break; + + case SpecialType.System_ValueType: + // System.Enum is the only class that derives from ValueType + if (this.SpecialType != SpecialType.System_Enum) + { + // Struct + result = TypeKind.Struct; + } + break; } } } @@ -1626,7 +1630,7 @@ public override TypeKind TypeKind _lazyKind = result; } - return _lazyKind; + return result; } } @@ -1873,7 +1877,7 @@ private PEMethodSymbol GetAccessorMethod(PEModule module, Dictionary> GroupByName(ArrayBuilder symbols) { - return symbols.ToDictionary(s => s.Name); + return symbols.ToDictionary(s => s.Name, StringOrdinalComparer.Instance); } private static Dictionary> GroupByName(ArrayBuilder symbols) @@ -1883,7 +1887,7 @@ private PEMethodSymbol GetAccessorMethod(PEModule module, Dictionary s.Name); + return symbols.ToDictionary(s => s.Name, StringOrdinalComparer.Instance); } diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamespaceSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamespaceSymbol.cs index 55b49fd21f64a5aadb54dc988da47468879bdae8..a31b9818028f7b0e200bb642455ccd6dd435d268 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamespaceSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamespaceSymbol.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Roslyn.Utilities; using System; using System.Collections.Generic; using System.Collections.Immutable; @@ -180,7 +181,7 @@ protected void LoadAllMembers(IEnumerable /// Create symbols for nested namespaces and initialize namespaces map. /// @@ -201,7 +217,7 @@ protected void LoadAllMembers(IEnumerable(); + var namespaces = new Dictionary(StringOrdinalComparer.Instance); foreach (var c in children) { @@ -241,7 +257,7 @@ private void LazyInitializeTypes(IEnumerable(); + noPiaLocalTypes = new Dictionary(StringOrdinalComparer.Instance); } noPiaLocalTypes[typeDefName] = t; @@ -252,7 +268,7 @@ private void LazyInitializeTypes(IEnumerable c.Name); + var typesDict = children.ToDictionary(c => c.Name, StringOrdinalComparer.Instance); children.Free(); if (noPiaLocalTypes != null) diff --git a/src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs index fc12d0c14b4aa48c44285116b2ef6662bfab791b..d418e5abc5694383848a52ae45907aba80e63b73 100644 --- a/src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs @@ -364,21 +364,26 @@ internal void GetExtensionMethods(ArrayBuilder methods, string nam { if (this.MightContainExtensionMethods) { - var members = nameOpt == null - ? this.GetMembersUnordered() - : this.GetSimpleNonTypeMembers(nameOpt); + DoGetExtensionMethods(methods, nameOpt, arity, options); + } + } - foreach (var member in members) + internal void DoGetExtensionMethods(ArrayBuilder methods, string nameOpt, int arity, LookupOptions options) + { + var members = nameOpt == null + ? this.GetMembersUnordered() + : this.GetSimpleNonTypeMembers(nameOpt); + + foreach (var member in members) + { + if (member.Kind == SymbolKind.Method) { - if (member.Kind == SymbolKind.Method) + var method = (MethodSymbol)member; + if (method.IsExtensionMethod && + ((options & LookupOptions.AllMethodsOnArityZero) != 0 || arity == method.Arity)) { - var method = (MethodSymbol)member; - if (method.IsExtensionMethod && - ((options & LookupOptions.AllMethodsOnArityZero) != 0 || arity == method.Arity)) - { - Debug.Assert(method.MethodKind != MethodKind.ReducedExtension); - methods.Add(method); - } + Debug.Assert(method.MethodKind != MethodKind.ReducedExtension); + methods.Add(method); } } } diff --git a/src/Compilers/CSharp/Portable/Symbols/NamespaceSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/NamespaceSymbol.cs index 700cec7e02b99ecb22f1ffc46448b1201656a1cb..26a40e605be3a329670c285a221e99141ddf0cce 100644 --- a/src/Compilers/CSharp/Portable/Symbols/NamespaceSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/NamespaceSymbol.cs @@ -12,6 +12,10 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols /// internal abstract partial class NamespaceSymbol : NamespaceOrTypeSymbol, INamespaceSymbol { + // PERF: initialization of the following fields will allocate, so we make them lazy + private ImmutableArray _lazyTypesMightContainExtensionMethods; + private string _lazyQualifiedName; + // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! // Changes to the public interface of this class should remain synchronized with the VB version. // Do not make any changes to the public interface without making the corresponding change @@ -311,6 +315,22 @@ internal NamespaceSymbol GetNestedNamespace(NameSyntax name) return null; } + private ImmutableArray TypesMightContainExtensionMethods + { + get + { + var typesWithExtensionMethods = this._lazyTypesMightContainExtensionMethods; + if (typesWithExtensionMethods.IsDefault) + { + this._lazyTypesMightContainExtensionMethods = this.GetTypeMembersUnordered().WhereAsArray(t => t.MightContainExtensionMethods); + typesWithExtensionMethods = this._lazyTypesMightContainExtensionMethods; + } + + return typesWithExtensionMethods; + } + } + + /// /// Add all extension methods in this namespace to the given list. If name or arity /// or both are provided, only those extension methods that match are included. @@ -332,10 +352,11 @@ internal virtual void GetExtensionMethods(ArrayBuilder methods, st return; } - var types = this.GetTypeMembersUnordered(); - foreach (var type in types) + var typesWithExtensionMethods = this.TypesMightContainExtensionMethods; + + foreach (var type in typesWithExtensionMethods) { - type.GetExtensionMethods(methods, nameOpt, arity, options); + type.DoGetExtensionMethods(methods, nameOpt, arity, options); } } @@ -377,6 +398,15 @@ ImmutableArray INamespaceSymbol.ConstituentNamespaces } } + internal string QualifiedName + { + get + { + return _lazyQualifiedName ?? + (_lazyQualifiedName = this.ToDisplayString(SymbolDisplayFormat.QualifiedNameOnlyFormat)); + } + } + #endregion #region ISymbol Members diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceAssemblySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceAssemblySymbol.cs index 3a541bf135ca784ac3ae896225791ed7d68ca4ed..8f8f542af858de6a5934a0780911c3ded2520ad1 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceAssemblySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceAssemblySymbol.cs @@ -2521,7 +2521,7 @@ internal override NamedTypeSymbol TryLookupForwardedMetadataTypeWithCycleDetecti if (wellKnownAttributeData != null && wellKnownAttributeData.ForwardedTypes != null) { - forwardedTypesFromSource = new Dictionary(); + forwardedTypesFromSource = new Dictionary(StringOrdinalComparer.Instance); foreach (NamedTypeSymbol forwardedType in wellKnownAttributeData.ForwardedTypes) { diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceLabelSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceLabelSymbol.cs index 5f25a0ec37e0a5b49b00fb14f76d2105f58e7b73..a4cc96fcc92f82644ec6646d83fea7f7a75eabbb 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceLabelSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceLabelSymbol.cs @@ -17,21 +17,48 @@ internal sealed class SourceLabelSymbol : LabelSymbol /// private readonly ConstantValue _switchCaseLabelConstant; + // PERF: Often we do not need this, so we make this lazy + private string _lazyName; + public SourceLabelSymbol( MethodSymbol containingMethod, SyntaxNodeOrToken identifierNodeOrToken, ConstantValue switchCaseLabelConstant = null) - : base(identifierNodeOrToken.IsToken ? identifierNodeOrToken.AsToken().ValueText : identifierNodeOrToken.ToString()) { _containingMethod = containingMethod; _identifierNodeOrToken = identifierNodeOrToken; _switchCaseLabelConstant = switchCaseLabelConstant; } + public override string Name + { + get + { + return _lazyName ?? + (_lazyName = MakeLabelName()); + } + } + + private string MakeLabelName() + { + var node = _identifierNodeOrToken.AsNode(); + if (node != null) + { + return node.ToString(); + } + + var tk = _identifierNodeOrToken.AsToken(); + if (tk.Kind() != SyntaxKind.None) + { + return tk.ValueText; + } + + return _switchCaseLabelConstant?.ToString() ?? ""; + } + public SourceLabelSymbol( MethodSymbol containingMethod, - ConstantValue switchCaseLabelConstant = null) - : base(switchCaseLabelConstant.ToString()) + ConstantValue switchCaseLabelConstant) { _containingMethod = containingMethod; _identifierNodeOrToken = default(SyntaxToken); diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs index 51b522b959fed1c0ef2885193e095e7f8339b92b..f8099181986b281d3434087fb8cdca405bea0961 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Runtime.InteropServices; using System.Threading; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Collections; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -154,7 +155,7 @@ public void SetIsManagedType(bool isManagedType) private Dictionary> _lazyMembersDictionary; private Dictionary> _lazyEarlyAttributeDecodingMembersDictionary; - private static readonly Dictionary> s_emptyTypeMembers = new Dictionary>(); + private static readonly Dictionary> s_emptyTypeMembers = new Dictionary>(EmptyComparer.Instance); private Dictionary> _lazyTypeMembers; private ImmutableArray _lazyMembersFlattened; private ImmutableArray _lazySynthesizedExplicitImplementations; @@ -1080,7 +1081,9 @@ public override ImmutableArray GetTypeMembers(string name, int } Debug.Assert(s_emptyTypeMembers.Count == 0); - return symbols.Count > 0 ? symbols.ToDictionary(s => s.Name) : s_emptyTypeMembers; + return symbols.Count > 0 ? + symbols.ToDictionary(s => s.Name, StringOrdinalComparer.Instance) : + s_emptyTypeMembers; } finally { @@ -1227,12 +1230,7 @@ internal override ImmutableArray GetEarlyAttributeDecodingMembers(string // NOTE: members were added in a single pass over the syntax, so they're already // in lexical order. - // TODO: Can we move ToDictionary() off ArrayBuilder so that we don't need a temp here? - var temp = ArrayBuilder.GetInstance(); - temp.AddRange(membersAndInitializers.NonTypeNonIndexerMembers); - var membersByName = temp.ToDictionary(s => s.Name); - temp.Free(); - + var membersByName = membersAndInitializers.NonTypeNonIndexerMembers.ToDictionary(s => s.Name); AddNestedTypesToDictionary(membersByName, GetTypeMembersDictionary()); Interlocked.CompareExchange(ref _lazyEarlyAttributeDecodingMembersDictionary, membersByName, null); @@ -2168,7 +2166,7 @@ private static bool ContainsModifier(SyntaxTokenList modifiers, SyntaxKind modif merged.Add(indexerMembers[indexerPos]); } - var membersByName = merged.ToDictionary(s => s.Name); + var membersByName = merged.ToDictionary(s => s.Name, StringOrdinalComparer.Instance); merged.Free(); return membersByName; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceNamespaceSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceNamespaceSymbol.cs index 1382f51ab5133b6b620a3de3acf28f8fa30c3ce0..3b8e5222fb1b2afff4e811a6b9741358b0c84b0c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceNamespaceSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceNamespaceSymbol.cs @@ -248,7 +248,7 @@ internal override NamespaceExtent Extent // NOTE: This method depends on MakeNameToMembersMap() on creating a proper // NOTE: type of the array, see comments in MakeNameToMembersMap() for details - var dictionary = new Dictionary>(); + var dictionary = new Dictionary>(StringOrdinalComparer.Instance); Dictionary> map = this.GetNameToMembersMap(); foreach (var kvp in map) @@ -484,7 +484,7 @@ private struct NameToSymbolMapBuilder public NameToSymbolMapBuilder(int capacity) { - _dictionary = new Dictionary(capacity); + _dictionary = new Dictionary(capacity, StringOrdinalComparer.Instance); } public void Add(NamespaceOrTypeSymbol symbol) @@ -510,7 +510,7 @@ public void Add(NamespaceOrTypeSymbol symbol) public Dictionary> CreateMap() { - var result = new Dictionary>(_dictionary.Count); + var result = new Dictionary>(_dictionary.Count, StringOrdinalComparer.Instance); foreach (var kvp in _dictionary) { diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedLabelSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedLabelSymbol.cs index 657447e6c16bd2cacdd041e3c00225de15b1bdbf..4479732b239b177ff8ad58b386dcce7c2e627637 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedLabelSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedLabelSymbol.cs @@ -9,9 +9,19 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols { internal sealed class GeneratedLabelSymbol : LabelSymbol { + private readonly string _name; + public GeneratedLabelSymbol(string name) - : base(LabelName(name)) { + _name = LabelName(name); + } + + public override string Name + { + get + { + return _name; + } } #if DEBUG diff --git a/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxNode.cs b/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxNode.cs index 54e77cc50fa18124a7c460b9ec29ef99bd6b6dcc..0f0f9af4df424204a6a3abfc38605f3c3e17633b 100644 --- a/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxNode.cs +++ b/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxNode.cs @@ -68,13 +68,13 @@ internal new SyntaxTree SyntaxTree { get { - ComputeSyntaxTree(this); - Debug.Assert(this._syntaxTree != null); - return this._syntaxTree; + var result = this._syntaxTree ?? ComputeSyntaxTree(this); + Debug.Assert(result != null); + return result; } } - private static void ComputeSyntaxTree(CSharpSyntaxNode node) + private static SyntaxTree ComputeSyntaxTree(CSharpSyntaxNode node) { ArrayBuilder nodes = null; SyntaxTree tree = null; @@ -91,21 +91,21 @@ private static void ComputeSyntaxTree(CSharpSyntaxNode node) var parent = node.Parent; if (parent == null) { + // set the tree on the root node atomically Interlocked.CompareExchange(ref node._syntaxTree, CSharpSyntaxTree.CreateWithoutClone(node), null); tree = node._syntaxTree; break; } - else if (parent._syntaxTree != null) + + tree = parent._syntaxTree; + if (tree != null) { - Interlocked.CompareExchange(ref node._syntaxTree, parent._syntaxTree, null); - tree = node._syntaxTree; + node._syntaxTree = tree; break; } - else - { - (nodes ?? (nodes = ArrayBuilder.GetInstance())).Add(node); - node = parent; - } + + (nodes ?? (nodes = ArrayBuilder.GetInstance())).Add(node); + node = parent; } // Propagate the syntax tree downwards if necessary @@ -115,15 +115,21 @@ private static void ComputeSyntaxTree(CSharpSyntaxNode node) foreach (var n in nodes) { - var existingTree = Interlocked.CompareExchange(ref n._syntaxTree, tree, null); + var existingTree = n._syntaxTree; if (existingTree != null) { - tree = existingTree; + Debug.Assert(existingTree == tree, "how could this node belong to a different tree?"); + + // yield the race + break; } + n._syntaxTree = tree; } nodes.Free(); } + + return tree; } public abstract TResult Accept(CSharpSyntaxVisitor visitor); @@ -262,7 +268,7 @@ public override void WriteTo(System.IO.TextWriter writer) this.Green.WriteTo(writer, true, true); } - #region serialization +#region serialization private static readonly RecordingObjectBinder s_defaultBinder = new ConcurrentRecordingObjectBinder(); @@ -393,7 +399,7 @@ private static IEnumerable GetSerializationData() return s_serializationData; } - #endregion +#endregion /// /// Determines whether this node is structurally equivalent to another. @@ -451,7 +457,7 @@ internal override SyntaxNode GetLambda() return LambdaUtilities.GetLambda(this); } - #region Directives +#region Directives internal IList GetDirectives(Func filter = null) { @@ -538,9 +544,9 @@ public DirectiveTriviaSyntax GetLastDirective(Func return null; } - #endregion +#endregion - #region Node Lookup +#region Node Lookup /// /// Returns child node or token that contains given position. @@ -558,9 +564,9 @@ public override SyntaxNodeOrToken ChildThatContainsPosition(int position) Debug.Assert(childNodeOrToken.FullSpan.Contains(position), "ChildThatContainsPosition's return value does not contain the requested position."); return childNodeOrToken; } - #endregion +#endregion - #region Token Lookup +#region Token Lookup /// /// Gets the first token of the tree rooted by this node. @@ -664,9 +670,9 @@ internal SyntaxToken FindTokenIncludingCrefAndNameAttributes(int position) return nonTriviaToken; } - #endregion +#endregion - #region Trivia Lookup +#region Trivia Lookup /// /// Finds a descendant trivia of this node at the specified position, where the position is @@ -693,9 +699,9 @@ public new SyntaxTrivia FindTrivia(int position, bool findInsideTrivia = false) return base.FindTrivia(position, findInsideTrivia); } - #endregion +#endregion - #region SyntaxNode members +#region SyntaxNode members /// /// Determine if this node is structurally equivalent to another. @@ -771,6 +777,6 @@ protected override bool IsEquivalentToCore(SyntaxNode node, bool topLevel = fals return SyntaxFactory.AreEquivalent(this, (CSharpSyntaxNode)node, topLevel); } - #endregion +#endregion } } diff --git a/src/Compilers/CSharp/Test/Syntax/LexicalAndXml/XmlDocCommentTests.cs b/src/Compilers/CSharp/Test/Syntax/LexicalAndXml/XmlDocCommentTests.cs index ff640426e5deaaed6d3292e9eea26bb7a8a9b0cc..4548dcef9a99b05dc25008a5a19d56aff0d02dc7 100644 --- a/src/Compilers/CSharp/Test/Syntax/LexicalAndXml/XmlDocCommentTests.cs +++ b/src/Compilers/CSharp/Test/Syntax/LexicalAndXml/XmlDocCommentTests.cs @@ -2862,6 +2862,40 @@ public class Program Diagnostic(ErrorCode.WRN_XMLParseError, " ")); } + [Fact] + public void WhitespaceInXmlEndName() + { + var text = @" +/// +/// good +/// +/// +/// bad +/// +/// +/// bad +/// +public class Program +{ +}"; + + var tree = Parse(text); + tree.GetDiagnostics().Verify( + // (7,7): warning CS1570: XML comment has badly formed XML -- 'End tag 'A :B' does not match the start tag 'A:B'.' + // /// + Diagnostic(ErrorCode.WRN_XMLParseError, "A :B").WithArguments("A :B", "A:B").WithLocation(7, 7), + // (7,8): warning CS1570: XML comment has badly formed XML -- 'Whitespace is not allowed at this location.' + // /// + Diagnostic(ErrorCode.WRN_XMLParseError, " ").WithLocation(7, 8), + // (10,7): warning CS1570: XML comment has badly formed XML -- 'End tag 'A: B' does not match the start tag 'A:B'.' + // /// + Diagnostic(ErrorCode.WRN_XMLParseError, "A: B").WithArguments("A: B", "A:B").WithLocation(10, 7), + // (10,9): warning CS1570: XML comment has badly formed XML -- 'Whitespace is not allowed at this location.' + // /// + Diagnostic(ErrorCode.WRN_XMLParseError, " ").WithLocation(10, 9) + ); + } + [Fact] [Trait("Feature", "Xml Documentation Comments")] public void TestDocumentationComment() diff --git a/src/Compilers/Core/Portable/CodeAnalysis.csproj b/src/Compilers/Core/Portable/CodeAnalysis.csproj index 903049ab0d6ab6a51ad0e10568550e97deb48b7d..c69d0156c033e61aa7deb740a1ba44148dd01b34 100644 --- a/src/Compilers/Core/Portable/CodeAnalysis.csproj +++ b/src/Compilers/Core/Portable/CodeAnalysis.csproj @@ -62,6 +62,8 @@ + + diff --git a/src/Compilers/Core/Portable/Collections/ImmutableArrayExtensions.cs b/src/Compilers/Core/Portable/Collections/ImmutableArrayExtensions.cs index 3ea15127f6e3cf8adf7b990ce12d8199a577349a..1ded99974e36360c2cd2806847a6f8312029f95b 100644 --- a/src/Compilers/Core/Portable/Collections/ImmutableArrayExtensions.cs +++ b/src/Compilers/Core/Portable/Collections/ImmutableArrayExtensions.cs @@ -456,5 +456,49 @@ public static int Count(this ImmutableArray items, Func predicate return count; } + + internal static Dictionary> ToDictionary(this ImmutableArray items, Func keySelector, IEqualityComparer comparer = null) + { + if (items.Length == 1) + { + var dictionary1 = new Dictionary>(1, comparer); + T value = items[0]; + dictionary1.Add(keySelector(value), ImmutableArray.Create(value)); + return dictionary1; + } + + if (items.Length == 0) + { + return new Dictionary>(comparer); + } + + // bucketize + // prevent reallocation. it may not have 'count' entries, but it won't have more. + var accumulator = new Dictionary>(items.Length, comparer); + for (int i = 0; i < items.Length; i++) + { + var item = items[i]; + var key = keySelector(item); + + ArrayBuilder bucket; + if (!accumulator.TryGetValue(key, out bucket)) + { + bucket = ArrayBuilder.GetInstance(); + accumulator.Add(key, bucket); + } + + bucket.Add(item); + } + + var dictionary = new Dictionary>(accumulator.Count, comparer); + + // freeze + foreach (var pair in accumulator) + { + dictionary.Add(pair.Key, pair.Value.ToImmutableAndFree()); + } + + return dictionary; + } } } diff --git a/src/Compilers/Core/Portable/Diagnostic/NoLocation.cs b/src/Compilers/Core/Portable/Diagnostic/NoLocation.cs index bc12ca91c5e6c98dde34a7c2eaecbab4c4680955..08aa07916aebb7b3c1ad46fd355559bad8064baf 100644 --- a/src/Compilers/Core/Portable/Diagnostic/NoLocation.cs +++ b/src/Compilers/Core/Portable/Diagnostic/NoLocation.cs @@ -21,7 +21,7 @@ public override LocationKind Kind public override bool Equals(object obj) { - return ReferenceEquals(this, obj); + return (object)this == obj; } public override int GetHashCode() diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DiagnosticAnalyzer.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DiagnosticAnalyzer.cs index 5b021b6439cb4401c8b40a36f8c1e2f66be8a918..bffaf42fd951167ee25fdeda357aa24129a23e9e 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DiagnosticAnalyzer.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DiagnosticAnalyzer.cs @@ -23,12 +23,12 @@ public abstract class DiagnosticAnalyzer public sealed override bool Equals(object obj) { - return ReferenceEqualityComparer.Instance.Equals(this, obj); + return (object)this == obj; } public sealed override int GetHashCode() { - return ReferenceEqualityComparer.Instance.GetHashCode(this); + return ReferenceEqualityComparer.GetHashCode(this); } public sealed override string ToString() diff --git a/src/Compilers/Core/Portable/DocumentationComments/DocumentationProvider.NullDocumentationProvider.cs b/src/Compilers/Core/Portable/DocumentationComments/DocumentationProvider.NullDocumentationProvider.cs index f6c737cae1ac01a453b11e9df43c0d1eb345654c..049b0deda3d570b99df7fe36cf352c51a5ea727f 100644 --- a/src/Compilers/Core/Portable/DocumentationComments/DocumentationProvider.NullDocumentationProvider.cs +++ b/src/Compilers/Core/Portable/DocumentationComments/DocumentationProvider.NullDocumentationProvider.cs @@ -22,7 +22,7 @@ protected internal override string GetDocumentationForSymbol(string documentatio public override bool Equals(object obj) { // Only one instance is expected to exist, so reference equality is fine. - return ReferenceEquals(this, obj); + return (object)this == obj; } public override int GetHashCode() diff --git a/src/Compilers/Core/Portable/InternalUtilities/EmptyComparer.cs b/src/Compilers/Core/Portable/InternalUtilities/EmptyComparer.cs new file mode 100644 index 0000000000000000000000000000000000000000..3e829ccd756be26efd66ebee9f34f2b7066d17ae --- /dev/null +++ b/src/Compilers/Core/Portable/InternalUtilities/EmptyComparer.cs @@ -0,0 +1,33 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.Diagnostics; +using System.Runtime.CompilerServices; + +namespace Roslyn.Utilities +{ + /// + /// Very cheap trivial comparer that never matches the keys, + /// should only be used in empty dictionaries. + /// + internal sealed class EmptyComparer : IEqualityComparer + { + public static readonly EmptyComparer Instance = new EmptyComparer(); + + private EmptyComparer() + { + } + + bool IEqualityComparer.Equals(object a, object b) + { + Debug.Assert(false, "Are we using empty comparer with nonempty dictionary?"); + return false; + } + + int IEqualityComparer.GetHashCode(object s) + { + // dictionary will call this often + return 0; + } + } +} diff --git a/src/Compilers/Core/Portable/InternalUtilities/ReferenceEqualityComparer.cs b/src/Compilers/Core/Portable/InternalUtilities/ReferenceEqualityComparer.cs index 54c80682ab10723bcd5c7311d60b0dbb0a704472..e41b866880a20e70ed407002fc05cefe82e048d4 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/ReferenceEqualityComparer.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/ReferenceEqualityComparer.cs @@ -10,7 +10,7 @@ namespace Roslyn.Utilities /// internal class ReferenceEqualityComparer : IEqualityComparer { - public static readonly IEqualityComparer Instance = new ReferenceEqualityComparer(); + public static readonly ReferenceEqualityComparer Instance = new ReferenceEqualityComparer(); private ReferenceEqualityComparer() { @@ -22,6 +22,11 @@ bool IEqualityComparer.Equals(object a, object b) } int IEqualityComparer.GetHashCode(object a) + { + return ReferenceEqualityComparer.GetHashCode(a); + } + + public static int GetHashCode(object a) { return RuntimeHelpers.GetHashCode(a); } diff --git a/src/Compilers/Core/Portable/InternalUtilities/StringOrdinalComparer.cs b/src/Compilers/Core/Portable/InternalUtilities/StringOrdinalComparer.cs new file mode 100644 index 0000000000000000000000000000000000000000..a1bcf1d388c75cbcb4b30fb9de7cf3d0cd002edb --- /dev/null +++ b/src/Compilers/Core/Portable/InternalUtilities/StringOrdinalComparer.cs @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.Runtime.CompilerServices; + +namespace Roslyn.Utilities +{ + /// + /// Compares string based upon their ordinal equality. + /// We use this comparer for string identifiers because it does exactly what we need and nothing more + /// The StringComparer.Ordinal as implemented by StringComparer is more complex to support + /// case sensitive and insensitive compares depending on flags. + /// It also defers to the default string hash function that might not be the best for our scenarios. + /// + internal sealed class StringOrdinalComparer : IEqualityComparer + { + public static readonly StringOrdinalComparer Instance = new StringOrdinalComparer(); + + private StringOrdinalComparer() + { + } + + bool IEqualityComparer.Equals(string a, string b) + { + return StringOrdinalComparer.Equals(a, b); + } + + public static bool Equals(string a, string b) + { + // this is fast enough + return string.Equals(a, b); + } + + int IEqualityComparer.GetHashCode(string s) + { + // PERF: the default string hashcode is not always good or fast and cannot be changed for compat reasons. + // We, however, can use anything we want in our dictionaries. + // Our typical scenario is a relatively short string (identifier) + // FNV performs pretty well in such cases + return Hash.GetFNVHashCode(s); + } + } +} diff --git a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs index c0b192391b95202af57965d3884a20be044a3e28..6bab659cf86b6bfde3f6d3c8da60edbacf0230c3 100644 --- a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs +++ b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs @@ -99,23 +99,25 @@ internal TypeSymbol GetTypeOfToken(EntityHandle token, out bool isNoPiaLocalType TypeSymbol type; HandleKind tokenType = token.Kind; - if (tokenType == HandleKind.TypeDefinition) + switch (tokenType) { - type = GetTypeOfTypeDef((TypeDefinitionHandle)token, out isNoPiaLocalType, isContainingType: false); - } - else if (tokenType == HandleKind.TypeSpecification) - { - isNoPiaLocalType = false; - type = GetTypeOfTypeSpec((TypeSpecificationHandle)token); - } - else if (tokenType == HandleKind.TypeReference) - { - type = GetTypeOfTypeRef((TypeReferenceHandle)token, out isNoPiaLocalType); - } - else - { - isNoPiaLocalType = false; - type = GetUnsupportedMetadataTypeSymbol(); + case HandleKind.TypeDefinition: + type = GetTypeOfTypeDef((TypeDefinitionHandle)token, out isNoPiaLocalType, isContainingType: false); + break; + + case HandleKind.TypeSpecification: + isNoPiaLocalType = false; + type = GetTypeOfTypeSpec((TypeSpecificationHandle)token); + break; + + case HandleKind.TypeReference: + type = GetTypeOfTypeRef((TypeReferenceHandle)token, out isNoPiaLocalType); + break; + + default: + isNoPiaLocalType = false; + type = GetUnsupportedMetadataTypeSymbol(); + break; } Debug.Assert(type != null); diff --git a/src/Compilers/Core/Portable/Syntax/SyntaxNode.cs b/src/Compilers/Core/Portable/Syntax/SyntaxNode.cs index 2ea70c76c0f4742df05e86fa24db0972e37b06ce..fe398f804d4b4cc21c9311e3e05bad3347c008dd 100644 --- a/src/Compilers/Core/Portable/Syntax/SyntaxNode.cs +++ b/src/Compilers/Core/Portable/Syntax/SyntaxNode.cs @@ -141,8 +141,8 @@ internal SyntaxNode GetRed(ref SyntaxNode field, int slot) var green = this.Green.GetSlot(slot); if (green != null) { - result = green.CreateRed(this, this.GetChildPosition(slot)); - result = Interlocked.CompareExchange(ref field, result, null) ?? result; + Interlocked.CompareExchange(ref field, green.CreateRed(this, this.GetChildPosition(slot)), null); + result = field; } } @@ -159,8 +159,8 @@ internal SyntaxNode GetRedAtZero(ref SyntaxNode field) var green = this.Green.GetSlot(0); if (green != null) { - result = green.CreateRed(this, this.Position); - result = Interlocked.CompareExchange(ref field, result, null) ?? result; + Interlocked.CompareExchange(ref field, green.CreateRed(this, this.Position), null); + result = field; } } @@ -176,8 +176,8 @@ internal SyntaxNode GetRedAtZero(ref SyntaxNode field) var green = this.Green.GetSlot(slot); if (green != null) { - result = (T)green.CreateRed(this, this.GetChildPosition(slot)); - result = Interlocked.CompareExchange(ref field, result, null) ?? result; + Interlocked.CompareExchange(ref field, (T)green.CreateRed(this, this.GetChildPosition(slot)), null); + result = field; } } @@ -194,8 +194,8 @@ internal SyntaxNode GetRedAtZero(ref SyntaxNode field) var green = this.Green.GetSlot(0); if (green != null) { - result = (T)green.CreateRed(this, this.Position); - result = Interlocked.CompareExchange(ref field, result, null) ?? result; + Interlocked.CompareExchange(ref field, (T)green.CreateRed(this, this.Position), null); + result = field; } } @@ -216,11 +216,9 @@ internal SyntaxNode GetRedElement(ref SyntaxNode element, int slot) if (result == null) { var green = this.Green.GetSlot(slot); - result = green.CreateRed(this.Parent, this.GetChildPosition(slot)); // <- passing list's parent - if (Interlocked.CompareExchange(ref element, result, null) != null) - { - result = element; - } + // passing list's parent + Interlocked.CompareExchange(ref element, green.CreateRed(this.Parent, this.GetChildPosition(slot)), null); + result = element; } return result; @@ -240,11 +238,9 @@ internal SyntaxNode GetRedElementIfNotToken(ref SyntaxNode element) var green = this.Green.GetSlot(1); if (!green.IsToken) { - result = green.CreateRed(this.Parent, this.GetChildPosition(1)); // <- passing list's parent - if (Interlocked.CompareExchange(ref element, result, null) != null) - { - result = element; - } + // passing list's parent + Interlocked.CompareExchange(ref element, green.CreateRed(this.Parent, this.GetChildPosition(1)), null); + result = element; } } diff --git a/src/Compilers/Core/SharedCollections/ArrayBuilder.cs b/src/Compilers/Core/SharedCollections/ArrayBuilder.cs index a61c6fce30c795acff1c31cccd5e4531ae5f239f..58031f51c3169264e227e6e50ee5421a08539c4d 100644 --- a/src/Compilers/Core/SharedCollections/ArrayBuilder.cs +++ b/src/Compilers/Core/SharedCollections/ArrayBuilder.cs @@ -352,41 +352,44 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { if (this.Count == 1) { - var dictionary = new Dictionary>(1, comparer); + var dictionary1 = new Dictionary>(1, comparer); T value = this[0]; - dictionary.Add(keySelector(value), ImmutableArray.Create(value)); - return dictionary; + dictionary1.Add(keySelector(value), ImmutableArray.Create(value)); + return dictionary1; } - else + + if (this.Count == 0) { - // bucketize - // prevent reallocation. it may not have 'count' entries, but it won't have more. - var accumulator = new Dictionary>(Count, comparer); - for (int i = 0; i < Count; i++) - { - var item = this[i]; - var key = keySelector(item); + return new Dictionary>(comparer); + } - ArrayBuilder bucket; - if (!accumulator.TryGetValue(key, out bucket)) - { - bucket = ArrayBuilder.GetInstance(); - accumulator.Add(key, bucket); - } + // bucketize + // prevent reallocation. it may not have 'count' entries, but it won't have more. + var accumulator = new Dictionary>(Count, comparer); + for (int i = 0; i < Count; i++) + { + var item = this[i]; + var key = keySelector(item); - bucket.Add(item); + ArrayBuilder bucket; + if (!accumulator.TryGetValue(key, out bucket)) + { + bucket = ArrayBuilder.GetInstance(); + accumulator.Add(key, bucket); } - var dictionary = new Dictionary>(accumulator.Count, comparer); + bucket.Add(item); + } - // freeze - foreach (var pair in accumulator) - { - dictionary.Add(pair.Key, pair.Value.ToImmutableAndFree()); - } + var dictionary = new Dictionary>(accumulator.Count, comparer); - return dictionary; + // freeze + foreach (var pair in accumulator) + { + dictionary.Add(pair.Key, pair.Value.ToImmutableAndFree()); } + + return dictionary; } public void AddRange(ArrayBuilder items) diff --git a/src/EditorFeatures/Test/Workspaces/TestWorkspace_XmlConsumption.cs b/src/EditorFeatures/Test/Workspaces/TestWorkspace_XmlConsumption.cs index 7609c182235d3abdf0029f3d1b88f2a087b694a5..663d2df31e4065f888af55b565584830b7560b8a 100644 --- a/src/EditorFeatures/Test/Workspaces/TestWorkspace_XmlConsumption.cs +++ b/src/EditorFeatures/Test/Workspaces/TestWorkspace_XmlConsumption.cs @@ -51,7 +51,7 @@ protected override string GetDocumentationForSymbol(string documentationMemberID public override bool Equals(object obj) { - return ReferenceEquals(this, obj); + return (object)this == obj; } public override int GetHashCode()