Fix17713 - Reverting PR - 17649 - Make the interaction between #line and #nowarn directives consistent (#17724)

* fix

* temp

* revert

* readme

---------

Co-authored-by: Vlad Zarytovskii <[email protected]>

Author: KevinRansom

buildtools/fsyacc/fsyaccdriver.fs

@@ -199,8 +199,6 @@ let writeSpecToFile (generatorState: GeneratorState) (spec: ParserSpec) (compile
               writer.WriteLineInterface "module %s" s;
 
       writer.WriteLine "#nowarn \"64\";; // turn off warnings that type variables used in production annotations are instantiated to concrete type";
-      writer.WriteLine "#nowarn \"1182\"  // the generated code often has unused variable 'parseState'"
-      writer.WriteLine "#nowarn \"3261\"  // the generated code would need to properly annotate nulls, e.g. changing System.Object to `obj|null`";
 
       for s in generatorState.opens do
           writer.WriteLine          "open %s" s;

docs/release-notes/.FSharp.Compiler.Service/9.0.100.md

@@ -1,6 +1,5 @@
 ### Fixed
 
-* Fix a bug in the interaction between ``#line` and `#nowarn` directives ([PR #17649](https://github.com/dotnet/fsharp/pull/17649))
 * Fix wrong TailCall warning ([Issue #17604](https://github.com/dotnet/fsharp/issues/17604), [PR #17637](https://github.com/dotnet/fsharp/pull/17637))
 * Compiler hangs when compiling inline recursive invocation ([Issue #17376](https://github.com/dotnet/fsharp/issues/17376), [PR #17394](https://github.com/dotnet/fsharp/pull/17394))
 * Fix reporting IsFromComputationExpression only for CE builder type constructors and let bindings. ([PR #17375](https://github.com/dotnet/fsharp/pull/17375))

src/Compiler/AbstractIL/ilpars.fsy

@@ -2,7 +2,6 @@
 
 %{
 
-#nowarn "64" // turn off warnings that type variables used in production annotations are instantiated to concrete type
 #nowarn "1182"  // the generated code often has unused variable "parseState"
 #nowarn "3261"  // the generated code would need to properly annotate nulls, e.g. changing System.Object to `obj|null`
 

src/Compiler/Driver/CompilerDiagnostics.fs

@@ -24,7 +24,6 @@ open FSharp.Compiler.ConstraintSolver
 open FSharp.Compiler.DiagnosticMessage
 open FSharp.Compiler.Diagnostics
 open FSharp.Compiler.DiagnosticsLogger
-open FSharp.Compiler.Features
 open FSharp.Compiler.Infos
 open FSharp.Compiler.IO
 open FSharp.Compiler.Lexhelp
@@ -2300,13 +2299,17 @@ type PhasedDiagnostic with
 // Scoped #nowarn pragmas
 
 /// Build an DiagnosticsLogger that delegates to another DiagnosticsLogger but filters warnings turned off by the given pragma declarations
+//
+// NOTE: we allow a flag to turn of strict file checking. This is because file names sometimes don't match due to use of
+// #line directives, e.g. for pars.fs/pars.fsy. In this case we just test by line number - in most cases this is sufficient
+// because we install a filtering error handler on a file-by-file basis for parsing and type-checking.
+// However this is indicative of a more systematic problem where source-line
+// sensitive operations (lexfilter and warning filtering) do not always
+// interact well with #line directives.
 type DiagnosticsLoggerFilteringByScopedPragmas
-    (langVersion: LanguageVersion, scopedPragmas, diagnosticOptions: FSharpDiagnosticOptions, diagnosticsLogger: DiagnosticsLogger) =
+    (checkFile, scopedPragmas, diagnosticOptions: FSharpDiagnosticOptions, diagnosticsLogger: DiagnosticsLogger) =
     inherit DiagnosticsLogger("DiagnosticsLoggerFilteringByScopedPragmas")
 
-    let needCompatibilityWithEarlierInconsistentInteraction =
-        not (langVersion.SupportsFeature LanguageFeature.ConsistentNowarnLineDirectiveInteraction)
-
     let mutable realErrorPresent = false
 
     override _.DiagnosticSink(diagnostic: PhasedDiagnostic, severity) =
@@ -2320,10 +2323,12 @@ type DiagnosticsLoggerFilteringByScopedPragmas
                 match diagnostic.Range with
                 | Some m ->
                     scopedPragmas
-                    |> List.exists (fun (ScopedPragma.WarningOff(pragmaRange, warningNumFromPragma)) ->
+                    |> List.exists (fun pragma ->
+                        let (ScopedPragma.WarningOff(pragmaRange, warningNumFromPragma)) = pragma
+
                         warningNum = warningNumFromPragma
-                        && (needCompatibilityWithEarlierInconsistentInteraction
-                            || m.FileIndex = pragmaRange.FileIndex && posGeq m.Start pragmaRange.Start))
+                        && (not checkFile || m.FileIndex = pragmaRange.FileIndex)
+                        && posGeq m.Start pragmaRange.Start)
                     |> not
                 | None -> true
 
@@ -2339,5 +2344,5 @@ type DiagnosticsLoggerFilteringByScopedPragmas
 
     override _.CheckForRealErrorsIgnoringWarnings = realErrorPresent
 
-let GetDiagnosticsLoggerFilteringByScopedPragmas (langVersion, scopedPragmas, diagnosticOptions, diagnosticsLogger) =
-    DiagnosticsLoggerFilteringByScopedPragmas(langVersion, scopedPragmas, diagnosticOptions, diagnosticsLogger) :> DiagnosticsLogger
+let GetDiagnosticsLoggerFilteringByScopedPragmas (checkFile, scopedPragmas, diagnosticOptions, diagnosticsLogger) =
+    DiagnosticsLoggerFilteringByScopedPragmas(checkFile, scopedPragmas, diagnosticOptions, diagnosticsLogger) :> DiagnosticsLogger

src/Compiler/Driver/CompilerDiagnostics.fsi

@@ -7,7 +7,6 @@ open System.Text
 open FSharp.Compiler.CompilerConfig
 open FSharp.Compiler.Diagnostics
 open FSharp.Compiler.DiagnosticsLogger
-open FSharp.Compiler.Features
 open FSharp.Compiler.Syntax
 open FSharp.Compiler.Text
 
@@ -85,7 +84,7 @@ type PhasedDiagnostic with
 
 /// Get a diagnostics logger that filters the reporting of warnings based on scoped pragma information
 val GetDiagnosticsLoggerFilteringByScopedPragmas:
-    langVersion: LanguageVersion *
+    checkFile: bool *
     scopedPragmas: ScopedPragma list *
     diagnosticOptions: FSharpDiagnosticOptions *
     diagnosticsLogger: DiagnosticsLogger ->

src/Compiler/Driver/ParseAndCheckInputs.fs

@@ -511,7 +511,7 @@ let ParseInput
     finally
         // OK, now commit the errors, since the ScopedPragmas will (hopefully) have been scraped
         let filteringDiagnosticsLogger =
-            GetDiagnosticsLoggerFilteringByScopedPragmas(lexbuf.LanguageVersion, scopedPragmas, diagnosticOptions, diagnosticsLogger)
+            GetDiagnosticsLoggerFilteringByScopedPragmas(false, scopedPragmas, diagnosticOptions, diagnosticsLogger)
 
         delayLogger.CommitDelayedDiagnostics filteringDiagnosticsLogger
 
@@ -1429,7 +1429,7 @@ let CheckOneInput
 
 // Within a file, equip loggers to locally filter w.r.t. scope pragmas in each input
 let DiagnosticsLoggerForInput (tcConfig: TcConfig, input: ParsedInput, oldLogger) =
-    GetDiagnosticsLoggerFilteringByScopedPragmas(tcConfig.langVersion, input.ScopedPragmas, tcConfig.diagnosticsOptions, oldLogger)
+    GetDiagnosticsLoggerFilteringByScopedPragmas(false, input.ScopedPragmas, tcConfig.diagnosticsOptions, oldLogger)
 
 /// Typecheck a single file (or interactive entry into F# Interactive)
 let CheckOneInputEntry (ctok, checkForErrors, tcConfig: TcConfig, tcImports, tcGlobals, prefixPathOpt) tcState input =

src/Compiler/Driver/fsc.fs

@@ -745,7 +745,7 @@ let main2
                     yield! pragmas
             ]
 
-        GetDiagnosticsLoggerFilteringByScopedPragmas(tcConfig.langVersion, scopedPragmas, tcConfig.diagnosticsOptions, oldLogger)
+        GetDiagnosticsLoggerFilteringByScopedPragmas(true, scopedPragmas, tcConfig.diagnosticsOptions, oldLogger)
 
     SetThreadDiagnosticsLoggerNoUnwind diagnosticsLogger
 

src/Compiler/FSComp.txt

@@ -1783,5 +1783,4 @@ featureEmptyBodiedComputationExpressions,"Support for computation expressions wi
 featureAllowAccessModifiersToAutoPropertiesGettersAndSetters,"Allow access modifiers to auto properties getters and setters"
 3871,tcAccessModifiersNotAllowedInSRTPConstraint,"Access modifiers cannot be applied to an SRTP constraint."
 featureAllowObjectExpressionWithoutOverrides,"Allow object expressions without overrides"
-3872,tcPartialActivePattern,"Multi-case partial active patterns are not supported. Consider using a single-case partial active pattern or a full active pattern."
-featureConsistentNowarnLineDirectiveInteraction,"The interaction between #nowarn and #line is now consistent."
+3872,tcPartialActivePattern,"Multi-case partial active patterns are not supported. Consider using a single-case partial active pattern or a full active pattern."

src/Compiler/Facilities/LanguageFeatures.fs

@@ -94,7 +94,6 @@ type LanguageFeature =
     | ParsedHashDirectiveArgumentNonQuotes
     | EmptyBodiedComputationExpressions
     | AllowObjectExpressionWithoutOverrides
-    | ConsistentNowarnLineDirectiveInteraction
 
 /// LanguageVersion management
 type LanguageVersion(versionText) =
@@ -213,7 +212,6 @@ type LanguageVersion(versionText) =
                 LanguageFeature.LowerSimpleMappingsInComprehensionsToFastLoops, languageVersion90
                 LanguageFeature.ParsedHashDirectiveArgumentNonQuotes, languageVersion90
                 LanguageFeature.EmptyBodiedComputationExpressions, languageVersion90
-                LanguageFeature.ConsistentNowarnLineDirectiveInteraction, languageVersion90
 
                 // F# preview
                 LanguageFeature.EnforceAttributeTargets, previewVersion // waiting for fix of https://github.com/dotnet/fsharp/issues/17731
@@ -377,7 +375,6 @@ type LanguageVersion(versionText) =
         | LanguageFeature.ParsedHashDirectiveArgumentNonQuotes -> FSComp.SR.featureParsedHashDirectiveArgumentNonString ()
         | LanguageFeature.EmptyBodiedComputationExpressions -> FSComp.SR.featureEmptyBodiedComputationExpressions ()
         | LanguageFeature.AllowObjectExpressionWithoutOverrides -> FSComp.SR.featureAllowObjectExpressionWithoutOverrides ()
-        | LanguageFeature.ConsistentNowarnLineDirectiveInteraction -> FSComp.SR.featureConsistentNowarnLineDirectiveInteraction ()
 
     /// Get a version string associated with the given feature.
     static member GetFeatureVersionString feature =

src/Compiler/Facilities/LanguageFeatures.fsi

@@ -85,7 +85,6 @@ type LanguageFeature =
     | ParsedHashDirectiveArgumentNonQuotes
     | EmptyBodiedComputationExpressions
     | AllowObjectExpressionWithoutOverrides
-    | ConsistentNowarnLineDirectiveInteraction
 
 /// LanguageVersion management
 type LanguageVersion =

src/Compiler/Service/IncrementalBuild.fs

@@ -259,7 +259,7 @@ type BoundModel private (
 
             IncrementalBuilderEventTesting.MRU.Add(IncrementalBuilderEventTesting.IBETypechecked fileName)
             let capturingDiagnosticsLogger = CapturingDiagnosticsLogger("TypeCheck")
-            let diagnosticsLogger = GetDiagnosticsLoggerFilteringByScopedPragmas(tcConfig.langVersion, input.ScopedPragmas, tcConfig.diagnosticsOptions, capturingDiagnosticsLogger)
+            let diagnosticsLogger = GetDiagnosticsLoggerFilteringByScopedPragmas(false, input.ScopedPragmas, tcConfig.diagnosticsOptions, capturingDiagnosticsLogger)
             use _ = new CompilationGlobalsScope(diagnosticsLogger, BuildPhase.TypeCheck)
 
             beforeFileChecked.Trigger fileName

src/Compiler/Service/TransparentCompiler.fs

@@ -1303,12 +1303,7 @@ type internal TransparentCompiler
                 let diagnosticsLogger = errHandler.DiagnosticsLogger
 
                 let diagnosticsLogger =
-                    GetDiagnosticsLoggerFilteringByScopedPragmas(
-                        tcConfig.langVersion,
-                        input.ScopedPragmas,
-                        tcConfig.diagnosticsOptions,
-                        diagnosticsLogger
-                    )
+                    GetDiagnosticsLoggerFilteringByScopedPragmas(false, input.ScopedPragmas, tcConfig.diagnosticsOptions, diagnosticsLogger)
 
                 use _ = new CompilationGlobalsScope(diagnosticsLogger, BuildPhase.TypeCheck)
 

src/Compiler/pars.fsy

@@ -2,7 +2,6 @@
 
 %{
 
-#nowarn "64" // turn off warnings that type variables used in production annotations are instantiated to concrete type
 #nowarn "1182"  // generated code has lots of unused "parseState"
 #nowarn "3261"  // the generated code would need to properly annotate nulls, e.g. changing System.Object to `obj|null`
 

src/Compiler/pppars.fsy

@@ -3,7 +3,6 @@
 %{
 open FSharp.Compiler.DiagnosticsLogger
 
-#nowarn "64" // turn off warnings that type variables used in production annotations are instantiated to concrete type
 #nowarn "3261"  // the generated code would need to properly annotate nulls, e.g. changing System.Object to `obj|null`
 
 let dummy       = IfdefId("DUMMY")