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

Access modifier is lost in extern declaration #1213

Closed
auduchinok opened this issue Nov 3, 2020 · 4 comments
Closed

Access modifier is lost in extern declaration #1213

auduchinok opened this issue Nov 3, 2020 · 4 comments

Comments

@auduchinok
Copy link
Contributor

Issue created from fantomas-online

Code

[<DllImport("kernel32.dll")>]
extern UIntPtr private GetProcessHeap()

Result

[<DllImport("kernel32.dll")>]
extern UIntPtr GetProcessHeap()

The code is taken from Lib.fs.

@nojaf
Copy link
Contributor

nojaf commented Nov 6, 2020

The AST I get for this is:

ParsedInput.ImplFile
  (ParsedImplFileInput
     ("tmp.fsx", true, QualifiedNameOfFile Tmp$fsx, [], [],
      [SynModuleOrNamespace
         ([Tmp], false, AnonModule,
          [SynModuleDecl.Let
             (false,
              [Binding
                 (None, NormalBinding, false, false,
                  [{ Attributes =
                                 [{ TypeName =
                                              LongIdentWithDots
                                                ([DllImport], [])
                                    ArgExpr =
                                             SynExpr.Paren
                                               (SynExpr.Const
                                                  (SynConst.String
                                                     ("kernel32.dll",
                                                      tmp.fsx (1,12--1,26) IsSynthetic=false),
                                                   tmp.fsx (1,12--1,26) IsSynthetic=false),
                                                tmp.fsx (1,11--1,12) IsSynthetic=false,
                                                Some
                                                  tmp.fsx (1,26--1,27) IsSynthetic=false,
                                                tmp.fsx (1,11--1,27) IsSynthetic=false)
                                    Target = None
                                    AppliesToGetterAndSetter = false
                                    Range =
                                           tmp.fsx (1,2--1,11) IsSynthetic=false }]
                     Range = tmp.fsx (1,0--1,29) IsSynthetic=false }],
                  PreXmlDoc ((2,6), FSharp.Compiler.XmlDoc+XmlDocCollector),
                  SynValData
                    (None, SynValInfo ([[]], SynArgInfo ([], false, None)), None),
                  SynPat.LongIdent
                    (LongIdentWithDots ([GetProcessHeap], []), None,
                     Some (SynValTyparDecls ([], false, [])),
                     Pats
                       [SynPat.Tuple
                          (false, [], tmp.fsx (2,37--2,38) IsSynthetic=false)],
                     None, tmp.fsx (2,15--2,22) IsSynthetic=false),
                  Some
                    (SynBindingReturnInfo
                       (SynType.App
                          (SynType.LongIdent (LongIdentWithDots ([UIntPtr], [])),
                           None, [], [], None, false,
                           tmp.fsx (2,7--2,14) IsSynthetic=false),
                        tmp.fsx (2,7--2,14) IsSynthetic=false, [])),
                  SynExpr.Typed
                    (SynExpr.App
                       (NonAtomic, false, SynExpr.Ident failwith,
                        SynExpr.Const
                          (SynConst.String
                             ("extern was not given a DllImport attribute",
                              tmp.fsx (2,38--2,39) IsSynthetic=false),
                           tmp.fsx (2,38--2,39) IsSynthetic=false),
                        tmp.fsx (2,0--2,39) IsSynthetic=false),
                     SynType.App
                       (SynType.LongIdent (LongIdentWithDots ([UIntPtr], [])),
                        None, [], [], None, false,
                        tmp.fsx (2,7--2,14) IsSynthetic=false),
                     tmp.fsx (2,0--2,39) IsSynthetic=false),
                  tmp.fsx (2,0--2,39) IsSynthetic=false,
                  NoDebugPointAtInvisibleBinding)],
              tmp.fsx (2,0--2,39) IsSynthetic=false)], PreXmlDocEmpty, [], None,
          tmp.fsx (1,0--2,39) IsSynthetic=false)], (true, true)))

The access modifier should be the first item of SynBinding but it is not present.

Binding
                 (None, NormalBinding, false, false,

Am I missing something here or is this a bug in the compiler?

@auduchinok
Copy link
Contributor Author

auduchinok commented Nov 6, 2020

Yes, it seems the access modifier is always ignored here: https://github.com/dotnet/fsharp/blob/05b0569a7d6acefd21ad31bce49f780d961db651/src/fsharp/pars.fsy#L2634

@dawedawe
Copy link
Member

@auduchinok @nojaf I think, this can be closed. Current fantomas/fsharp keeps the access modifier.

@nojaf
Copy link
Contributor

nojaf commented Feb 13, 2022

I tend to close issues that we can no longer reproduce with the latest version by adding a PR with a regression test.
Just so we ensure it stays fixed.

nojaf pushed a commit that referenced this issue Feb 13, 2022
…aration (#2090)

* Add regression test for #1213, Access modifier is lost in extern declaration

* Explain what's needed to close issues, that aren't reproducable anymore.

* Add regression test for #1213, Access modifier is lost in extern declaration

* Move test to FunctionDefinitionTests.fs

* Add regression test for #1213, Access modifier is lost in extern declaration

* Move test to FunctionDefinitionTests.fs

* Add regression test for #1213, Access modifier is lost in extern declaration

* Remove regression test for #1213 from SignatureTests.fs
@nojaf nojaf closed this as completed Feb 13, 2022
jindraivanek pushed a commit to jindraivanek/fantomas that referenced this issue Mar 30, 2022
…xtern declaration (fsprojects#2090)

* Add regression test for fsprojects#1213, Access modifier is lost in extern declaration

* Explain what's needed to close issues, that aren't reproducable anymore.

* Add regression test for fsprojects#1213, Access modifier is lost in extern declaration

* Move test to FunctionDefinitionTests.fs

* Add regression test for fsprojects#1213, Access modifier is lost in extern declaration

* Move test to FunctionDefinitionTests.fs

* Add regression test for fsprojects#1213, Access modifier is lost in extern declaration

* Remove regression test for fsprojects#1213 from SignatureTests.fs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants