Generic Attributes

[mflibby]

Description

Resolves FSLang 965

RFC FS-1143

Intention:

The intent of this PR is to implement Generic Attribute support, e.g.:

type ARecordAttribute<^T>()=
    inherit Attribute()

type AFieldAttribute<^T>()=
    inherit Attribute()

[<ARecord<int>>]
type SomeRecord =
    {
        [<AField<string>>]
        someField : string
    }

Checklist

• [x] Test cases added
• [ ] Performance benchmarks added in case of performance changes
• [x] Release notes entry updated:

[github-actions[bot]]

:heavy_exclamation_mark: Release notes required

---

:white_check_mark: Found changes and release notes in following paths:

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/8.0.400.md
LanguageFeatures.fsi	docs/release-notes/.Language/preview.md

[mflibby]

Adding of test cases, and fullfilment of minutia (documentation, various "feature" tags, general cleanup, etc) still WIP.

Currently, [<SomeAttribute\<int>()>] compiles down to the expected IL. I plan to explore what will be required for inferencing before marking as ready.

Drafting now while I complete the minor details in case anyone wants to provide feedback in the meantime.

[mflibby]

@dotnet-policy-service agree company="Hillcrest Research and Development LLC"

[abelbraaksma]

Hi @mflibby, this looks very interesting! In your original description, can you link to the reported issue that this is fixing? And if it is a language change (i.e., a new feature that is being added), can you link to the RFC (in https://github.com/fsharp/fslang-design) and the language proposal or discussion chat (https://github.com/fsharp/fslang-suggestions)?

If none exist, can you update the OP with a summary of what this change is doing and why?

[mflibby]

@abelbraaksma I've updated the description with the desired information. Please let me know if this is satisfactory, or if more information will be helpful :)

[mflibby]

I'm not sure why, but commit 64607ef passed all checks, yet after a few cleanup/a minor change/merges to other changes on main, a large number of the checks have been failing (the only substantive change I made since the passing commit was a minor change to the LexFilter that allowed the type arguments immediately adjacent to the close of an attribute list to be properly parsed).

I'm wondering why I am all of a sudden getting a bunch of seemingly unrelated tests failing? Additionally, windows defender seems to be flagging a bunch of tests, and occasionally quarantining innocuous tests like "Conformance/../E_Literals02.fs".

The investigation continues.

[abelbraaksma]

I've updated the description with the desired information. Please let me know if this is satisfactory, or if more information will be helpful :)

Hey @mflibby, excellent, thanks, now we know where it's coming from and that it was approved 👍. We still need an RFC, though. I may be able to help with that. Language changes require an RFC for documentation and consensus. They capture the what, how and scope of any new feature. The go here: https://github.com/fsharp/fslang-design. It is also used to set up a discussion thread to deal with any unresolved questions.

I can help with the process.

[mflibby]

@abelbraaksma Oh well the order in which I've done things certainly should make drawing up the RFC easier, I will get on that ASAP - thank you for the heads up!

[vzarytovskii]

The problem is we try to maintain the metadata compatibility with the older compilers, and currently there's no preferred way to add new data to it without breaking existing compilers.

@vzarytovskii Do you know if there been any changes in this direction?

Rule of thumb is usually:

1. Don't break it if possible ^(duh). Problem with this is that we don't really have a mechanism of merging pickled data with IL metadata (in this case - merging FSharpAttribute with ILAttribute) as well as not all the information we would like to have, is there (like aliases, or type measures for example), which, as a result will get lost cross-assembly.
2. If breaking change is necessary - make it explicit - i.e. only pickle new data when library author explicitly opted in to use that feature, and put the feature under language flag and don't use it in FSharp.Core (for example - let bindings in types, or, in this case, generic attributes).

We have one more case - when we introduced another resource for nullability data, it has multiple problems unfortunately:

1. We don't have standard mechanisms for managing those, code for handling every single one has to be literally copy-pasted, which kinda makes it prone to errors.
2. It turns out to be suboptimal - a lot of additional excessive data - a bunch of new tables, instead of just new bit/type somewhere.
3. It needs to be taken care of in the trimming config files every time it's introduced.

We probably want to think of such mechanism in future, but it will require a very thorough design and will be a huge chunk of work.

I would assume that the purpose of the pickled resources is for the sake of performance improvements (correct me if I'm wrong)i n which case I would think that there are some kind of benchmarks concerning the gain of using pickling as opposed to loading the same resources from the IL?

Not necessary, main purpose is to have/store additional metadata, which we want to use cross-assembly (mostly). I don't really think it will affect perf significatly.

Assuming that the cost in this case wouldn't be great, and that it is feasible, loading the info from the IL and not breaking backward compatibility sounds like the better option.

Yeah, but please, refer to part of this comment below - not all the info we would like to have is in the IL metadata.

TL;DR - I would personally go with second point in the beginning of my comment if possible. We have done it before. We will likely do it in future. And at the moment we, unfortunately, don't have a better mechanism for it.

cc @T-Gro @dsyme