-
Notifications
You must be signed in to change notification settings - Fork 831
Include attributes in ranges of signature nodes. #11503
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
Include attributes in ranges of signature nodes. #11503
Conversation
cartermp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
@nojaf some conflicts. |
…dModule and SynValSpfn.
519e853 to
251e2b7
Compare
|
Resolved! |
|
This PR breaks ranges for type signatures that don't have attributes. For the following signature the range now starts at type R =
{ F: int }I'm working on a fix. |
|
Would that not be consist with the range you'd get when it is inside an implementation file? |
|
So you're suggesting that there should be this difference? type R1 = // type is not included
{ F: int }
[<A>]
type R = // type is included
{ F: int }
It looks very wrong to me.
IIRC implementation files should already contain them. If not, we should fix it. |
|
I'd be in favor of having them included, just wanted to point out that in FCS 39, the type R = { Foo: int }ImplFile
(ParsedImplFileInput
("tmp.fsx", true, QualifiedNameOfFile Tmp$fsx, [], [],
[SynModuleOrNamespace
([Tmp], false, AnonModule,
[Types
([TypeDefn
(ComponentInfo
([], [], [], [R],
PreXmlDoc ((1,6), FSharp.Compiler.XmlDoc+XmlDocCollector),
false, None, tmp.fsx (1,5--1,6) IsSynthetic=false),
Simple
(Record
(None,
[Field
([], false, Some Foo,
LongIdent (LongIdentWithDots ([int], [])), false,
PreXmlDoc ((1,14), FSharp.Compiler.XmlDoc+XmlDocCollector),
None, tmp.fsx (1,11--1,19) IsSynthetic=false)],
tmp.fsx (1,9--1,21) IsSynthetic=false),
tmp.fsx (1,9--1,21) IsSynthetic=false), [],
tmp.fsx (1,5--1,21) IsSynthetic=false)],
tmp.fsx (1,0--1,21) IsSynthetic=false)], PreXmlDocEmpty, [], None,
tmp.fsx (1,0--1,21) IsSynthetic=false)], (true, true))) |
|
Ah, I see where I wasn't clear. We use |
|
Ah ok, now I understand what regressed. Thanks for explaining. |
Fixes #11487.