Init-only & required properties support #13490

vzarytovskii · 2022-07-11T17:27:16Z

RFC: published
Produce error-level diagnostic on setting init-only IL properties (courtesy of @smoothdeveloper - (experimental) forbid setter call on init properties #11564)
- Test that set_Prop is forbidden too.
- Test if init-properties are only allowed from the constructor, e.g. disallow the following:
  instance.MethodThatReturnsThis(initOnlyProp = 123)
  Which will mutate the instance
Support for CompilerFeatureRequiredAttribute (Roslyn codegen)
Enforcing of required properties initialization (via the properties initialization)

vzarytovskii · 2022-07-14T17:28:05Z

I will need to add one more language feature, and hide init-only checking under it, so F#6 continues to allow current behaviour.

src/Compiler/Checking/CheckExpressions.fs

src/Compiler/Checking/AttributeChecking.fs

vzarytovskii · 2022-07-15T17:08:34Z

This is ready for a first wave of reviews.
cc @dsyme

dsyme · 2022-07-16T16:04:21Z

System.Exception : Unable to emit C# compilation.
seq
[(26,48): error CS0518: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported;
(15,43): error CS0518: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported]

vzarytovskii · 2022-07-16T16:14:14Z

System.Exception : Unable to emit C# compilation.
seq
[(26,48): error CS0518: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported;
(15,43): error CS0518: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported]

That is okay, C# compiler cannot find the attribute in the net472 runtime, it's known, i will fix it.

Quote from Jared:

The IsExternalInit type is only included in the net5.0 (and future) target frameworks. When compiling against older target frameworks you will need to manually define this type.

src/Compiler/FSComp.txt

src/Compiler/Checking/CheckExpressions.fs

src/Compiler/TypedTree/TcGlobals.fs

src/Compiler/Checking/AttributeChecking.fs

src/Compiler/Checking/CheckExpressions.fs

src/Compiler/Checking/infos.fs

vzarytovskii · 2022-07-22T17:09:05Z

@dsyme Addressed all comments except 1 (not entirely sure whether we should warn user when they place RequiredMemberAttribute or not).

vzarytovskii · 2022-07-25T15:14:04Z

We may want to retarget this feature to 17.4, and update sdk to .NET7-preview, since it will be much easier to test runtime features without the need of adding attributes ourselves.

vzarytovskii · 2022-07-25T16:25:26Z

Added attributes to unsupported ones, compiler will produce a warning for them if used in F# code.

KevinRansom

Nice

0101 · 2022-07-28T09:19:06Z

src/Compiler/Checking/infos.fs

@@ -1562,6 +1579,16 @@ type ILPropInfo =
    /// Indicates if the IL property has a 'set' method
    member x.HasSetter = Option.isSome x.RawMetadata.SetMethod

+    /// Indidcates whether IL property has an init-only setter (i.e. has the `System.Runtime.CompilerServices.IsExternalInit` modifer)
+    member x.IsSetterInitOnly =
+        assert x.HasSetter


This probably shouldn't be here.

src/Compiler/Checking/infos.fs

tests/FSharp.Compiler.ComponentTests/Interop/RequiredAndInitOnlyProperties.fs

src/Compiler/Checking/CheckExpressions.fs

Co-authored-by: Petr Pokorny <petr@innit.cz>

src/Compiler/Checking/infos.fs

tests/FSharp.Compiler.ComponentTests/Interop/RequiredAndInitOnlyProperties.fs

src/Compiler/Checking/CheckExpressions.fs

Co-authored-by: Petr Pokorny <petr@innit.cz>

src/Compiler/AbstractIL/il.fs

src/Compiler/Checking/CheckExpressions.fs

dsyme

A couple of small changes needed

dsyme · 2022-07-28T18:12:01Z

@vzarytovskii A couple of small changes needed - Please feel free to clear my review after you've addressed them, no 2nd review needed

As agreed

WIP: Added init-only properties error + added some tests

9eedfb7

vzarytovskii self-assigned this Jul 11, 2022

vzarytovskii added the Area-Compiler-ImportAndInterop import of .NET DLLs and interop label Jul 11, 2022

vzarytovskii added this to the July-2022 milestone Jul 11, 2022

vzarytovskii linked an issue Jul 11, 2022 that may be closed by this pull request

Tracking Init properties and required members support #13451

Closed

vzarytovskii added 5 commits July 11, 2022 19:34

WIP: Separate tests

e2d9d68

Added check for explicit setters

2a05ed2

Fix tests

8a7d48c

Forbit calling setters on the methods

bdcab6b

wip: support required attributes

63fc2fb

Added required properties support + apply fantomas

b1ffb44

vzarytovskii commented Jul 15, 2022

View reviewed changes

src/Compiler/Checking/CheckExpressions.fs Outdated Show resolved Hide resolved

vzarytovskii commented Jul 15, 2022

View reviewed changes

src/Compiler/Checking/AttributeChecking.fs Outdated Show resolved Hide resolved

vzarytovskii changed the title ~~WIP Init-only & required properties support~~ Init-only & required properties support Jul 15, 2022

vzarytovskii requested review from dsyme and KevinRansom July 15, 2022 17:08

Merge branch 'main' into required-and-init

bb8b77a

vzarytovskii removed this from the July-2022 milestone Jul 15, 2022

0101 reviewed Jul 18, 2022

View reviewed changes

src/Compiler/FSComp.txt Outdated Show resolved Hide resolved

src/Compiler/Checking/CheckExpressions.fs Outdated Show resolved Hide resolved

src/Compiler/TypedTree/TcGlobals.fs Outdated Show resolved Hide resolved

dsyme requested changes Jul 18, 2022

View reviewed changes

vzarytovskii added 4 commits July 22, 2022 13:36

Merge remote-tracking branch 'upstream/main' into required-and-init

4c7029f

Moved compilerfeaturerequired check to a method

2681745

Updated

81da22c

fantomas

662186f

Internalize empty

005af55

vzarytovskii added 2 commits July 25, 2022 17:22

Add setter check + add more unsupported attributes

5784869

Merge remote-tracking branch 'upstream/main' into required-and-init

bf2ae5e

vzarytovskii requested review from dsyme and 0101 July 25, 2022 16:24

Fix prop pages

cc928ef

KevinRansom approved these changes Jul 27, 2022

View reviewed changes

0101 reviewed Jul 28, 2022

View reviewed changes

vzarytovskii and others added 3 commits July 28, 2022 13:08

Update src/Compiler/Checking/CheckExpressions.fs

0ef72fd

Co-authored-by: Petr Pokorny <petr@innit.cz>

Addressed PR comments + added additional tests

07762ca

Merge remote-tracking branch 'upstream/main' into required-and-init

4051bdc

vzarytovskii requested a review from 0101 July 28, 2022 12:43

0101 reviewed Jul 28, 2022

View reviewed changes

src/Compiler/Checking/infos.fs Outdated Show resolved Hide resolved

tests/FSharp.Compiler.ComponentTests/Interop/RequiredAndInitOnlyProperties.fs Show resolved Hide resolved

src/Compiler/Checking/CheckExpressions.fs Outdated Show resolved Hide resolved

0101 approved these changes Jul 28, 2022

View reviewed changes

vzarytovskii and others added 2 commits July 28, 2022 15:39

Update src/Compiler/Checking/infos.fs

3bc625b

Co-authored-by: Petr Pokorny <petr@innit.cz>

Moved condition outside of list expression

66407b2

dsyme reviewed Jul 28, 2022

View reviewed changes

src/Compiler/AbstractIL/il.fs Outdated Show resolved Hide resolved

dsyme reviewed Jul 28, 2022

View reviewed changes

src/Compiler/Checking/CheckExpressions.fs Outdated Show resolved Hide resolved

dsyme previously requested changes Jul 28, 2022

View reviewed changes

PR fixes

764c158

vzarytovskii closed this Jul 28, 2022

vzarytovskii reopened this Jul 28, 2022

vzarytovskii merged commit efdcde2 into dotnet:main Jul 29, 2022

vzarytovskii deleted the required-and-init branch July 29, 2022 09:48

This was referenced Sep 6, 2022

(experimental) forbid setter call on init properties #11564

Closed

Check expressions: safer init properties check #13892

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Init-only & required properties support #13490

Init-only & required properties support #13490

vzarytovskii commented Jul 11, 2022 •

edited by dsyme

Loading

vzarytovskii commented Jul 14, 2022

vzarytovskii commented Jul 15, 2022

dsyme commented Jul 16, 2022

vzarytovskii commented Jul 16, 2022 •

edited

Loading

vzarytovskii commented Jul 22, 2022 •

edited

Loading

vzarytovskii commented Jul 25, 2022

vzarytovskii commented Jul 25, 2022

KevinRansom left a comment

0101 Jul 28, 2022

dsyme left a comment

dsyme commented Jul 28, 2022 •

edited

Loading

Init-only & required properties support #13490

Init-only & required properties support #13490

Conversation

vzarytovskii commented Jul 11, 2022 • edited by dsyme Loading

vzarytovskii commented Jul 14, 2022

vzarytovskii commented Jul 15, 2022

dsyme commented Jul 16, 2022

vzarytovskii commented Jul 16, 2022 • edited Loading

vzarytovskii commented Jul 22, 2022 • edited Loading

vzarytovskii commented Jul 25, 2022

vzarytovskii commented Jul 25, 2022

KevinRansom left a comment

Choose a reason for hiding this comment

0101 Jul 28, 2022

Choose a reason for hiding this comment

dsyme left a comment

Choose a reason for hiding this comment

dsyme commented Jul 28, 2022 • edited Loading

vzarytovskii commented Jul 11, 2022 •

edited by dsyme

Loading

vzarytovskii commented Jul 16, 2022 •

edited

Loading

vzarytovskii commented Jul 22, 2022 •

edited

Loading

dsyme commented Jul 28, 2022 •

edited

Loading