Skip to content

const evaluator: string values and init operations #21711

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

Merged
merged 2 commits into from
Jan 12, 2019

Conversation

marcrasi
Copy link

@marcrasi marcrasi commented Jan 8, 2019

Adds string values, string initialization operations (using a "WellKnownFunction" mechanism), and some tests that invoke string initialization but do not check the contents of the strings.

This very basic string handling is all that we have in the tensorflow branch const evaluator right now.

return None;
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@marcrasi I wonder if it would be better to make a separate function for lines 560 to 591 that handles well-known functions. As we plan to add more well-known functions, it would make this more modular.

Copy link
Author

Choose a reason for hiding this comment

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

done!

@ravikandhadai
Copy link
Contributor

@marcrasi LGTM otherwise.

@ravikandhadai
Copy link
Contributor

Practically speaking, though, would it be better to represent those common collection types in a more symbolic fashion for compile-time evaluation? Especially if the compile-time evaluator has a cap on number of operations or some other complexity cap, interpreting the standard library implementations at such a low level seems like it could easily blow that cap, or else just be painfully slow to interpret in practice.

@jckarter Related to your suggestion on handling collection types symbolically, I just wanted to give heads up that this PR adds basic support for symbolically representing strings and interpreting their initializers. Representation for arrays and array initialization is added by this PR #21703

@ravikandhadai
Copy link
Contributor

@swift-ci Please smoke test

Copy link
Contributor

@devincoughlin devincoughlin left a comment

Choose a reason for hiding this comment

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

LGTM.

@ravikandhadai
Copy link
Contributor

@swift-ci Please test and merge

1 similar comment
@ravikandhadai
Copy link
Contributor

@swift-ci Please test and merge

@ravikandhadai
Copy link
Contributor

@marcrasi Once this is merged, can we upstream enums before merging arrays?

@swift-ci swift-ci merged commit b3a2052 into master Jan 12, 2019
@compnerd compnerd deleted the marcrasi-const-evaluator-strings branch May 31, 2025 20:30
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.

4 participants