Preview feature CSharpExtensionAttributeNotRequired causes existing compilations to fail

[dsyme]

Just to mention that /langversion:preview breaks the build as things currently standard, particularly the build of FSharp.Editor.Tests.dll.

The explanation is a bit convoluted but itt's because this feature:

    if g.langVersion.SupportsFeature(LanguageFeature.CSharpExtensionAttributeNotRequired) then

causes more metadata to be traversed when opening a namespace.  Whenever we traverse more metadata than before we can hit situations where we hit missing types that were are not in the set of referenced assemblies (so-called "incomplete reference sets"), which trigger errors which weren't present before. 

Testing for incomplete reference sets is annoyingly hard and we don't do it much.  However it happens in our build for FSharp.Editor.Tests.dll, which references FSharp.Editor correctly, which in turn contains a transitive reference to Microsoft.VisualStudio.ProjectSystem  via this code:

type internal SetGlobalPropertiesForSdkProjects [<ImportingConstructor>] (projectService: IProjectService) =
    inherit StaticGlobalPropertiesProviderBase(projectService.Services)

When compiling FSharp.Editor.Tests.dll we don't actually include the reference to Microsoft.VisualStudio.ProjectSystem  so the referenced assembly set is incomplete, and nothing in .NET build system completes the set for us. Note this means the metadata for StaticGlobalPropertiesProviderBase literally isn't available and any attempt to dereference that type will fail tycon.Deref and raise UnresolvedPathReferenceNoRange. Also, this reference is via the base type of the type - and a lot of operations traverse that!.  In short, if we go poking around the type hierarchy in new ways when compiling FSharp.Editor.Tests.dll then bang. The newly active code that does this is this call:

        let minfos =
            GetImmediateIntrinsicMethInfosOfType (None, AccessorDomain.AccessibleFromSomeFSharpCode) g amap m ty
            |> List.filter (IsMethInfoPlainCSharpStyleExtensionMember g m true)

via this callstack

Anyway, the short story is the when /langversion:preview is on we trigger this missing assembly exception in code that previously compiled, giving this error:

C:\GitHub\dsyme\fsharp\vsintegration\tests\FSharp.Editor.Tests\Helpers\RoslynHelpers.fs(15,1): error FS0074: The type referenced through 'Microsoft.VisualStudio.ProjectSystem.Build.StaticGlobalPropertiesProviderBase' is defined in an as
sembly that is not referenced. You must add a reference to assembly 'Microsoft.VisualStudio.ProjectSystem'. [C:\GitHub\dsyme\fsharp\vsintegration\tests\FSharp.Editor.Tests\FSharp.Editor.Tests.fsproj]
C:\GitHub\dsyme\fsharp\vsintegration\tests\FSharp.Editor.Tests\Helpers\RoslynHelpers.fs(180,31): error FS0039: The namespace or module 'SettingsStore' is not defined. [C:\GitHub\dsyme\fsharp\vsintegration\tests\FSharp.Editor.Tests\FShar
p.Editor.Tests.fsproj]
....

The best solution to this is probably to use protectAssemblyExploration like this:

    let minfos =
        protectAssemblyExploration [] (fun () ->
            GetImmediateIntrinsicMethInfosOfType (None, AccessorDomain.AccessibleFromSomeFSharpCode) g amap m ty
            |> List.filter (IsMethInfoPlainCSharpStyleExtensionMember g m true))

I'll add this to the nullness PR but

1. It might make sense to add it to main earlier.
2. Perhaps a CI test that the compiler bootstraps with "/langversion:preview" too?

[dsyme]

Note I'd also recommend double checking the performance implications of CSharpExtensionAttributeNotRequired - I haven't fully thought it through completely but my understanding is that it causes the methods of every type in every opened namespace to be "looked at" - this may be much more expensive than before.

[dsyme]

I've added a note about the perf implications of CSharpExtensionAttributeNotRequired and how to address it here: https://github.com/dotnet/fsharp/pull/13839/files#r1180881070