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

chore: add deprecation warnings when casting string/bytes to regex #20944

Merged
merged 3 commits into from
Nov 29, 2022

Conversation

arezaii
Copy link
Collaborator

@arezaii arezaii commented Nov 1, 2022

This PR marks casts from strings or bytes to regex as deprecated and
suggests using compile() from Regex module as a replacement until
throwing initializers are implemented and we can suggest new regex().

Updates some test .good files to expect the deprecation message.

TESTING:

  • paratest

reviewd by @jeremiah-corrado - thanks!

@arezaii arezaii marked this pull request as ready for review November 1, 2022 16:06
* update .good with deprecation messages for string to regex

Signed-off-by: arezaii <ahmad.rezaii@hpe.com>
@arezaii arezaii force-pushed the deprecate-regex-to-string branch from 990cb1e to 6d3d5c8 Compare November 1, 2022 18:50
@@ -1444,7 +1444,10 @@ module ChapelBase {
// string to a type. Otherwise, we can't resolve chpl_debug_writeln in
// `range.these`
{ var dummyRange = 1..0; for i in dummyRange {} }
return str:t;
if t:string == "regex(string)" || t:string == "regex(bytes)" then
Copy link
Member

Choose a reason for hiding this comment

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

how about if t == regex(string) || t == regex(bytes) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I attempted to make this change and ran into issues. I added some comments to document the problems encountered and noted that we would prefer to use the type comparison rather than string comparison here.

Copy link
Contributor

@jeremiah-corrado jeremiah-corrado 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!

I just have one minor suggestion:

@@ -1,3 +1,9 @@
cast.chpl:8: warning: Casting strings to regex is deprecated. Use regex.compile(string) instead.
cast.chpl:9: warning: Casting bytes to regex is deprecated. Use regex.compile(bytes) instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

I might remove lines 8-15 from this test since the deprecated cast is still tested in /deprecated/regexTertiary

@arezaii arezaii force-pushed the deprecate-regex-to-string branch from 2d239f5 to 6d3d5c8 Compare November 28, 2022 20:18
Signed-off-by: arezaii <ahmad.rezaii@hpe.com>
Signed-off-by: arezaii <ahmad.rezaii@hpe.com>
@arezaii arezaii force-pushed the deprecate-regex-to-string branch from 5bee0b5 to 4bab3aa Compare November 28, 2022 20:43
@arezaii arezaii merged commit e54cef9 into chapel-lang:main Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants