Parsing improvements: no reactor, add parsing options, error severity options #3601

auduchinok · 2017-09-18T16:15:14Z

Parsing without queuing to the reactor may help with delays in getting possible breakpoint ranges, navigation bars in VS. We've been using it in Rider for some time already and it has greatly improved parsing and some editor features performance. It can also be used to implement features like code auto-formatting while typing.

This PR introduces two new options types (parsing and error severity) that may be useful for FCS users. Parsing options have a reasonable default instance that only needs updating source files list. The default errors severity options may be used as it is.
Parsing options can be created from command line args so it is easy to replace FSharpProjectOptions in many scenarios.

A questionable change: the removal of DependencyFiles from FSharpParseFileResults. The only place it is used in is now-obsolete FSharpSource. If this field is needed somewhere else, I'll revert it. The copy of this list is still stored in type check results.

Currently this PR has changes to FCS related code only (fcs/FSharp.Compiler.Service.sln) and doesn't contain needed changes for VFT projects, so expect the VFT compilation and tests to fail. I'd like to add needed changes but would like this code to be reviewed and probably to have some assistance.

Fixes #3107, #3404 and fsharp/fsharp-compiler-docs#774

realvictorprm · 2017-09-18T17:10:46Z

Hey @auduchinok. Many many thanks for sharing this with the community!

vasily-kirichenko · 2017-09-18T17:10:50Z

7>E:\github\visualfsharp\src\fsharp\vs\service.fs(1521,19): error FS1182: The value 'conditionalCompilationDefines' is unused
7>E:\github\visualfsharp\src\fsharp\vs\service.fs(1525,19): error FS1182: The value 'lexResourceManager' is unused

realvictorprm · 2017-09-18T17:12:21Z

Can we (or more specifically me) help you fixing it or do you prefer to keep it clean @auduchinok?

auduchinok · 2017-09-18T17:19:31Z

@realvictorprm Help with fixing VFT projects/tests would be appreciated.
I'm going to try making VisualFSharp.sln load locally again and will try to update usages for changed APIs.

saul · 2017-09-18T20:56:32Z

Using @vasily-kirichenko's branch with the FSharp.Editor fixes and I'm getting syntax highlighting immediately in Project.fs - previously this would take an age to appear (if ever). This is a fantastic improvement to responsiveness.

cartermp · 2017-09-18T22:25:29Z

Setting breakpoints deep in TypeChecker.fs, e.g.

https://github.com/Microsoft/visualfsharp/blob/f52362ad50e51f897d2b624adecc8c15cab56fbe/src/fsharp/TypeChecker.fs#L13286

Certainly feels faster to me. I haven't been able the popup in #3404 to hit yet. Nice!

@abelbraaksma, if you have the time, could you pull down this branch and see how it fares on your solution?

dsyme · 2017-09-20T12:05:09Z

Some build errors:

  D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\FSharpSource.fs(372,16): error FS0039: The field, constructor or member 'ParseFileInProject' is not defined. Maybe you want one of the following:�   ParseAndCheckFileInProject�   ParseFile�   ParseAndCheckProject�   CheckFileInProject [D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\FSharp.LanguageService.fsproj]
  D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\BackgroundRequests.fs(140,89): error FS0001: This expression was expected to have type�    'FSharpParsingOptions'    �but here has type�    'FSharpProjectOptions' [D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\FSharp.LanguageService.fsproj]
  D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\BackgroundRequests.fs(152,55): error FS0039: The field, constructor or member 'ParseFileInProject' is not defined. Maybe you want one of the following:�   ParseAndCheckFileInProject�   ParseFile�   ParseAndCheckProject�   CheckFileInProject [D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\FSharp.LanguageService.fsproj]
  D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\BackgroundRequests.fs(159,63): error FS0039: The field, constructor or member 'ParseFileInProject' is not defined. Maybe you want one of the following:�   ParseAndCheckFileInProject�   ParseFile�   ParseAndCheckProject�   CheckFileInProject [D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\FSharp.LanguageService.fsproj]
  D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\BackgroundRequests.fs(187,58): error FS0039: The field, constructor or member 'ParseFileInProject' is not defined. Maybe you want one of the following:�   ParseAndCheckFileInProject�   ParseFile�   ParseAndCheckProject�   CheckFileInProject [D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\FSharp.LanguageService.fsproj]
  D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\BackgroundRequests.fs(207,85): error FS0039: The field, constructor or member 'DependencyFiles' is not defined. [D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\src\FSharp.LanguageService\FSharp.LanguageService.fsproj]

dsyme

This is wonderful, thanks. I made some comments - some were already mentioned above. I think revert the change around DependencyFiles for now

dsyme · 2017-09-20T12:06:37Z

src/fsharp/CompileOptions.fsi

@@ -69,6 +69,9 @@ val GetCoreFscCompilerOptions     : TcConfigBuilder -> CompilerOptionBlock list
 val GetCoreFsiCompilerOptions     : TcConfigBuilder -> CompilerOptionBlock list
 val GetCoreServiceCompilerOptions : TcConfigBuilder -> CompilerOptionBlock list

+// Apply args to TcConfigBuilder and return new list of source files


Use /// comments please

dsyme · 2017-09-20T12:07:32Z

src/fsharp/CompileOptions.fsi

