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

[SR-10131] AST Dump - float literals should include their sign #52533

Closed
swift-ci opened this issue Mar 19, 2019 · 7 comments
Closed

[SR-10131] AST Dump - float literals should include their sign #52533

swift-ci opened this issue Mar 19, 2019 · 7 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself good first issue Good for newcomers

Comments

@swift-ci
Copy link
Collaborator

Previous ID SR-10131
Radar None
Original Reporter vinivendra (JIRA User)
Type Bug
Status Resolved
Resolution Done
Environment

Using Swift 5, toolchain 4220190203a. Also tested with the most recent Swift 5 toolchain (5020190310a, from March 10th 2019).

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, StarterBug
Assignee vinivendra (JIRA)
Priority Medium

md5: 45b85c9388061a6e207ae00b2a568249

Issue Description:

In AST dumps, negative integer literals include a "negative" flag, but float literals don't dump any info on whether or not they are negative.

For instance, dumping a file with these contents:

-1
-1.0

yields an AST dump like this:

(source_file "<<testFilePath>>"
  (top_level_code_decl range=[<<testFilePath>>:1:1 - line:1:2]
    (brace_stmt range=[<<testFilePath>>:1:1 - line:1:2]
      (call_expr implicit type='Int' location=<<testFilePath>>:1:1 range=[<<testFilePath>>:1:1 - line:1:2] nothrow arg_labels=_builtinIntegerLiteral:
        (constructor_ref_call_expr implicit type='(IntLiteral) -> Int' location=<<testFilePath>>:1:1 range=[<<testFilePath>>:1:1 - line:1:1] nothrow
          (declref_expr implicit type='(Int.Type) -> (IntLiteral) -> Int' location=<<testFilePath>>:1:1 range=[<<testFilePath>>:1:1 - line:1:1] decl=Swift.(file).Int.init(_builtinIntegerLiteral:) function_ref=single)
          (type_expr implicit type='Int.Type' location=<<testFilePath>>:1:1 range=[<<testFilePath>>:1:1 - line:1:1] typerepr='Int'))
        (tuple_expr implicit type='(_builtinIntegerLiteral: Builtin.IntLiteral)' location=<<testFilePath>>:1:1 range=[<<testFilePath>>:1:1 - line:1:2] names=_builtinIntegerLiteral
          (integer_literal_expr type='Builtin.IntLiteral' location=<<testFilePath>>:1:1 range=[<<testFilePath>>:1:1 - line:1:2] negative value=1)))))
  (top_level_code_decl range=[<<testFilePath>>:2:1 - line:2:2]
    (brace_stmt range=[<<testFilePath>>:2:1 - line:2:2]
      (call_expr implicit type='Double' location=<<testFilePath>>:2:1 range=[<<testFilePath>>:2:1 - line:2:2] nothrow arg_labels=_builtinFloatLiteral:
        (constructor_ref_call_expr implicit type='(FPIEEE80) -> Double' location=<<testFilePath>>:2:1 range=[<<testFilePath>>:2:1 - line:2:1] nothrow
          (declref_expr implicit type='(Double.Type) -> (FPIEEE80) -> Double' location=<<testFilePath>>:2:1 range=[<<testFilePath>>:2:1 - line:2:1] decl=Swift.(file).Double extension.init(_builtinFloatLiteral:) function_ref=single)
          (type_expr implicit type='Double.Type' location=<<testFilePath>>:2:1 range=[<<testFilePath>>:2:1 - line:2:1] typerepr='Double'))
        (tuple_expr implicit type='(_builtinFloatLiteral: FPIEEE80)' location=<<testFilePath>>:2:1 range=[<<testFilePath>>:2:1 - line:2:2] names=_builtinFloatLiteral
          (float_literal_expr type='FPIEEE80' location=<<testFilePath>>:2:1 range=[<<testFilePath>>:2:1 - line:2:2] value=1.0))))))

in which the integer literal ends with negative value=1) but the float literal just ends with value=1.0).

@belkadan
Copy link
Contributor

The place to change this is visitFloatLiteralExpr in ASTDumper.cpp; you can copy what visitIntegerLiteralExpr is doing right above it.

(Good catch, Vinicius!)

@swift-ci
Copy link
Collaborator Author

Comment by Vinicius Vendramini (JIRA)

Thanks 🙂

I'd love to get this fixed asap. Do you think it could go into Swift 5 or 5.1?

@belkadan
Copy link
Contributor

It seems small enough that it'd be safe to cherry-pick for 5.1. 5 is pretty locked down at this point, though.

@swift-ci
Copy link
Collaborator Author

Comment by Vinicius Vendramini (JIRA)

Great! Should I make the change on master or on a specific branch?

@belkadan
Copy link
Contributor

Everything goes to master first, then you can cherry-pick per the 5.1 release process.

@swift-ci
Copy link
Collaborator Author

Comment by Vinicius Vendramini (JIRA)

Ok. I'm still running the tests, but it seemed pretty straightforward. I opened a pull request (#23484 if you want to take a look.

@swift-ci
Copy link
Collaborator Author

Comment by Vinicius Vendramini (JIRA)

Great, it's done! I tried to create a cherry-pick for 5.1 like you mentioned, it's here. Not sure if it's right though, I've never done this before.

Thanks for the help. I'm gonna close this bug report.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants