Skip to content

Commit

Permalink
Using Ordinal is both faster and more correct as our intent is to do … (
Browse files Browse the repository at this point in the history
dotnet#16439)

* Using Ordinal is both faster and more correct as our intent is to do symbolic compares.

* format code

* use extension methods
  • Loading branch information
dawedawe authored Dec 15, 2023
1 parent e9576a2 commit 54cfe95
Show file tree
Hide file tree
Showing 24 changed files with 53 additions and 32 deletions.
4 changes: 2 additions & 2 deletions src/Compiler/Checking/CheckFormatStrings.fs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ let escapeDotnetFormatString str =

[<return: Struct>]
let (|PrefixedBy|_|) (prefix: string) (str: string) =
if str.StartsWith prefix then
if str.StartsWithOrdinal(prefix) then
ValueSome prefix.Length
else
ValueNone
Expand Down Expand Up @@ -371,7 +371,7 @@ let parseFormatStringInternal
// type checker. They should always have '(...)' after for format string.
let requireAndSkipInterpolationHoleFormat i =
if i < len && fmt[i] = '(' then
let i2 = fmt.IndexOf(")", i+1)
let i2 = fmt.IndexOfOrdinal(")", i+1)
if i2 = -1 then
failwith (FSComp.SR.forFormatInvalidForInterpolated3())
else
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/ConstraintSolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1713,7 +1713,7 @@ and SolveMemberConstraint (csenv: ConstraintSolverEnv) ignoreUnresolvedOverload
None

let anonRecdPropSearch =
let isGetProp = nm.StartsWith "get_"
let isGetProp = nm.StartsWithOrdinal("get_")
if not isRigid && isGetProp && memFlags.IsInstance then
let propName = nm[4..]
let props =
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/MethodCalls.fs
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ type CalledMeth<'T>
// Detect the special case where an indexer setter using param aray takes 'value' argument after ParamArray arguments
let isIndexerSetter =
match pinfoOpt with
| Some pinfo when pinfo.HasSetter && minfo.LogicalName.StartsWith "set_" && (List.concat fullCurriedCalledArgs).Length >= 2 -> true
| Some pinfo when pinfo.HasSetter && minfo.LogicalName.StartsWithOrdinal("set_") && (List.concat fullCurriedCalledArgs).Length >= 2 -> true
| _ -> false

let argSetInfos =
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/NicePrint.fs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ module internal PrintUtilities =

let comment str = wordL (tagText (sprintf "(* %s *)" str))

let isDiscard (name: string) = name.StartsWith("_")
let isDiscard (name: string) = name.StartsWithOrdinal("_")

let ensureFloat (s: string) =
if String.forall (fun c -> Char.IsDigit c || c = '-') s then
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/SignatureHash.fs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ let calculateHashOfImpliedSignature g observer (expr: ModuleOrNamespaceContents)

let rec hashModuleOrNameSpaceBinding (monb: ModuleOrNamespaceBinding) =
match monb with
| ModuleOrNamespaceBinding.Binding b when b.Var.LogicalName.StartsWith("doval@") -> 0
| ModuleOrNamespaceBinding.Binding b when b.Var.LogicalName.StartsWithOrdinal("doval@") -> 0
| ModuleOrNamespaceBinding.Binding b -> HashTastMemberOrVals.hashValOrMemberNoInst (g, observer) (mkLocalValRef b.Var)
| ModuleOrNamespaceBinding.Module(moduleInfo, contents) -> hashSingleModuleOrNameSpaceIncludingName (moduleInfo, contents)

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/infos.fs
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ type MethInfo =
member x.IsUnionCaseTester =
let tcref = x.ApparentEnclosingTyconRef
tcref.IsUnionTycon &&
x.LogicalName.StartsWith("get_Is") &&
x.LogicalName.StartsWithOrdinal("get_Is") &&
match x.ArbitraryValRef with
| Some v -> v.IsImplied
| None -> false
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/CodeGen/IlxGen.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2695,7 +2695,7 @@ let CodeGenThen (cenv: cenv) mgbuf (entryPointInfo, methodName, eenv, alreadyUse
match selfArgOpt with
| Some selfArg when
selfArg.LogicalName <> "this"
&& not (selfArg.LogicalName.StartsWith("_"))
&& not (selfArg.LogicalName.StartsWithOrdinal("_"))
&& not cenv.options.localOptimizationsEnabled
->
let ilTy = selfArg.Type |> GenType cenv m eenv.tyenv
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/DependencyManager/DependencyProvider.fs
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ type DependencyProvider
let managers =
RegisteredDependencyManagers compilerTools (Option.ofString outputDir) reportError

match managers |> Seq.tryFind (fun kv -> path.StartsWith(kv.Value.Key + ":")) with
match managers |> Seq.tryFind (fun kv -> path.StartsWithOrdinal(kv.Value.Key + ":")) with
| None ->
let err, msg =
this.CreatePackageManagerUnknownError(compilerTools, outputDir, path.Split(':').[0], reportError)
Expand Down
3 changes: 2 additions & 1 deletion src/Compiler/DependencyManager/NativeDllResolveHandler.fs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ open System.IO
open System.Reflection
open System.Runtime.InteropServices
open Internal.Utilities
open Internal.Utilities.Library
open Internal.Utilities.FSharpEnvironment
open FSharp.Compiler.IO

Expand Down Expand Up @@ -88,7 +89,7 @@ type internal NativeDllResolveHandlerCoreClr(nativeProbingRoots: NativeResolutio
let isRooted = Path.IsPathRooted name

let useSuffix s =
not (name.Contains(s + ".") || name.EndsWith(s)) // linux devs often append version # to libraries I.e mydll.so.5.3.2
not (name.Contains(s + ".") || name.EndsWithOrdinal(s)) // linux devs often append version # to libraries I.e mydll.so.5.3.2

let usePrefix =
name.IndexOf(Path.DirectorySeparatorChar) = -1 // If name has directory information no add no prefix
Expand Down
8 changes: 4 additions & 4 deletions src/Compiler/Driver/FxResolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ type internal FxResolver
dotnetConfig.IndexOf(pattern, StringComparison.OrdinalIgnoreCase)
+ pattern.Length

let endPos = dotnetConfig.IndexOf("\"", startPos)
let endPos = dotnetConfig.IndexOfOrdinal("\"", startPos)
let ver = dotnetConfig[startPos .. endPos - 1]

let path =
Expand Down Expand Up @@ -364,7 +364,7 @@ type internal FxResolver
let implDir, warnings = getImplementationAssemblyDir ()
let version = DirectoryInfo(implDir).Name

if version.StartsWith("x") then
if version.StartsWithOrdinal("x") then
// Is running on the desktop
(None, None), warnings
else
Expand Down Expand Up @@ -403,7 +403,7 @@ type internal FxResolver
| ".NET", "Core" when arr.Length >= 3 -> Some("netcoreapp" + (getTfmNumber arr[2]))

| ".NET", "Framework" when arr.Length >= 3 ->
if arr[2].StartsWith("4.8") then
if arr[2].StartsWithOrdinal("4.8") then
Some "net48"
else
Some "net472"
Expand Down Expand Up @@ -560,7 +560,7 @@ type internal FxResolver
dotnetConfig.IndexOf(pattern, StringComparison.OrdinalIgnoreCase)
+ pattern.Length

let endPos = dotnetConfig.IndexOf("\"", startPos)
let endPos = dotnetConfig.IndexOfOrdinal("\"", startPos)
let tfm = dotnetConfig[startPos .. endPos - 1]
tfm
with _ ->
Expand Down
3 changes: 2 additions & 1 deletion src/Compiler/Facilities/prim-lexing.fs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace FSharp.Compiler.Text

open System
open System.IO
open Internal.Utilities.Library

type ISourceText =

Expand Down Expand Up @@ -97,7 +98,7 @@ type StringText(str: string) =
if lastIndex <= startIndex || lastIndex >= str.Length then
invalidArg "target" "Too big."

str.IndexOf(target, startIndex, target.Length) <> -1
str.IndexOfOrdinal(target, startIndex, target.Length) <> -1

member _.Length = str.Length

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Interactive/fsi.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1526,7 +1526,7 @@ let ConvReflectionTypeToILTypeRef (reflectionTy: Type) =
let scoref = ILScopeRef.Assembly aref

let fullName = reflectionTy.FullName
let index = fullName.IndexOf("[")
let index = fullName.IndexOfOrdinal("[")

let fullName =
if index = -1 then
Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/Optimize/LowerStateMachines.fs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ let isExpandVar g (v: Val) =
let isStateMachineBindingVar g (v: Val) =
isExpandVar g v ||
(let nm = v.LogicalName
(nm.StartsWith "builder@" || v.IsMemberThisVal) &&
(nm.StartsWithOrdinal("builder@") || v.IsMemberThisVal) &&
not v.IsCompiledAsTopLevel)

type env =
Expand Down Expand Up @@ -833,7 +833,7 @@ type LowerStateMachine(g: TcGlobals) =
|> Result.Ok
elif bind.Var.IsCompiledAsTopLevel ||
not (resBody.resumableVars.FreeLocals.Contains(bind.Var)) ||
bind.Var.LogicalName.StartsWith stackVarPrefix then
bind.Var.LogicalName.StartsWithOrdinal(stackVarPrefix) then
if sm_verbose then printfn "LetExpr (non-control-flow, rewrite rhs, RepresentBindingAsTopLevelOrLocal)"
RepresentBindingAsTopLevelOrLocal bind resBody m
|> Result.Ok
Expand Down
6 changes: 3 additions & 3 deletions src/Compiler/Optimize/Optimizer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1534,11 +1534,11 @@ let ValueOfExpr expr =

let IsMutableStructuralBindingForTupleElement (vref: ValRef) =
vref.IsCompilerGenerated &&
vref.LogicalName.EndsWith suffixForTupleElementAssignmentTarget
vref.LogicalName.EndsWithOrdinal suffixForTupleElementAssignmentTarget

let IsMutableForOutArg (vref: ValRef) =
vref.IsCompilerGenerated &&
vref.LogicalName.StartsWith(outArgCompilerGeneratedName)
vref.LogicalName.StartsWithOrdinal(outArgCompilerGeneratedName)

let IsKnownOnlyMutableBeforeUse (vref: ValRef) =
IsMutableStructuralBindingForTupleElement vref ||
Expand Down Expand Up @@ -1672,7 +1672,7 @@ let TryEliminateBinding cenv _env bind e2 _m =
not vspec1.IsCompilerGenerated then
None
elif vspec1.IsFixed then None
elif vspec1.LogicalName.StartsWith stackVarPrefix ||
elif vspec1.LogicalName.StartsWithOrdinal stackVarPrefix ||
vspec1.LogicalName.Contains suffixForVariablesThatMayNotBeEliminated then None
else

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Service/FSharpCheckerResults.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1909,7 +1909,7 @@ type internal TypeCheckInfo
| Some(_, lines) ->
let lines =
lines
|> List.filter (fun line -> not (line.StartsWith("//")) && not (String.IsNullOrEmpty line))
|> List.filter (fun line -> not (line.StartsWithOrdinal("//")) && not (String.IsNullOrEmpty line))

ToolTipText.ToolTipText
[
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Service/SemanticClassification.fs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ module TcResolutionsExtensions =
&& protectAssemblyExplorationNoReraise false false (fun () ->
ExistsHeadTypeInEntireHierarchy g amap range0 ty g.tcref_System_IDisposable)

let isDiscard (str: string) = str.StartsWith("_")
let isDiscard (str: string) = str.StartsWithOrdinal("_")

let isValRefDisposable g amap (vref: ValRef) =
not (isDiscard vref.DisplayName)
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Service/ServiceAnalysis.fs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ module UnusedDeclarations =
su.IsFromDefinition
&& su.Symbol.DeclarationLocation.IsSome
&& (isScript || su.IsPrivateToFile)
&& not (su.Symbol.DisplayName.StartsWith "_")
&& not (su.Symbol.DisplayName.StartsWithOrdinal "_")
&& isPotentiallyUnusedDeclaration su.Symbol
then
Some(su, usages.Contains su.Symbol.DeclarationLocation.Value)
Expand Down
5 changes: 4 additions & 1 deletion src/Compiler/SyntaxTree/SyntaxTreeOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,10 @@ let opNameQMark = CompileOpName qmark

let mkSynOperator (opm: range) (oper: string) =
let trivia =
if oper.StartsWith("~") && ((opm.EndColumn - opm.StartColumn) = (oper.Length - 1)) then
if
oper.StartsWithOrdinal("~")
&& ((opm.EndColumn - opm.StartColumn) = (oper.Length - 1))
then
// PREFIX_OP token where the ~ was actually absent
IdentTrivia.OriginalNotation(string (oper.[1..]))
else
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/TypedTree/TcGlobals.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1882,7 +1882,7 @@ type TcGlobals(
let memberName =
let nm = t.MemberLogicalName
let coreName =
if nm.StartsWith "op_" then nm[3..]
if nm.StartsWithOrdinal "op_" then nm[3..]
elif nm = "get_Zero" then "GenericZero"
elif nm = "get_One" then "GenericOne"
else nm
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/TypedTree/TypedTreeOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -10417,7 +10417,7 @@ let (|SequentialResumableCode|_|) (g: TcGlobals) expr =
Some (e1, e2, m, (fun e1 e2 -> Expr.Sequential(e1, e2, NormalSeq, m)))

// let __stack_step = e1 in e2
| Expr.Let(bind, e2, m, _) when bind.Var.CompiledName(g.CompilerGlobalState).StartsWith(stackVarPrefix) ->
| Expr.Let(bind, e2, m, _) when bind.Var.CompiledName(g.CompilerGlobalState).StartsWithOrdinal(stackVarPrefix) ->
Some (bind.Expr, e2, m, (fun e1 e2 -> mkLet bind.DebugPoint m bind.Var e1 e2))

| _ -> None
Expand Down
7 changes: 4 additions & 3 deletions src/Compiler/Utilities/PathMap.fs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace Internal.Utilities
open System
open System.IO

open Internal.Utilities.Library
open FSharp.Compiler.IO

type PathMap = PathMap of Map<string, string>
Expand All @@ -22,7 +23,7 @@ module internal PathMap =
let normalSrc = FileSystem.GetFullPathShim src

let oldPrefix =
if normalSrc.EndsWith dirSepStr then
if normalSrc.EndsWithOrdinal dirSepStr then
normalSrc
else
normalSrc + dirSepStr
Expand All @@ -41,7 +42,7 @@ module internal PathMap =
// oldPrefix always ends with a path separator, so there's no need
// to check if it was a partial match
// e.g. for the map /goo=/bar and file name /goooo
if filePath.StartsWith(oldPrefix, StringComparison.Ordinal) then
if filePath.StartsWithOrdinal oldPrefix then
let replacement = replacementPrefix + filePath.Substring(oldPrefix.Length - 1)

// Normalize the path separators if used uniformly in the replacement
Expand All @@ -60,7 +61,7 @@ module internal PathMap =
|> Option.defaultValue filePath

let applyDir pathMap (dirName: string) : string =
if dirName.EndsWith dirSepStr then
if dirName.EndsWithOrdinal dirSepStr then
apply pathMap dirName
else
let mapped = apply pathMap (dirName + dirSepStr)
Expand Down
9 changes: 9 additions & 0 deletions src/Compiler/Utilities/illib.fs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,15 @@ module internal PervasiveAutoOpens =
member inline x.EndsWithOrdinalIgnoreCase value =
x.EndsWith(value, StringComparison.OrdinalIgnoreCase)

member inline x.IndexOfOrdinal value =
x.IndexOf(value, StringComparison.Ordinal)

member inline x.IndexOfOrdinal(value, startIndex) =
x.IndexOf(value, startIndex, StringComparison.Ordinal)

member inline x.IndexOfOrdinal(value, startIndex, count) =
x.IndexOf(value, startIndex, count, StringComparison.Ordinal)

/// Get an initialization hole
let getHole (r: _ ref) =
match r.Value with
Expand Down
6 changes: 6 additions & 0 deletions src/Compiler/Utilities/illib.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ module internal PervasiveAutoOpens =

member inline EndsWithOrdinalIgnoreCase: value: string -> bool

member inline IndexOfOrdinal: value: string -> int

member inline IndexOfOrdinal: value: string * startIndex: int -> int

member inline IndexOfOrdinal: value: string * startIndex: int * count: int -> int

type Async with

/// Runs the computation synchronously, always starting on the current thread.
Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/Utilities/sformat.fs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ module Layout =
#if COMPILER
let rec endsWithL (text: string) layout =
match layout with
| Leaf(_, s, _) -> s.Text.EndsWith(text)
| Leaf(_, s, _) -> s.Text.EndsWith(text, StringComparison.Ordinal)
| Node(_, r, _) -> endsWithL text r
| Attr(_, _, l) -> endsWithL text l
| ObjLeaf _ -> false
Expand Down Expand Up @@ -512,7 +512,7 @@ module ReflectUtils =
FSharpValue.GetTupleFields obj |> Array.mapi (fun i v -> (v, tyArgs[i]))

let tupleType =
if reprty.Name.StartsWith "ValueTuple" then
if reprty.Name.StartsWith("ValueTuple", StringComparison.Ordinal) then
TupleType.Value
else
TupleType.Reference
Expand Down

0 comments on commit 54cfe95

Please sign in to comment.