Make the interaction between #line and #nowarn directives consistent (#…

…17649) * fix the checkFile bug * change the feature flag name * updated xlf files * Another small name change --------- Co-authored-by: Petr <psfinaki@users.noreply.github.com>
dotnet · Sep 10, 2024 · ac3d3c8 · ac3d3c8
1 parent e42cff6
commit ac3d3c8
Show file tree

Hide file tree

Showing 30 changed files with 152 additions and 22 deletions.
diff --git a/buildtools/fsyacc/fsyaccdriver.fs b/buildtools/fsyacc/fsyaccdriver.fs
@@ -199,6 +199,8 @@ 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;

diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md
@@ -1,5 +1,6 @@
 ### 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))

diff --git a/docs/release-notes/.Language/preview.md b/docs/release-notes/.Language/preview.md
@@ -21,6 +21,7 @@
 * Enforce AttributeTargets on union case declarations. ([PR #16764](https://github.com/dotnet/fsharp/pull/16764))
 * Enforce AttributeTargets on implicit constructors. ([PR #16845](https://github.com/dotnet/fsharp/pull/16845/))
 * Enforce AttributeTargets on structs and classes ([PR #16790](https://github.com/dotnet/fsharp/pull/16790))
+* Ensure consistent interaction between ``#line` and `#nowarn` directives ([PR #17649](https://github.com/dotnet/fsharp/pull/17649))
 
 ### Changed
 

diff --git a/src/Compiler/AbstractIL/ilpars.fsy b/src/Compiler/AbstractIL/ilpars.fsy
@@ -2,6 +2,7 @@
 
 %{
 
+#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`
 

diff --git a/src/Compiler/Driver/CompilerDiagnostics.fs b/src/Compiler/Driver/CompilerDiagnostics.fs
@@ -24,6 +24,7 @@ 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
@@ -2299,17 +2300,13 @@ 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
-    (checkFile, scopedPragmas, diagnosticOptions: FSharpDiagnosticOptions, diagnosticsLogger: DiagnosticsLogger) =
+    (langVersion: LanguageVersion, scopedPragmas, diagnosticOptions: FSharpDiagnosticOptions, diagnosticsLogger: DiagnosticsLogger) =
     inherit DiagnosticsLogger("DiagnosticsLoggerFilteringByScopedPragmas")
 
+    let needCompatibilityWithEarlierInconsistentInteraction =
+        not (langVersion.SupportsFeature LanguageFeature.ConsistentNowarnLineDirectiveInteraction)
+
     let mutable realErrorPresent = false
 
     override _.DiagnosticSink(diagnostic: PhasedDiagnostic, severity) =
@@ -2323,12 +2320,10 @@ type DiagnosticsLoggerFilteringByScopedPragmas
                 match diagnostic.Range with
                 | Some m ->
                     scopedPragmas
-                    |> List.exists (fun pragma ->
-                        let (ScopedPragma.WarningOff(pragmaRange, warningNumFromPragma)) = pragma
-
+                    |> List.exists (fun (ScopedPragma.WarningOff(pragmaRange, warningNumFromPragma)) ->
                         warningNum = warningNumFromPragma
-                        && (not checkFile || m.FileIndex = pragmaRange.FileIndex)
-                        && posGeq m.Start pragmaRange.Start)
+                        && (needCompatibilityWithEarlierInconsistentInteraction
+                            || m.FileIndex = pragmaRange.FileIndex && posGeq m.Start pragmaRange.Start))
                     |> not
                 | None -> true
 
@@ -2344,5 +2339,5 @@ type DiagnosticsLoggerFilteringByScopedPragmas
 
     override _.CheckForRealErrorsIgnoringWarnings = realErrorPresent
 
-let GetDiagnosticsLoggerFilteringByScopedPragmas (checkFile, scopedPragmas, diagnosticOptions, diagnosticsLogger) =
-    DiagnosticsLoggerFilteringByScopedPragmas(checkFile, scopedPragmas, diagnosticOptions, diagnosticsLogger) :> DiagnosticsLogger
+let GetDiagnosticsLoggerFilteringByScopedPragmas (langVersion, scopedPragmas, diagnosticOptions, diagnosticsLogger) =
+    DiagnosticsLoggerFilteringByScopedPragmas(langVersion, scopedPragmas, diagnosticOptions, diagnosticsLogger) :> DiagnosticsLogger
diff --git a/src/Compiler/Driver/CompilerDiagnostics.fsi b/src/Compiler/Driver/CompilerDiagnostics.fsi
@@ -7,6 +7,7 @@ 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
 
@@ -84,7 +85,7 @@ type PhasedDiagnostic with
 
 /// Get a diagnostics logger that filters the reporting of warnings based on scoped pragma information
 val GetDiagnosticsLoggerFilteringByScopedPragmas:
-    checkFile: bool *
+    langVersion: LanguageVersion *
     scopedPragmas: ScopedPragma list *
     diagnosticOptions: FSharpDiagnosticOptions *
     diagnosticsLogger: DiagnosticsLogger ->

diff --git a/src/Compiler/Driver/ParseAndCheckInputs.fs b/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(false, scopedPragmas, diagnosticOptions, diagnosticsLogger)
+            GetDiagnosticsLoggerFilteringByScopedPragmas(lexbuf.LanguageVersion, 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(false, input.ScopedPragmas, tcConfig.diagnosticsOptions, oldLogger)
+    GetDiagnosticsLoggerFilteringByScopedPragmas(tcConfig.langVersion, 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 =

diff --git a/src/Compiler/Driver/fsc.fs b/src/Compiler/Driver/fsc.fs
@@ -745,7 +745,7 @@ let main2
                     yield! pragmas
             ]
 
-        GetDiagnosticsLoggerFilteringByScopedPragmas(true, scopedPragmas, tcConfig.diagnosticsOptions, oldLogger)
+        GetDiagnosticsLoggerFilteringByScopedPragmas(tcConfig.langVersion, scopedPragmas, tcConfig.diagnosticsOptions, oldLogger)
 
     SetThreadDiagnosticsLoggerNoUnwind diagnosticsLogger
 

diff --git a/src/Compiler/FSComp.txt b/src/Compiler/FSComp.txt
@@ -1783,4 +1783,5 @@ 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."
+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."
diff --git a/src/Compiler/Facilities/LanguageFeatures.fs b/src/Compiler/Facilities/LanguageFeatures.fs
@@ -94,6 +94,7 @@ type LanguageFeature =
     | ParsedHashDirectiveArgumentNonQuotes
     | EmptyBodiedComputationExpressions
     | AllowObjectExpressionWithoutOverrides
+    | ConsistentNowarnLineDirectiveInteraction
 
 /// LanguageVersion management
 type LanguageVersion(versionText) =
@@ -213,6 +214,7 @@ type LanguageVersion(versionText) =
                 LanguageFeature.ParsedHashDirectiveArgumentNonQuotes, languageVersion90
                 LanguageFeature.EmptyBodiedComputationExpressions, languageVersion90
                 LanguageFeature.EnforceAttributeTargets, languageVersion90
+                LanguageFeature.ConsistentNowarnLineDirectiveInteraction, languageVersion90
 
                 // F# preview
                 LanguageFeature.UnmanagedConstraintCsharpInterop, previewVersion // not enabled because: https://github.com/dotnet/fsharp/issues/17509
@@ -375,6 +377,7 @@ 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 =

diff --git a/src/Compiler/Facilities/LanguageFeatures.fsi b/src/Compiler/Facilities/LanguageFeatures.fsi
@@ -85,6 +85,7 @@ type LanguageFeature =
     | ParsedHashDirectiveArgumentNonQuotes
     | EmptyBodiedComputationExpressions
     | AllowObjectExpressionWithoutOverrides
+    | ConsistentNowarnLineDirectiveInteraction
 
 /// LanguageVersion management
 type LanguageVersion =

diff --git a/src/Compiler/Service/IncrementalBuild.fs b/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(false, input.ScopedPragmas, tcConfig.diagnosticsOptions, capturingDiagnosticsLogger)
+            let diagnosticsLogger = GetDiagnosticsLoggerFilteringByScopedPragmas(tcConfig.langVersion, input.ScopedPragmas, tcConfig.diagnosticsOptions, capturingDiagnosticsLogger)
             use _ = new CompilationGlobalsScope(diagnosticsLogger, BuildPhase.TypeCheck)
 
             beforeFileChecked.Trigger fileName

diff --git a/src/Compiler/Service/TransparentCompiler.fs b/src/Compiler/Service/TransparentCompiler.fs
@@ -1299,7 +1299,12 @@ type internal TransparentCompiler
                 let diagnosticsLogger = errHandler.DiagnosticsLogger
 
                 let diagnosticsLogger =
-                    GetDiagnosticsLoggerFilteringByScopedPragmas(false, input.ScopedPragmas, tcConfig.diagnosticsOptions, diagnosticsLogger)
+                    GetDiagnosticsLoggerFilteringByScopedPragmas(
+                        tcConfig.langVersion,
+                        input.ScopedPragmas,
+                        tcConfig.diagnosticsOptions,
+                        diagnosticsLogger
+                    )
 
                 use _ = new CompilationGlobalsScope(diagnosticsLogger, BuildPhase.TypeCheck)
 

diff --git a/src/Compiler/pars.fsy b/src/Compiler/pars.fsy
@@ -2,6 +2,7 @@
 
 %{
 
+#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`
 

diff --git a/src/Compiler/pppars.fsy b/src/Compiler/pppars.fsy
@@ -3,6 +3,7 @@
 %{
 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")

diff --git a/src/Compiler/xlf/FSComp.txt.cs.xlf b/src/Compiler/xlf/FSComp.txt.cs.xlf
diff --git a/src/Compiler/xlf/FSComp.txt.de.xlf b/src/Compiler/xlf/FSComp.txt.de.xlf
diff --git a/src/Compiler/xlf/FSComp.txt.es.xlf b/src/Compiler/xlf/FSComp.txt.es.xlf
diff --git a/src/Compiler/xlf/FSComp.txt.fr.xlf b/src/Compiler/xlf/FSComp.txt.fr.xlf
diff --git a/src/Compiler/xlf/FSComp.txt.it.xlf b/src/Compiler/xlf/FSComp.txt.it.xlf
diff --git a/src/Compiler/xlf/FSComp.txt.ja.xlf b/src/Compiler/xlf/FSComp.txt.ja.xlf
diff --git a/src/Compiler/xlf/FSComp.txt.ko.xlf b/src/Compiler/xlf/FSComp.txt.ko.xlf
diff --git a/src/Compiler/xlf/FSComp.txt.pl.xlf b/src/Compiler/xlf/FSComp.txt.pl.xlf
diff --git a/src/Compiler/xlf/FSComp.txt.pt-BR.xlf b/src/Compiler/xlf/FSComp.txt.pt-BR.xlf
diff --git a/src/Compiler/xlf/FSComp.txt.ru.xlf b/src/Compiler/xlf/FSComp.txt.ru.xlf
diff --git a/src/Compiler/xlf/FSComp.txt.tr.xlf b/src/Compiler/xlf/FSComp.txt.tr.xlf