@@ -69,6 +69,9 @@ val GetCoreFscCompilerOptions     : TcConfigBuilder -> CompilerOptionBlock list
 val GetCoreFsiCompilerOptions     : TcConfigBuilder -> CompilerOptionBlock list
 val GetCoreServiceCompilerOptions : TcConfigBuilder -> CompilerOptionBlock list

+// Apply args to TcConfigBuilder and return new list of source files
+val ApplyCommandLineArgs: TcConfigBuilder * string list * string list -> string list


We don't yet enforce this, and don't do it often enough in the compiler, but please add argument names to the signature, e.g.

val ApplyCommandLineArgs: x: TcConfigBuilder * y: string list * z: string list -> string list

dsyme · 2017-09-20T12:08:49Z

src/fsharp/vs/IncrementalBuild.fs

@@ -1537,7 +1532,6 @@ type IncrementalBuilder(tcGlobals,frameworkTcImports, nonFrameworkAssemblyInputs
    member __.FileChecked = fileChecked.Publish
    member __.ProjectChecked = projectChecked.Publish
    member __.ImportedCcusInvalidated = importsInvalidated.Publish
-    member __.AllDependenciesDeprecated = allDependencies 


Is there a specific reason to remove this as part of this PR? Just wondering, thanks

No, not really, I've reverted it.

dsyme · 2017-09-20T12:09:39Z

src/fsharp/vs/service.fs

+    member x.LastFileName =
+        Debug.Assert(not (Array.isEmpty x.SourceFiles), "Parsing options don't contain any file")
+        Array.last x.SourceFiles
+    static member Default =


Please put a blank line between each member definition, thanks

dsyme · 2017-09-20T12:10:48Z

src/fsharp/vs/service.fs

        // This function gets called whenever an error happens during parsing or checking
-        let diagnosticSink sev (exn:PhasedDiagnostic) = 


Try to avoid the whitespace changes in a PR. If you'd like to remove all trailing whitespaces, then please submit a single PR tht does it globally across all source files under src and vsintegration\src - that would be great - thanks!

I would be easier to do it in a few parts, I think, rather than a single PR.
Should I revert these formatting changes in this PR?

@auduchinok it's not a big deal for this one - you can leave these changes in - it was just a comment for future PRs

dsyme · 2017-09-20T12:12:46Z

src/fsharp/vs/service.fs

-    let parseFileInProjectCache = 
-        MruCache<ParseCacheLockToken, _, _>(parseFileInProjectCacheSize, 
-            areSame=AreSameForParsing3,
-            areSimilar=AreSubsumable3)


Is it really ok to drop the areSimilar function here?

@auduchinok Could you comment on the removal of areSimilar here? Is it needed? Perhaps now same and similar are the same?

I think I shouldn't have removed it completely. I've reverted and changed it to check file name. Please tell if it's a wrong change.

That looks right

dsyme · 2017-09-20T12:14:12Z

src/fsharp/vs/service.fsi

@@ -515,7 +528,9 @@ type internal FSharpChecker =
    /// so that an 'unload' and 'reload' action will cause the script to be considered as a new project,
    /// so that references are re-resolved.</param>
    member GetProjectOptionsFromCommandLineArgs : projectFileName: string * argv: string[] * ?loadedTimeStamp: DateTime * ?extraProjectInfo: obj -> FSharpProjectOptions
-
+
+    member GetParsingOptionsFromCommandLineArgs: string list -> FSharpParsingOptions * FSharpErrorInfo list


Please add /// comments and <param> entries, thanks!

dsyme · 2017-09-20T12:14:37Z

src/fsharp/vs/service.fsi

-    /// <param name="userOpName">An optional string used for tracing compiler operations associated with this request.</param>
-    member ParseFileInProject : filename: string * source: string * options: FSharpProjectOptions * ?userOpName: string -> Async<FSharpParseFileResults>
+    /// <param name="options">Parsing options for the project or script.</param>
+    member ParseFile: filename: string * source: string * options: FSharpParsingOptions -> Async<FSharpParseFileResults>


Please add a ParseFileInProject for compatibility which extracts the parsing options from the project options.

Should I mark it as deprecated?

Should it have the same signature and include userOpName?

@auduchinok I think don't mark it as deprecated yet - we would need to update all docs. Add an item on https://github.com/fsharp/FSharp.Compiler.Service/issues to track this

Yes, please use the same signature , and yes include userOpName - we should include that for ParseFile even if we don't use it as we should pass in causality-tracking information for any operation that consumes CPU thread or Reactor-thread resources

auduchinok · 2017-09-20T16:59:14Z

@dsyme I've fixed most issues and reverted parsing APIs so existing code will fallback to the new APIs with creating parsing options.
Should I trace userOpName as well or looking at it in debug would be sufficient?
Thank you.

dsyme · 2017-09-20T17:12:40Z

Should I trace userOpName as well or looking at it in debug would be sufficient?

I suppose it would be best to add a single tracing line

dsyme · 2017-09-20T17:30:08Z

@auduchinok There are a lot of failures, some listed here https://gist.github.com/dsyme/f75dde5e1bbc3f249566f55638766797

I think these are (mostly) to do with the removal of the added "new line" - tests are sensitive to that. You should probably do that change in a separate PR.

dsyme · 2017-09-20T17:36:14Z

src/fsharp/vs/service.fs

-          let errHandler = new ErrorHandler(reportErrors, mainInputFileName, tcConfig, source)
-
-          // Adding this new-line character at the end of the source seems odd but is required for some unit tests
-          let source = if source.Length = 0 || not (source.[source.Length - 1] = '\n') then source + "\n" else source 


This is the line that needs to be re-instated, or at least dealt with in a separate PR. What failures did you find this caused?

Thanks

It was causing access to non-existent lines in R# documents, which led to runtime exceptions.
It is possible to make a workaround for this hack but it clearly looks like a bug when a parser parses not a source you passed it but some different one.

I had reverted it and filed a separate issue (#3617).

@auduchinok Yes, I agree. I've added the context I know to the linked bug. On the bright side it could be worse - up until last year the same routine added three new line characters! :)

dsyme · 2017-09-20T23:15:39Z

Here are the current "Errors and Failures" from the part 1 test log:

1) Error : Tests.LanguageService.TimeStamp.UsingMSBuild.Regression.NoContainedString.Timestamps.Bug3368a
System.Exception : Error seen containing "project1"
   at Tests.LanguageService.TimeStamp.UsingMSBuild.AssertNoErrorSeenContaining(OpenProject project, String text) in D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.TimeStamp.fs:line 44
   at Tests.LanguageService.TimeStamp.UsingMSBuild.Regression.NoContainedString.Timestamps.Bug3368a() in D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.TimeStamp.fs:line 119

2) Failed : Tests.LanguageService.TimeStamp.UsingMSBuild.Timestamps.ProjectReferenceAssemblyChange
  Expected: not equal to 0
  But was:  0
at Tests.LanguageService.TimeStamp.UsingMSBuild.Timestamps.ProjectReferenceAssemblyChange() in D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.TimeStamp.fs:line 316

3) Failed : Tests.LanguageService.TimeStamp.UsingMSBuild.Timestamps.ReferenceAssemblyChangeAbsolute
  Expected: not equal to 0
  But was:  0
at Tests.LanguageService.TimeStamp.UsingMSBuild.Timestamps.ReferenceAssemblyChangeAbsolute() in D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.TimeStamp.fs:line 208

4) Failed : Tests.LanguageService.TimeStamp.UsingMSBuild.Timestamps.ReferenceAssemblyChangeRelative
  Expected: not equal to 0
  But was:  0
at Tests.LanguageService.TimeStamp.UsingMSBuild.Timestamps.ReferenceAssemblyChangeRelative() in D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.TimeStamp.fs:line 262

5) Error : Tests.LanguageService.TimeStamp.UsingProjectSystem.Regression.NoContainedString.Timestamps.Bug3368a
System.Exception : Error seen containing "project1"
   at Tests.LanguageService.TimeStamp.UsingMSBuild.AssertNoErrorSeenContaining(OpenProject project, String text) in D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.TimeStamp.fs:line 44
   at Tests.LanguageService.TimeStamp.UsingMSBuild.Regression.NoContainedString.Timestamps.Bug3368a() in D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.TimeStamp.fs:line 119

6) Failed : Tests.LanguageService.TimeStamp.UsingProjectSystem.Timestamps.ProjectReferenceAssemblyChange
  Expected: not equal to 0
  But was:  0
at Tests.LanguageService.TimeStamp.UsingMSBuild.Timestamps.ProjectReferenceAssemblyChange() in D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.TimeStamp.fs:line 316

7) Failed : Tests.LanguageService.TimeStamp.UsingProjectSystem.Timestamps.ReferenceAssemblyChangeAbsolute
  Expected: not equal to 0
  But was:  0
at Tests.LanguageService.TimeStamp.UsingMSBuild.Timestamps.ReferenceAssemblyChangeAbsolute() in D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.TimeStamp.fs:line 208

8) Failed : Tests.LanguageService.TimeStamp.UsingProjectSystem.Timestamps.ReferenceAssemblyChangeRelative
  Expected: not equal to 0
  But was:  0
at Tests.LanguageService.TimeStamp.UsingMSBuild.Timestamps.ReferenceAssemblyChangeRelative() in D:\j\workspace\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.TimeStamp.fs:line 262

Some of the problems may be related to areSimilar, I will take a look

dsyme

I left a few more comments - great work, thanks!!

dsyme · 2017-09-20T23:21:18Z

src/fsharp/vs/service.fs

@@ -2352,49 +2357,23 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
            foregroundTypeCheckCount <- foregroundTypeCheckCount + 1
            parseCacheLock.AcquireLock (fun ltok -> 
                parseAndCheckFileInProjectCachePossiblyStale.Set(ltok, (filename,options),(parseResults,typedResults,fileVersion))  
-                parseAndCheckFileInProjectCache.Set(ltok, (filename,source,options),(parseResults,typedResults,fileVersion,priorTimeStamp))
-                parseFileInProjectCache.Set(ltok, (filename,source,options),parseResults))


It's possible some of the tests are failing because parseFileInProjectCache is not being populated at this point. Is it possible to populate it? I guess that would involve calling GetParsingOptionsFromProjectOptions which is costly, defeating the purpose of the cache

BTW note that parseFileInProjectCache should now be renamed to parseFileCache please

I think it would be a good idea to store parsing options like it is currently done for project options and (probably?) pass it here as well.

dsyme · 2017-09-20T23:27:41Z

src/fsharp/vs/service.fs

-        MruCache<ParseCacheLockToken, _, _>(parseFileInProjectCacheSize, 
-            areSame=AreSameForParsing3,
-            areSimilar=AreSubsumable3)
+    let parseFileInProjectCache = MruCache<_,_,_>(parseFileInProjectCacheSize, areSimilar = AreSimilarForParsing, areSame = AreSameForParsing)


Please rename to parseFileCache

Also please be explicit that this can be used from any caller thread, i.e. MruCache<AnyCallerThreadToken,_,_> like braceMatchCacheMru

That said, I would feel a happier if we took a lock when accessing this cache as in the previous code - I don't think taking the lock on cache access will harm performance at all - and it might be better if the PR wasn't changing this cache from locked to lock free. I believe it's valid to do so (if not we have a bug in the braceMatchMruCache) - I'm just trying to minimize the size of the change in order to help you to get the tests to pass clean.

dsyme · 2017-09-20T23:28:36Z

src/fsharp/vs/service.fs

@@ -2789,7 +2755,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
            parseCacheLock.AcquireLock (fun ltok -> 
                parseAndCheckFileInProjectCachePossiblyStale.Clear ltok
                parseAndCheckFileInProjectCache.Clear ltok
-                parseFileInProjectCache.Clear ltok)
+                parseFileInProjectCache.Clear (AssumeAnyCallerThreadWithoutEvidence()))


As mentioned above, I think it's better for parseFileInProjectCache to still need a lock token

dsyme · 2017-09-20T23:33:12Z

src/fsharp/vs/service.fs

+
+    member ic.ParseFileInProject(filename, source, options, ?userOpName: string) =
+        let userOpName = defaultArg userOpName "Unknown"
+        ic.ParseFile(filename, source, ic.GetParsingOptionsFromProjectOptions(options), userOpName)


Now I see the implementation of ParseFileInProject I'm concerned it's a bit costly, involving the full reparse of the command line arguments, setting up a TcConfigBuilder and so on. Hmmm.... I suppose we should just deprecate this and adjust the Visual F# Tools to use ParseFile

dsyme · 2017-09-20T23:33:57Z

src/fsharp/vs/service.fsi

+    /// <para>Get the FSharpParsingOptions implied by a set of command line arguments.</para>
+    /// </summary>
+    ///
+    /// <param name="projectFileName">Used to differentiate between projects and for the base directory of the project.</param>


These param entries are wrong for this method

cartermp · 2017-10-02T20:45:35Z

@Pilchie FYI this is one we should make a case to take with the remainder of @KevinRansom's work for Core SDK project support. Big win for working in large files.

dsyme · 2017-10-02T20:51:04Z

@cartermp I don't think we should make a special case for this to get it into VS at the end of a release phase - there is enough churn here that there is a slim chance of a regression of some kind - the perf gains are good but I'd count on letting it bake in master for a bit

dsyme · 2017-10-02T21:27:41Z

One more failures now

1) Error : Tests.LanguageService.AutoCompletion.UsingMSBuild.Filenames.MayBeDifferentlyCased
System.Exception : No Authoring Scope
   at Salsa.Salsa.Privates.SimpleOpenFile.DoIntellisenseRequest(BackgroundRequestReason parseReason) in D:\j\w\release_ci_pa---3f142ccc\vsintegration\tests\Salsa\Salsa.fs:line 1201
   at Salsa.Salsa.Privates.SimpleOpenFile.AutoCompleteAtCursorImpl(BackgroundRequestReason reason, FSharpOption`1 filterText) in D:\j\w\release_ci_pa---3f142ccc\vsintegration\tests\Salsa\Salsa.fs:line 1325
   at Salsa.Salsa.MSBuildTestFlavor.Salsa-Salsa-VsOps-AutoCompleteAtCursor(OpenFile file) in D:\j\w\release_ci_pa---3f142ccc\vsintegration\tests\Salsa\Salsa.fs:line 1540
   at Tests.LanguageService.AutoCompletion.UsingMSBuild.Filenames.MayBeDifferentlyCased() in D:\j\w\release_ci_pa---3f142ccc\vsintegration\tests\unittests\Tests.LanguageService.Completion.fs:line 4357

auduchinok · 2017-10-02T22:15:42Z

@dsyme Thank you!
I'll look into this failure tomorrow.

… (builder had been created by ParseFileInProject)

dsyme · 2017-10-02T22:39:00Z

2f3298c removes the use of CheckFileInProjectAllowingStaleCachedResults in our old code (which is what these tests are running) in favour of CheckFileInProject. This may however cause other tests to fail.

If that doesn't work then e0b2a13 and 569e076 deprecate the test which is relying on really obscure behaviour of the combination of ParseFileInProject and the deprecated CheckFileInProjectAllowingStaleCachedResults

…ltsBeforeFileInProjectEvenIfStale

KevinRansom · 2017-10-02T22:46:01Z

@dsyme, @cartermp 2017.5 Peview 2.0 is next and baked. I think preview 3.0 this PR alongside the project reference stuff for dotnet core would be aggressive but useful, and give us some time to stabilize it in the product before the actual release.

Kevin

Krzysztof-Cieslak · 2017-10-02T22:50:06Z

I'm happy to test it on Ionide users as soon as it gets merged and new version of FCS is released ;-)

dsyme · 2017-10-02T22:51:03Z

@KevinRansom We should actually wait a bit before pulling. The PR makes checker.ParseFile the fast entry point, and checker.ParseFileInProject becomes slower since the the FSharpParsingOptions are created repeatedly.

ParseFile is now used by Rider, ParseFileInProject is still used by Visual F# Tools (and presumably Ionide). We should do the extra work to fully deprecate checker.ParseFileInProject and make sure the FSharpParsingOptions are created only once by the Visual F# Tools. Ionide and FsAutoComplete can eventually update when they migrate nuget package

(Unlike other editing tools, Visual F# Tools always uses the latest FCS code, and so we can't integrate changes to FCS until we've updated VF# Tools to adjust for those changes)

KevinRansom · 2017-10-02T22:59:56Z

Maybe I can look at what that would take then.

dsyme · 2017-10-03T00:18:38Z

@KevinRansom I've updated this PR to use ParseFile instead of ParseFileInProject. This means maintaining both a FSharpParsingOptions and FSharpProjectOptions in the table in LanguageService.fs - please take a look at 0af0578

The FSharpParsingOptions is easily computable from FSharpProjectOptions but we just need to make sure we amortize the cost of this and store it in that table.

dsyme · 2017-10-03T00:21:26Z

@cartermp The latest additions should mean that #3404 is fixed, see the removal of the comment here https://github.com/Microsoft/visualfsharp/pull/3601/files#diff-8fcecd80bdf6aa0e75cf6116b43b793bL32

dsyme · 2017-10-03T00:23:15Z

@cartermp The latest additions should mean that #3404 is fixed, see the removal of the comment here https://github.com/Microsoft/visualfsharp/pull/3601/files#diff-8fcecd80bdf6aa0e75cf6116b43b793bL32

And indeed I validated this by hand - I was able to set breakpoints immediately in every file even in the context of VisualFSharp.sln, with no delays at all

cartermp · 2017-10-03T00:28:03Z

Brilliant. I'll have to test this out again this evening.

dsyme · 2017-10-03T00:42:19Z

@Krzysztof-Cieslak I've pushed FCS 16.0.1 incorporating this feature (since it is very close to being incorporated into master) based off this tag and commit: https://github.com/Microsoft/visualfsharp/tree/FCS-16.0.1

cartermp · 2017-10-03T14:06:06Z

🎉

@brettfo is there a way to get a merge over into a 15.5 branch so that we can validate with the latest bits? With nightlies still down (😢 ), this will be more difficult for folks to use, so I think that if we use it against the branch we're working with internally it'll get some decent experience-level validation.

brettfo · 2017-10-04T18:00:06Z

@KevinRansom and I are planning on a merge from master to vs2017-rtm (our shipping branch) soon. Since signing is still down we'll have to request a signed build, but then we can distribute the MSI/VSIX for testing.

dsyme · 2017-10-04T18:30:22Z

@brettfo That would be great

* Generate source for .resx files on build. (#3607) * add build task to generate *.fs from *.resx files * generate source for embedded resources in tests * generate source for embedded resources in FSharp.Editor * generate source for embedded resources in FSharp.LanguageService * generate source for embedded resources in FSharp.ProjectSystem.FSharp * generate source for embedded resources in FSharp.VS.FSI * don't generate non-string resources when <=netstandard1.6 * update baseline error message for tests The error output should be the exception message, not the exception type. * perform up-to-date check before generating *.fs from *.resx * remove non-idiomatic fold for an array comprehension * correct newline replacement * output more friendly error message * throw if boolean value isn't explicitly `true` or `false` * only generate object resource code on non `netstandard1.*` and `netcoreapp1.*` platforms * ensure FSharp.Core specifies a target framework for resource generaton * rename attributes to be non-ambiguous and properly include them * fix order of file items in FSharp.Core * Fix build.cmd for certain always-shown errors (like "unable to find registry key"), improve finding of VS2017 tools, fix DEVGUIDE.md (#3635) * Fix build.com displaying the following error on each run "ERROR: The system was unable to find the specified registry key or value." * Remove warning about reg.exe errors introduced in #3614 (in commit b548bd7, but unrelated to that fix), it is no longer necessary. * Fix #3638, VS2017 Preview installation was not found when VS2017RTM is not installed. Remove comment introduced in #3614 (through commit 966bd7f) * Fixing JaroWinkler tests with InvariantCulture and fixing async tests by removing Debugger.Break() (#3627) * Fixing JaroWinkler tests to use InvariantCulture for number-to-string * Fixing the crashing of test runners because of a Debugger.Break() in a test * update to System.Collections.Immutable 1.3.1 (#3641) * update to System.Collections.Immutable 1.3.1 * fixes * fix assembly reference * [WIP] Adds optimized emit for int64 constants (#3620) * Adds optimized emit for int64 constants. * Adds comment linking to the changing PR. Thanks for this PR. Kevin * fix assembly reference (#3646) * Remove a few more build warnings (#3647) * fix assembly reference * remove more build warnings * fix build * move BuildFromSource projects to their own directory * Adds tests for emitted IL for new Int64 constants. (#3650) * Enable FS as prefix and ignore invalid values for warnings (#3631) * enable fs as prefix and ignore invalid values for warnings + tests * Allow #pragma to validate warnings * do it right * use ordinal compare * In both places * Add fs prefix to warnaserror * Fixup tests * Fix stack overflow on assembly resolution (#3658) * Fix stack overflow on tp assembly resolution * Feedback * Add impl files to file check results (#3659) * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project * add ImplementationFiles to FSharpCheckFileResults * make FSharpImplementationFileContents ctor internal * throw if ImplementationFiles is called having keepAssemblyContents flag set to false * add a test * spelling and cosmetics * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd (#3672) * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * Remove ambiguous an irrelevant instruction, improved help and instructions * Fix a scenario where the return code wasn't nonzero for error conditions, fixes not creating backup dir when not backing up * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project (#3657) * bump FCS version (#3676) * bump version * Update RELEASE_NOTES.md * Parsing improvements: no reactor, add parsing options, error severity options (#3601) * Parse without reactor, add parsing options, error severity options * Revert parsing APIs (fallback to the new ones), fix VFT projects * Cache parse results after type check * Add impl files to file check results (#3659) * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project * add ImplementationFiles to FSharpCheckFileResults * make FSharpImplementationFileContents ctor internal * throw if ImplementationFiles is called having keepAssemblyContents flag set to false * add a test * spelling and cosmetics * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd (#3672) * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * Remove ambiguous an irrelevant instruction, improved help and instructions * Fix a scenario where the return code wasn't nonzero for error conditions, fixes not creating backup dir when not backing up * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project (#3657) * bump FCS version (#3676) * bump version * Update RELEASE_NOTES.md * updates to make tests pass * restore old behaviour of CheckFileInProjectAllowingStaleCachedResults (builder had been created by ParseFileInProject) * restore use of CheckFileInProjectAllowingStaleCachedResults * deprecate test relying on whacky behaviour of deprecated GetCheckResultsBeforeFileInProjectEvenIfStale * Use ParseFile and FSharpParsingOptions instead of ParseFileInProject * prepare FCS release with this feature * whitespace cleanup (#3682) * whitespace and docs (#3684) * Preserve XML docs for in-memory project references (#3683) * fix xmldocs for in-memory project references * add test * fix tests * whitespace and comments (#3686) * fix assembly reference * whitespace and comments * whitespace and comments * whitespace and comments * cherry pick two PRs from FCS (#3687) * fix assembly reference * remove line endings from all *.nuspec files * ProjectCracker returns *.fsi files in FSharpProjectOptions.SourceFiles array (in addition to *.fs files, in right order) * ProjectCracker raises exception if ProjectCrackerTool returns non null ProjectCrackerOptions.Error (new field) * fix build on linux * fix a test * slashes * revert slashes * Update FSharp.Compiler.Service.ProjectCracker.nuspec * try to fix travis * try to fix travis * list dependencies * no obsolete pdb in nuget * list dependencies * cherry pick of fsharp/fsharp change * bump FCS version number (#3688) * Update FSharp.Compiler.Service.MSBuild.v12.nuspec * fix FCS nuget on windows * fix-resource (#3690) * Bump FSharp.Compiler.Tools to 4.1.27 and align mono build files (#3693) * ri change from fsharp * fix test * bump FSC tools to 4.1.27 * remove fsharp.core from Mono GAC * align mono directory * fix typo * install back versions with Mono * fix typo * update FCS doc generation (#3694) * update DEVGUIDE to add addiitional steps before running build (#3725) * Split templates out into a seperate vsix (#3720) * merge error * Merge issues

… options (dotnet#3601) * Parse without reactor, add parsing options, error severity options * Revert parsing APIs (fallback to the new ones), fix VFT projects * Cache parse results after type check * Add impl files to file check results (dotnet#3659) * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project * add ImplementationFiles to FSharpCheckFileResults * make FSharpImplementationFileContents ctor internal * throw if ImplementationFiles is called having keepAssemblyContents flag set to false * add a test * spelling and cosmetics * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd (dotnet#3672) * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * Remove ambiguous an irrelevant instruction, improved help and instructions * Fix a scenario where the return code wasn't nonzero for error conditions, fixes not creating backup dir when not backing up * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project (dotnet#3657) * bump FCS version (dotnet#3676) * bump version * Update RELEASE_NOTES.md * updates to make tests pass * restore old behaviour of CheckFileInProjectAllowingStaleCachedResults (builder had been created by ParseFileInProject) * restore use of CheckFileInProjectAllowingStaleCachedResults * deprecate test relying on whacky behaviour of deprecated GetCheckResultsBeforeFileInProjectEvenIfStale * Use ParseFile and FSharpParsingOptions instead of ParseFileInProject * prepare FCS release with this feature

* Generate source for .resx files on build. (dotnet#3607) * add build task to generate *.fs from *.resx files * generate source for embedded resources in tests * generate source for embedded resources in FSharp.Editor * generate source for embedded resources in FSharp.LanguageService * generate source for embedded resources in FSharp.ProjectSystem.FSharp * generate source for embedded resources in FSharp.VS.FSI * don't generate non-string resources when <=netstandard1.6 * update baseline error message for tests The error output should be the exception message, not the exception type. * perform up-to-date check before generating *.fs from *.resx * remove non-idiomatic fold for an array comprehension * correct newline replacement * output more friendly error message * throw if boolean value isn't explicitly `true` or `false` * only generate object resource code on non `netstandard1.*` and `netcoreapp1.*` platforms * ensure FSharp.Core specifies a target framework for resource generaton * rename attributes to be non-ambiguous and properly include them * fix order of file items in FSharp.Core * Fix build.cmd for certain always-shown errors (like "unable to find registry key"), improve finding of VS2017 tools, fix DEVGUIDE.md (dotnet#3635) * Fix build.com displaying the following error on each run "ERROR: The system was unable to find the specified registry key or value." * Remove warning about reg.exe errors introduced in dotnet#3614 (in commit b548bd7, but unrelated to that fix), it is no longer necessary. * Fix dotnet#3638, VS2017 Preview installation was not found when VS2017RTM is not installed. Remove comment introduced in dotnet#3614 (through commit 966bd7f) * Fixing JaroWinkler tests with InvariantCulture and fixing async tests by removing Debugger.Break() (dotnet#3627) * Fixing JaroWinkler tests to use InvariantCulture for number-to-string * Fixing the crashing of test runners because of a Debugger.Break() in a test * update to System.Collections.Immutable 1.3.1 (dotnet#3641) * update to System.Collections.Immutable 1.3.1 * fixes * fix assembly reference * [WIP] Adds optimized emit for int64 constants (dotnet#3620) * Adds optimized emit for int64 constants. * Adds comment linking to the changing PR. Thanks for this PR. Kevin * fix assembly reference (dotnet#3646) * Remove a few more build warnings (dotnet#3647) * fix assembly reference * remove more build warnings * fix build * move BuildFromSource projects to their own directory * Adds tests for emitted IL for new Int64 constants. (dotnet#3650) * Enable FS as prefix and ignore invalid values for warnings (dotnet#3631) * enable fs as prefix and ignore invalid values for warnings + tests * Allow #pragma to validate warnings * do it right * use ordinal compare * In both places * Add fs prefix to warnaserror * Fixup tests * Fix stack overflow on assembly resolution (dotnet#3658) * Fix stack overflow on tp assembly resolution * Feedback * Add impl files to file check results (dotnet#3659) * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project * add ImplementationFiles to FSharpCheckFileResults * make FSharpImplementationFileContents ctor internal * throw if ImplementationFiles is called having keepAssemblyContents flag set to false * add a test * spelling and cosmetics * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd (dotnet#3672) * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * Remove ambiguous an irrelevant instruction, improved help and instructions * Fix a scenario where the return code wasn't nonzero for error conditions, fixes not creating backup dir when not backing up * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project (dotnet#3657) * bump FCS version (dotnet#3676) * bump version * Update RELEASE_NOTES.md * Parsing improvements: no reactor, add parsing options, error severity options (dotnet#3601) * Parse without reactor, add parsing options, error severity options * Revert parsing APIs (fallback to the new ones), fix VFT projects * Cache parse results after type check * Add impl files to file check results (dotnet#3659) * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project * add ImplementationFiles to FSharpCheckFileResults * make FSharpImplementationFileContents ctor internal * throw if ImplementationFiles is called having keepAssemblyContents flag set to false * add a test * spelling and cosmetics * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd (dotnet#3672) * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * Remove ambiguous an irrelevant instruction, improved help and instructions * Fix a scenario where the return code wasn't nonzero for error conditions, fixes not creating backup dir when not backing up * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project (dotnet#3657) * bump FCS version (dotnet#3676) * bump version * Update RELEASE_NOTES.md * updates to make tests pass * restore old behaviour of CheckFileInProjectAllowingStaleCachedResults (builder had been created by ParseFileInProject) * restore use of CheckFileInProjectAllowingStaleCachedResults * deprecate test relying on whacky behaviour of deprecated GetCheckResultsBeforeFileInProjectEvenIfStale * Use ParseFile and FSharpParsingOptions instead of ParseFileInProject * prepare FCS release with this feature * whitespace cleanup (dotnet#3682) * whitespace and docs (dotnet#3684) * Preserve XML docs for in-memory project references (dotnet#3683) * fix xmldocs for in-memory project references * add test * fix tests * whitespace and comments (dotnet#3686) * fix assembly reference * whitespace and comments * whitespace and comments * whitespace and comments * cherry pick two PRs from FCS (dotnet#3687) * fix assembly reference * remove line endings from all *.nuspec files * ProjectCracker returns *.fsi files in FSharpProjectOptions.SourceFiles array (in addition to *.fs files, in right order) * ProjectCracker raises exception if ProjectCrackerTool returns non null ProjectCrackerOptions.Error (new field) * fix build on linux * fix a test * slashes * revert slashes * Update FSharp.Compiler.Service.ProjectCracker.nuspec * try to fix travis * try to fix travis * list dependencies * no obsolete pdb in nuget * list dependencies * cherry pick of fsharp/fsharp change * bump FCS version number (dotnet#3688) * Update FSharp.Compiler.Service.MSBuild.v12.nuspec * fix FCS nuget on windows * fix-resource (dotnet#3690) * Bump FSharp.Compiler.Tools to 4.1.27 and align mono build files (dotnet#3693) * ri change from fsharp * fix test * bump FSC tools to 4.1.27 * remove fsharp.core from Mono GAC * align mono directory * fix typo * install back versions with Mono * fix typo * update FCS doc generation (dotnet#3694) * update DEVGUIDE to add addiitional steps before running build (dotnet#3725) * Split templates out into a seperate vsix (dotnet#3720) * merge error * Merge issues

msftclas added the cla-already-signed label Sep 18, 2017

auduchinok force-pushed the no-reactor-parsing branch from 61d15c3 to c9cd271 Compare September 18, 2017 17:17

auduchinok force-pushed the no-reactor-parsing branch from 20b8cce to 959fbd9 Compare September 18, 2017 21:15

realvictorprm mentioned this pull request Sep 19, 2017

Improving the compiler is painful #3591

Closed

auduchinok force-pushed the no-reactor-parsing branch from 959fbd9 to c9cd271 Compare September 19, 2017 10:56

auduchinok changed the title ~~[WIP] Parse without reactor, add parsing options, error severity options~~ [WIP] Parsing improvements: no reactor, add parsing options, error severity options Sep 19, 2017

dsyme requested changes Sep 20, 2017

View reviewed changes

auduchinok mentioned this pull request Sep 20, 2017

Replace ParseFileInProject with ParseFile fsharp/fsharp-compiler-docs#806

Closed

auduchinok force-pushed the no-reactor-parsing branch 2 times, most recently from 200d144 to 23a959d Compare September 20, 2017 15:27

dsyme reviewed Sep 20, 2017

View reviewed changes

auduchinok force-pushed the no-reactor-parsing branch 3 times, most recently from 8789950 to be680af Compare September 20, 2017 21:50

dsyme requested changes Sep 20, 2017

View reviewed changes

auduchinok force-pushed the no-reactor-parsing branch 2 times, most recently from 6f4be71 to 87f9642 Compare September 21, 2017 00:37

msftgits removed the cla-already-signed label Sep 26, 2017

dotnet deleted a comment from msftclas Sep 27, 2017

dsyme added 2 commits October 2, 2017 23:33

restore old behaviour of CheckFileInProjectAllowingStaleCachedResults…

2f3298c

… (builder had been created by ParseFileInProject)

restore use of CheckFileInProjectAllowingStaleCachedResults

e0b2a13

deprecate test relying on whacky behaviour of deprecated GetCheckResu…

569e076

…ltsBeforeFileInProjectEvenIfStale

Use ParseFile and FSharpParsingOptions instead of ParseFileInProject

0af0578

prepare FCS release with this feature

9b68c37

dsyme merged commit 4c244da into dotnet:master Oct 3, 2017

auduchinok deleted the no-reactor-parsing branch October 3, 2017 07:14

cartermp mentioned this pull request Oct 7, 2017

Popup "Validating breakpoint location..." waits rather long when removing a breakpoint #3404

Closed

cartermp mentioned this pull request May 31, 2018

Make matched brace highlighting work exactly as in C# editor #5049

Merged

auduchinok mentioned this pull request Oct 13, 2020

[WIP] fsc.exe - Parallel Parsing #10208

Closed

		// This function gets called whenever an error happens during parsing or checking
		let diagnosticSink sev (exn:PhasedDiagnostic) =

Parsing improvements: no reactor, add parsing options, error severity options #3601

Parsing improvements: no reactor, add parsing options, error severity options #3601

Conversation

auduchinok commented Sep 18, 2017 • edited Loading

realvictorprm commented Sep 18, 2017

vasily-kirichenko commented Sep 18, 2017

realvictorprm commented Sep 18, 2017

auduchinok commented Sep 18, 2017 • edited Loading

saul commented Sep 18, 2017

cartermp commented Sep 18, 2017

dsyme commented Sep 20, 2017

dsyme left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

auduchinok commented Sep 20, 2017

dsyme commented Sep 20, 2017

dsyme commented Sep 20, 2017

Choose a reason for hiding this comment

auduchinok Sep 20, 2017 • edited Loading

Choose a reason for hiding this comment

auduchinok Sep 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsyme commented Sep 20, 2017

dsyme left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsyme Sep 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cartermp commented Oct 2, 2017

dsyme commented Oct 2, 2017 • edited Loading

dsyme commented Oct 2, 2017 • edited Loading

auduchinok commented Oct 2, 2017

dsyme commented Oct 2, 2017 • edited Loading

KevinRansom commented Oct 2, 2017

Krzysztof-Cieslak commented Oct 2, 2017

dsyme commented Oct 2, 2017

KevinRansom commented Oct 2, 2017

dsyme commented Oct 3, 2017

dsyme commented Oct 3, 2017

dsyme commented Oct 3, 2017

cartermp commented Oct 3, 2017

dsyme commented Oct 3, 2017

cartermp commented Oct 3, 2017

brettfo commented Oct 4, 2017

dsyme commented Oct 4, 2017

auduchinok commented Sep 18, 2017 •

edited

Loading

auduchinok commented Sep 18, 2017 •

edited

Loading

auduchinok Sep 20, 2017 •

edited

Loading

auduchinok Sep 20, 2017 •

edited

Loading

dsyme Sep 20, 2017 •

edited

Loading

dsyme commented Oct 2, 2017 •

edited

Loading

dsyme commented Oct 2, 2017 •

edited

Loading

dsyme commented Oct 2, 2017 •

edited

Loading