Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FSharpMemberOrFunctionOrValue doesn't include attributes for params in CurriedParameterGroups #13786

Open
alfonsogarciacaro opened this issue Aug 26, 2022 · 0 comments · May be fixed by #13800
Labels
Area-FCS Feature Improvement Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Milestone

Comments

@alfonsogarciacaro
Copy link
Contributor

This seems to be a known issue because there are several comments in FSharpMemberOrFunctionOrValue like this one:

| M m | C m ->
[ for argTys in m.GetParamDatas(cenv.amap, range0, m.FormalMethodInst) do
yield
[ for ParamData(isParamArrayArg, isInArg, isOutArg, optArgInfo, _callerInfo, nmOpt, _reflArgInfo, pty) in argTys do
// INCOMPLETENESS: Attribs is empty here, so we can't look at attributes for
// either .NET or F# parameters
let argInfo: ArgReprInfo = { Name=nmOpt; Attribs= [] }
let m =
match nmOpt with
| Some v -> v.idRange
| None ->
defaultArg x.DeclarationLocationOpt range0
yield FSharpParameter(cenv, pty, argInfo, None, m, isParamArrayArg, isInArg, isOutArg, optArgInfo.IsOptional, false) ]
|> makeReadOnlyCollection ]
|> makeReadOnlyCollection

Not sure if this something that is not possible/easy to implement or it's been a TODO that has remained in the code base. Interestingly when members are internally represented as function values (.Data = V ...) the attributes for params can be accessed:

| V v ->
match v.ValReprInfo with
| None ->
let _, tau = v.GeneralizedType
if isFunTy cenv.g tau then
let argTysl, _typ = stripFunTy cenv.g tau
[ for ty in argTysl do
let allArguments =
if isRefTupleTy cenv.g ty
then tryDestRefTupleTy cenv.g ty
else [ty]
let m = defaultArg x.DeclarationLocationOpt range0
yield
allArguments
|> List.map (fun arg -> FSharpParameter(cenv, arg, ValReprInfo.unnamedTopArg1, m))
|> makeReadOnlyCollection ]
|> makeReadOnlyCollection

This seems to be the reason this issue hasn't surfaced in Fable until now. In Fable we sometimes use param attributes to give info about how a parameter should be compiled in the call-site. Until Fable 3 this information was directly taken from the member reference in FSharpExprPatterns.Call. It seems in this case the member is always represented as function value and the param attributes are kept. However, in Fable 4 we are using a different model where entity and member info are kept separated from the AST to make serialization easier. Because of this, the parameter info is accessed now through FSharpEntity.MembersFunctionsAndValues and apparently this causes members to be represented as methods (.Data = M ...) and the param attributes to be lost when calling CurriedParameterGroups.

@dsyme Would it be possible to fix this in FCS or is it a limitation we need to overcome somehow?

ncave pushed a commit to ncave/fsharp that referenced this issue Aug 29, 2022
Temporary fix, remove when upstream dotnet#13786 is fixed.
@vzarytovskii vzarytovskii added this to the Backlog milestone Sep 9, 2022
@vzarytovskii vzarytovskii added the Area-Compiler-Syntax lexfilter, indentation and parsing label Sep 9, 2022
@dsyme dsyme added Area-FCS and removed Area-Compiler-Syntax lexfilter, indentation and parsing labels Sep 23, 2022
ncave pushed a commit to ncave/fsharp that referenced this issue Sep 25, 2022
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Sep 26, 2022
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Sep 26, 2022
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Nov 14, 2022
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Nov 14, 2022
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Nov 30, 2022
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Nov 30, 2022
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Jan 28, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave added a commit to ncave/fsharp that referenced this issue Jan 31, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Feb 24, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave added a commit to ncave/fsharp that referenced this issue Feb 24, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave added a commit to ncave/fsharp that referenced this issue Feb 24, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Apr 15, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave added a commit to ncave/fsharp that referenced this issue Apr 15, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Oct 15, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave added a commit to ncave/fsharp that referenced this issue Oct 17, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave added a commit to ncave/fsharp that referenced this issue Oct 19, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Oct 19, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave added a commit to ncave/fsharp that referenced this issue Oct 19, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave added a commit to ncave/fsharp that referenced this issue Oct 19, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave added a commit to ncave/fsharp that referenced this issue Oct 23, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Oct 23, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave pushed a commit to ncave/fsharp that referenced this issue Dec 8, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
ncave added a commit to ncave/fsharp that referenced this issue Dec 8, 2023
Temporary fix, remove when upstream dotnet#13786 is fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-FCS Feature Improvement Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
Status: New
Development

Successfully merging a pull request may close this issue.

3 participants