-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Use narrow type for string parsing patterns #7211
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Sep 7, 2023
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
dhruvmanila
force-pushed
the
dhruv/fstring-parser-2
branch
3 times, most recently
from
September 7, 2023 07:21
90844fd
to
bddcd81
Compare
dhruvmanila
force-pushed
the
dhruv/fstring-parser-2
branch
2 times, most recently
from
September 7, 2023 12:19
0f3792b
to
c1dd71e
Compare
dhruvmanila
changed the title
Use narrow types for string parsing patterns
Use narrow type for string parsing patterns
Sep 7, 2023
dhruvmanila
force-pushed
the
dhruv/fstring-parser-2
branch
2 times, most recently
from
September 7, 2023 12:30
0d6dd65
to
b4505db
Compare
MichaReiser
approved these changes
Sep 7, 2023
dhruvmanila
force-pushed
the
dhruv/fstring-parser
branch
from
September 8, 2023 03:27
1977e2f
to
ad56d52
Compare
dhruvmanila
force-pushed
the
dhruv/fstring-parser-2
branch
from
September 8, 2023 03:27
b4505db
to
dd4cf37
Compare
dhruvmanila
force-pushed
the
dhruv/fstring-parser
branch
from
September 8, 2023 03:32
ad56d52
to
dd22d12
Compare
dhruvmanila
force-pushed
the
dhruv/fstring-parser-2
branch
from
September 8, 2023 03:32
dd4cf37
to
6ad8aeb
Compare
dhruvmanila
force-pushed
the
dhruv/fstring-parser
branch
from
September 11, 2023 15:59
dd22d12
to
6abb0ea
Compare
dhruvmanila
force-pushed
the
dhruv/fstring-parser-2
branch
from
September 11, 2023 15:59
6ad8aeb
to
cb367d1
Compare
dhruvmanila
force-pushed
the
dhruv/fstring-parser
branch
from
September 12, 2023 05:00
6abb0ea
to
8f2c36c
Compare
dhruvmanila
force-pushed
the
dhruv/fstring-parser-2
branch
from
September 12, 2023 05:00
cb367d1
to
8a1b9b5
Compare
This will be used to extract the `leading` and `trailing` text for f-string debug expressions.
dhruvmanila
force-pushed
the
dhruv/fstring-parser
branch
from
September 14, 2023 01:37
8f2c36c
to
19653a3
Compare
dhruvmanila
force-pushed
the
dhruv/fstring-parser-2
branch
from
September 14, 2023 01:37
8a1b9b5
to
47c3215
Compare
CodSpeed Performance ReportMerging #7211 will degrade performances by 9.7%Falling back to comparing Summary
Benchmarks breakdown
|
dhruvmanila
added a commit
that referenced
this pull request
Sep 18, 2023
## Summary This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? ## Test Plan `cargo test`
dhruvmanila
added a commit
that referenced
this pull request
Sep 19, 2023
## Summary This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? ## Test Plan `cargo test`
dhruvmanila
added a commit
that referenced
this pull request
Sep 20, 2023
## Summary This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? ## Test Plan `cargo test`
dhruvmanila
added a commit
that referenced
this pull request
Sep 22, 2023
This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? `cargo test`
dhruvmanila
added a commit
that referenced
this pull request
Sep 22, 2023
This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? `cargo test`
dhruvmanila
added a commit
that referenced
this pull request
Sep 22, 2023
This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? `cargo test`
dhruvmanila
added a commit
that referenced
this pull request
Sep 26, 2023
This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? `cargo test`
dhruvmanila
added a commit
that referenced
this pull request
Sep 27, 2023
This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? `cargo test`
dhruvmanila
added a commit
that referenced
this pull request
Sep 28, 2023
This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? `cargo test`
dhruvmanila
added a commit
that referenced
this pull request
Sep 29, 2023
This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? `cargo test`
dhruvmanila
added a commit
that referenced
this pull request
Sep 29, 2023
This PR adds a new enum type `StringType` which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted in `concatenate_strings` as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about. A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts: 1. A single string literal or a f-string: This means no checking for bytes / non-bytes and other unnecessary compution 2. Two or more of string/byte/f-string: This will call the `concatenate_strings` function. The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range. To give an example, for `"foo"`, the range of the string would be `0..5`, but for `f"foo"` the range of the string would be `2..5` while the range for the f-string expression would be `0..6`. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string? `cargo test`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds a new enum type
StringType
which is either a string literal, byte literal or f-string. The motivation behind this is to have a narrow type which is accepted inconcatenate_strings
as that function is only applicable for the mentioned 3 types. This makes the code more readable and easy to reason about.A future improvement (which was prototyped here and removed) is to split the current string literal pattern in LALRPOP definition into two parts:
concatenate_strings
function.The reason for removing the second change is because of how ranges work. The range for an individual string/byte is the entire range which includes the quotes as well but if the same string/byte is part of a f-string, then it only includes the range for the content (without the quotes / inner range). The current string parser returns with the former range.
To give an example, for
"foo"
, the range of the string would be0..5
, but forf"foo"
the range of the string would be2..5
while the range for the f-string expression would be0..6
. The ranges are correct but they differ in the context the string constant itself is being used for. Is it part of a f-string or is it a standalone string?Test Plan
cargo test