Skip to content

Add integerValue to IntegerLiteralExprSyntax and floatingValue to FloatLiteralExprSyntax #2605

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Apr 29, 2024
Merged

Add integerValue to IntegerLiteralExprSyntax and floatingValue to FloatLiteralExprSyntax #2605

merged 4 commits into from
Apr 29, 2024

Conversation

kimdv
Copy link
Contributor

@kimdv kimdv commented Apr 17, 2024

Resolves #405

@kimdv kimdv self-assigned this Apr 17, 2024
@kimdv kimdv requested review from ahoppen and bnbarham as code owners April 17, 2024 18:54
@kimdv kimdv changed the title Add integerValue to IntegerLiteralExprSyntax Add integerValue to IntegerLiteralExprSyntax and floatingValue to FloatLiteralExprSyntax Apr 17, 2024
@kimdv kimdv force-pushed the kimdv/405-sr-13584-make-convenience-methods-returning-an-the-value-of-an-integer-literal-as-int-and-of-floating-literal-as-float-return-an-optional branch from 22f796b to 992c2ce Compare April 17, 2024 19:15
ahoppen
ahoppen reviewed Apr 17, 2024
View reviewed changes
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Looks like the Float and Int initializers do indeed do the job here. Could you also add an entry to the release notes?

@rintaro
Copy link
Member

rintaro commented Apr 18, 2024 •
edited
Loading

StringLiteralExprSyntax has representedLiteralValue property. IMO, we should use the same name. One question is whether we should put them in SwiftParser, WDYT @ahoppen ?

@ahoppen
Copy link
Member

ahoppen commented Apr 18, 2024

StringLiteralExprSyntax has representedLiteralValue property. IMO, we should use the same name. One question is whether we should put them in SwiftParser, WDYT @ahoppen ?

I agree with the naming. Unless there is a reason to put them in the parser (which I don’t think there is, StringLIteralExprSyntax.representedLiteralValue only needs it because it hooks back into the lexer for unicode parsing (eg. \u{1F36D}). But since we don’t need it here, I’d just put it in the SwiftSyntax module.

@kimdv kimdv force-pushed the kimdv/405-sr-13584-make-convenience-methods-returning-an-the-value-of-an-integer-literal-as-int-and-of-floating-literal-as-float-return-an-optional branch 4 times, most recently from 7d5b12f to 371fa2a Compare April 20, 2024 20:31
@kimdv
Copy link
Contributor Author

kimdv commented Apr 20, 2024

representedLiteralValue

Renamed to representedLiteralValue

ahoppen
ahoppen approved these changes Apr 22, 2024
View reviewed changes
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@kimdv kimdv force-pushed the kimdv/405-sr-13584-make-convenience-methods-returning-an-the-value-of-an-integer-literal-as-int-and-of-floating-literal-as-float-return-an-optional branch from 371fa2a to 7ef31c5 Compare April 23, 2024 08:19
@kimdv
Copy link
Contributor Author

kimdv commented Apr 23, 2024

@ahoppen should this also have a release note?

@ahoppen
Copy link
Member

ahoppen commented Apr 23, 2024

Yes, please. I thought I amended my review with that but looks like I never hit the submit button.

@kimdv
Copy link
Contributor Author

kimdv commented Apr 24, 2024

Yes, please. I thought I amended my review with that but looks like I never hit the submit button.

Could this do it?

@kimdv kimdv force-pushed the kimdv/405-sr-13584-make-convenience-methods-returning-an-the-value-of-an-integer-literal-as-int-and-of-floating-literal-as-float-return-an-optional branch from 5bcb556 to c6a2c22 Compare April 24, 2024 18:47
@kimdv
Copy link
Contributor Author

kimdv commented Apr 24, 2024

@swift-ci please test

@kimdv kimdv enabled auto-merge April 24, 2024 18:48
hamishknight
hamishknight approved these changes Apr 24, 2024
View reviewed changes
@kimdv kimdv force-pushed the kimdv/405-sr-13584-make-convenience-methods-returning-an-the-value-of-an-integer-literal-as-int-and-of-floating-literal-as-float-return-an-optional branch 2 times, most recently from e303412 to 34a55fc Compare April 24, 2024 19:32
@kimdv
Copy link
Contributor Author

kimdv commented Apr 24, 2024

@swift-ci please test

@kimdv kimdv disabled auto-merge April 24, 2024 20:08
@kimdv
Copy link
Contributor Author

kimdv commented Apr 24, 2024

@swift-ci please test windows

bnbarham
bnbarham reviewed Apr 24, 2024
View reviewed changes
@kimdv kimdv force-pushed the kimdv/405-sr-13584-make-convenience-methods-returning-an-the-value-of-an-integer-literal-as-int-and-of-floating-literal-as-float-return-an-optional branch from 34a55fc to 425e80a Compare April 25, 2024 06:20
@kimdv kimdv requested review from bnbarham and rintaro April 25, 2024 06:20
@kimdv kimdv force-pushed the kimdv/405-sr-13584-make-convenience-methods-returning-an-the-value-of-an-integer-literal-as-int-and-of-floating-literal-as-float-return-an-optional branch from 425e80a to 0bc8743 Compare April 26, 2024 06:54
@kimdv kimdv force-pushed the kimdv/405-sr-13584-make-convenience-methods-returning-an-the-value-of-an-integer-literal-as-int-and-of-floating-literal-as-float-return-an-optional branch from 0bc8743 to e9fcb4d Compare April 28, 2024 16:16
@kimdv
Copy link
Contributor Author

kimdv commented Apr 28, 2024

@bnbarham @rintaro can you please review :D
Rebased on latest main

@kimdv
Copy link
Contributor Author

kimdv commented Apr 28, 2024

@swift-ci please test

@kimdv
Copy link
Contributor Author

kimdv commented Apr 28, 2024

@swift-ci please test windows

@kimdv kimdv force-pushed the kimdv/405-sr-13584-make-convenience-methods-returning-an-the-value-of-an-integer-literal-as-int-and-of-floating-literal-as-float-return-an-optional branch from e9fcb4d to a576b13 Compare April 28, 2024 19:12
@kimdv
Copy link
Contributor Author

kimdv commented Apr 28, 2024

@swift-ci please test

@ahoppen ahoppen merged commit 42c1cef into swiftlang:main Apr 29, 2024
3 checks passed
@kimdv kimdv deleted the kimdv/405-sr-13584-make-convenience-methods-returning-an-the-value-of-an-integer-literal-as-int-and-of-floating-literal-as-float-return-an-optional branch May 4, 2024 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahoppen ahoppen ahoppen approved these changes

@hamishknight hamishknight hamishknight approved these changes

@rintaro rintaro rintaro approved these changes

@bnbarham bnbarham Awaiting requested review from bnbarham bnbarham is a code owner

Assignees

@kimdv kimdv

Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@kimdv @rintaro @ahoppen @bnbarham @hamishknight