Skip to content

Commit

Permalink
Part 2 of "handle line directives correctly in scoped nowarn"
Browse files Browse the repository at this point in the history
  • Loading branch information
Martin521 committed Aug 18, 2024
1 parent ee7ce42 commit 976c159
Show file tree
Hide file tree
Showing 16 changed files with 95 additions and 34 deletions.
4 changes: 3 additions & 1 deletion buildtools/fsyacc/fsyaccdriver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,9 @@ let writeSpecToFile (generatorState: GeneratorState) (spec: ParserSpec) (compile
writer.WriteLine "module %s" s;
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 \"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;
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/AbstractIL/ilpars.fsy
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
14 changes: 9 additions & 5 deletions src/Compiler/Driver/CompilerDiagnostics.fs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.

/// Contains logic to prepare, post-process, filter and emit compiler diagnsotics
/// Contains logic to prepare, post-process, filter and emit compiler diagnostics
module internal FSharp.Compiler.CompilerDiagnostics

open System
Expand All @@ -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
Expand Down Expand Up @@ -2238,9 +2239,12 @@ type PhasedDiagnostic with
// sensitive operations (lexfilter and warning filtering) do not always
// interact well with #line directives.
type DiagnosticsLoggerFilteringByScopedPragmas
(checkFile, scopedPragmas, diagnosticOptions: FSharpDiagnosticOptions, diagnosticsLogger: DiagnosticsLogger) =
(scopedPragmas, langVersion: LanguageVersion, diagnosticOptions: FSharpDiagnosticOptions, diagnosticsLogger: DiagnosticsLogger) =
inherit DiagnosticsLogger("DiagnosticsLoggerFilteringByScopedPragmas")

let scopedNowarnNotSupported =
not (langVersion.SupportsFeature(LanguageFeature.ScopedNowarn))

let mutable realErrorPresent = false

override _.DiagnosticSink(diagnostic: PhasedDiagnostic, severity) =
Expand All @@ -2256,7 +2260,7 @@ type DiagnosticsLoggerFilteringByScopedPragmas
scopedPragmas
|> List.exists (fun (ScopedPragma.WarningOff(pragmaRange, pragmaWarningNum)) ->
warningNum = pragmaWarningNum
&& (not checkFile || m.FileIndex = pragmaRange.FileIndex)
&& (scopedNowarnNotSupported || m.FileIndex = pragmaRange.FileIndex)
&& m.StartLine > pragmaRange.StartLine
&& m.EndLine <= pragmaRange.EndLine)
|> not
Expand All @@ -2274,5 +2278,5 @@ type DiagnosticsLoggerFilteringByScopedPragmas

override _.CheckForRealErrorsIgnoringWarnings = realErrorPresent

let GetDiagnosticsLoggerFilteringByScopedPragmas (checkFile, scopedPragmas, diagnosticOptions, diagnosticsLogger) =
DiagnosticsLoggerFilteringByScopedPragmas(checkFile, scopedPragmas, diagnosticOptions, diagnosticsLogger) :> DiagnosticsLogger
let GetDiagnosticsLoggerFilteringByScopedPragmas (scopedPragmas, langVersion, diagnosticOptions, diagnosticsLogger) =
DiagnosticsLoggerFilteringByScopedPragmas(scopedPragmas, langVersion, diagnosticOptions, diagnosticsLogger) :> DiagnosticsLogger
3 changes: 2 additions & 1 deletion src/Compiler/Driver/CompilerDiagnostics.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -84,8 +85,8 @@ type PhasedDiagnostic with

/// Get a diagnostics logger that filters the reporting of warnings based on scoped pragma information
val GetDiagnosticsLoggerFilteringByScopedPragmas:
checkFile: bool *
scopedPragmas: ScopedPragma list *
langVersion: LanguageVersion *
diagnosticOptions: FSharpDiagnosticOptions *
diagnosticsLogger: DiagnosticsLogger ->
DiagnosticsLogger
Expand Down
19 changes: 4 additions & 15 deletions src/Compiler/Driver/ParseAndCheckInputs.fs
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,6 @@ let GetScopedPragmas (langVersion: LanguageVersion) endPos hashDirectives =
|> List.groupBy (fun (ParsedHashDirective(range = range)) -> range.FileIndex)
|> List.collect (snd >> getPragmas)

let private lastEndPos ranges =
let comparePos (r1: range) (r2: range) =
if posEq r1.End r2.End then
r1.End.Column.CompareTo r2
else
r1.End.Line.CompareTo r2.End.Line

ranges |> List.sortWith comparePos |> List.last |> _.End

let private collectCodeComments (lexbuf: UnicodeLexing.Lexbuf) (tripleSlashComments: range list) =
[
yield! LexbufCommentStore.GetComments(lexbuf)
Expand Down Expand Up @@ -338,8 +329,7 @@ let PostParseModuleImpls

(impls |> List.collect getImplHashDirectives) @ toplevelHashDirectives

let endPos =
impls |> List.map (fun (SynModuleOrNamespace(range = m)) -> m) |> lastEndPos
let endPos = mkPos 1000000 1 //TODO: lexbuf, unfortunately, doesn't have the correct one

hashDirectives |> GetScopedPragmas lexbuf.LanguageVersion endPos

Expand Down Expand Up @@ -405,8 +395,7 @@ let PostParseModuleSpecs

(specs |> List.collect getModuleSigHashDirectives) @ toplevelHashDirectives

let endPos =
specs |> List.map (fun (SynModuleOrNamespaceSig(range = m)) -> m) |> lastEndPos
let endPos = mkPos 1000000 1 //TODO: lexbuf, unfortunately, doesn't have the correct one

hashDirectives |> GetScopedPragmas lexbuf.LanguageVersion endPos

Expand Down Expand Up @@ -583,7 +572,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(scopedPragmas, lexbuf.LanguageVersion, diagnosticOptions, diagnosticsLogger)

delayLogger.CommitDelayedDiagnostics filteringDiagnosticsLogger

Expand Down Expand Up @@ -1501,7 +1490,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(input.ScopedPragmas, tcConfig.langVersion, tcConfig.diagnosticsOptions, oldLogger)

/// Typecheck a single file (or interactive entry into F# Interactive)
let CheckOneInputEntry (ctok, checkForErrors, tcConfig: TcConfig, tcImports, tcGlobals, prefixPathOpt) tcState input =
Expand Down
9 changes: 3 additions & 6 deletions src/Compiler/Driver/fsc.fs
Original file line number Diff line number Diff line change
Expand Up @@ -739,13 +739,10 @@ let main2
let oldLogger = diagnosticsLogger

let diagnosticsLogger =
let scopedPragmas =
[
for CheckedImplFile(pragmas = pragmas) in typedImplFiles do
yield! pragmas
]
let pragmas =
typedImplFiles |> List.collect (fun (CheckedImplFile(pragmas = p)) -> p)

GetDiagnosticsLoggerFilteringByScopedPragmas(true, scopedPragmas, tcConfig.diagnosticsOptions, oldLogger)
GetDiagnosticsLoggerFilteringByScopedPragmas(pragmas, tcGlobals.langVersion, tcConfig.diagnosticsOptions, oldLogger)

SetThreadDiagnosticsLoggerNoUnwind diagnosticsLogger

Expand Down
3 changes: 2 additions & 1 deletion src/Compiler/FSComp.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1778,5 +1778,6 @@ featureEmptyBodiedComputationExpressions,"Support for computation expressions wi
3870,parsExpectingUnionCaseField,"Expecting union case field"
featureAllowAccessModifiersToAutoPropertiesGettersAndSetters,"Allow access modifiers to auto properties getters and setters"
3871,tcAccessModifiersNotAllowedInSRTPConstraint,"Access modifiers cannot be applied to an SRTP constraint."
featureScopedNowarn,"#nowarn can be scoped to parts of a source file by a corresponding #warnon."
3872,emptyWarnonDirective,"#warnon without warning numbers is ignored."
3873,nonMatchingWarnonDirective,"Warning %d is ignored because there is no previous #nowarn directive for this warning."
3873,nonMatchingWarnonDirective,"#warnon %d is ignored because there is no previous #nowarn directive for this warning number."
3 changes: 3 additions & 0 deletions src/Compiler/Facilities/LanguageFeatures.fs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ type LanguageFeature =
| LowerSimpleMappingsInComprehensionsToFastLoops
| ParsedHashDirectiveArgumentNonQuotes
| EmptyBodiedComputationExpressions
| ScopedNowarn

/// LanguageVersion management
type LanguageVersion(versionText) =
Expand Down Expand Up @@ -217,6 +218,7 @@ type LanguageVersion(versionText) =
LanguageFeature.EnforceAttributeTargets, previewVersion // not enabled because: https://github.com/dotnet/fsharp/issues/17514
LanguageFeature.FromEndSlicing, previewVersion // Unfinished features --- needs work
LanguageFeature.AllowAccessModifiersToAutoPropertiesGettersAndSetters, previewVersion
LanguageFeature.ScopedNowarn, previewVersion
]

static let defaultLanguageVersion = LanguageVersion("default")
Expand Down Expand Up @@ -372,6 +374,7 @@ type LanguageVersion(versionText) =
FSComp.SR.featureLowerSimpleMappingsInComprehensionsToFastLoops ()
| LanguageFeature.ParsedHashDirectiveArgumentNonQuotes -> FSComp.SR.featureParsedHashDirectiveArgumentNonString ()
| LanguageFeature.EmptyBodiedComputationExpressions -> FSComp.SR.featureEmptyBodiedComputationExpressions ()
| LanguageFeature.ScopedNowarn -> FSComp.SR.featureScopedNowarn ()

/// Get a version string associated with the given feature.
static member GetFeatureVersionString feature =
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/Facilities/LanguageFeatures.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ type LanguageFeature =
| LowerSimpleMappingsInComprehensionsToFastLoops
| ParsedHashDirectiveArgumentNonQuotes
| EmptyBodiedComputationExpressions
| ScopedNowarn

/// LanguageVersion management
type LanguageVersion =
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Service/IncrementalBuild.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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(input.ScopedPragmas, tcConfig.langVersion, tcConfig.diagnosticsOptions, capturingDiagnosticsLogger)
use _ = new CompilationGlobalsScope(diagnosticsLogger, BuildPhase.TypeCheck)

beforeFileChecked.Trigger fileName
Expand Down
7 changes: 6 additions & 1 deletion src/Compiler/Service/TransparentCompiler.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1299,7 +1299,12 @@ type internal TransparentCompiler
let diagnosticsLogger = errHandler.DiagnosticsLogger

let diagnosticsLogger =
GetDiagnosticsLoggerFilteringByScopedPragmas(false, input.ScopedPragmas, tcConfig.diagnosticsOptions, diagnosticsLogger)
GetDiagnosticsLoggerFilteringByScopedPragmas(
input.ScopedPragmas,
tcConfig.langVersion,
tcConfig.diagnosticsOptions,
diagnosticsLogger
)

use _ = new CompilationGlobalsScope(diagnosticsLogger, BuildPhase.TypeCheck)

Expand Down
1 change: 1 addition & 0 deletions src/Compiler/pars.fsy
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
1 change: 1 addition & 0 deletions src/Compiler/pppars.fsy
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
57 changes: 56 additions & 1 deletion tests/FSharp.Compiler.ComponentTests/CompilerDirectives/Line.fs
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,59 @@ printfn ""
| ParsedInput.SigFile _ -> failwith "unexpected: sig file"
if exprRange <> expectedRange then
failwith $"case{case}: expected: {expectedRange}, found {exprRange}"


let private warning20Text = "The result of this expression has type 'string' and is implicitly ignored. Consider using 'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'."
let private warning25Text = "Incomplete pattern matches on this expression. For example, the value 'Some (_)' may indicate a case not covered by the pattern(s)."

let private sourceForInteractionWithNowarn = """
namespace X
#line 100 "xyz.fs"
#nowarn "25"
module A =
match None with None -> ()
namespace Y
#line 8 "lineNoWarn.fs"
#nowarn "20"
module B =
""
match None with None -> ()
"""

[<InlineData("8.0")>]
[<InlineData("preview")>]
[<Theory>]
let ``interaction with nowarn`` languageVersion =
FSharp sourceForInteractionWithNowarn
|> withLangVersion languageVersion
|> withFileName "lineNowarn.fs"
|> compile
|> withDiagnostics [
Warning 25, Line 11, Col 11, Line 11, Col 15, warning25Text
]

let private sourceForInteractionWithNowarn2 = """
namespace X
#line 1 "xyz.fs"
#nowarn "25"
module A =
match None with None -> ()
namespace Y
#line 8 "lineNoWarn.fs"
#nowarn "20"
module B =
""
match None with None -> ()
"""

[<InlineData("8.0")>]
[<InlineData("preview")>]
[<Theory>]
let ``interaction with nowarn (2)`` languageVersion =
FSharp sourceForInteractionWithNowarn2
|> withLangVersion languageVersion
|> withFileName "lineNowarn.fs"
|> compile
|> withDiagnostics [
if languageVersion = "preview" then
Warning 25, Line 11, Col 11, Line 11, Col 15, warning25Text
]
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,6 @@ match None with None -> ()
|> compile
|> withDiagnostics [
Warning 3873, Line 5, Col 1, Line 5, Col 13,
"Warning 26 is ignored because there is no previous #nowarn directive for this warning."
"#warnon 26 is ignored because there is no previous #nowarn directive for this warning number."
]

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ ImplFile
(ParsedImplFileInput
("/root/ParsedHashDirective/TripleQuoteStringAsParsedHashDirectiveArgument.fs",
false, QualifiedNameOfFile TripleQuoteStringAsParsedHashDirectiveArgument,
[WarningOff ((2,0--2,16), 40)], [],
[WarningOff ((2,0--1000000,1), 40)], [],
[SynModuleOrNamespace
([TripleQuoteStringAsParsedHashDirectiveArgument], false, AnonModule,
[HashDirective
Expand Down

0 comments on commit 976c159

Please sign in to comment.