[JIT] Resolve string literal type annotations using Resolver::resolveType
#47731
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
…eType` **Summary** This commit modifies `ScriptTypeParser::parseTypeFromExpr` so that string literal type annotations are resolved using `Resolver::resolveType`. At present, they are parsed in `parseBaseTypeName`, which inadvertently allows any key from `string_to_type_lut` to be used as a string literal type annotation. **Test Plan** Existing unit tests (most notably `TestClassType.test_self_referential_method` which tests the main feature, self-referential class type annotations, that make use of string literal type annotations). **Fixes** This commit fixes #47570. [ghstack-poisoned]
facebook-github-bot
added
cla signed
oncall: jit
SplitInfinity
pushed a commit
that referenced
this pull request
Nov 11, 2020
…eType` **Summary** This commit modifies `ScriptTypeParser::parseTypeFromExpr` so that string literal type annotations are resolved using `Resolver::resolveType`. At present, they are parsed in `parseBaseTypeName`, which inadvertently allows any key from `string_to_type_lut` to be used as a string literal type annotation. **Test Plan** Existing unit tests (most notably `TestClassType.test_self_referential_method` which tests the main feature, self-referential class type annotations, that make use of string literal type annotations). **Fixes** This commit fixes #47570. ghstack-source-id: 9e5a2c83f6211084a378df1863e662e2ced936ce Pull Request resolved: #47731
💊 CI failures summary and remediationsAs of commit 0eac8d4 (more details on the Dr. CI page):
1 failure not recognized by patterns:
This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 11 times. |
gmagogsfm
approved these changes
Nov 12, 2020
…ver::resolveType`" **Summary** This commit modifies `ScriptTypeParser::parseTypeFromExpr` so that string literal type annotations are resolved using `Resolver::resolveType`. At present, they are parsed in `parseBaseTypeName`, which inadvertently allows any key from `string_to_type_lut` to be used as a string literal type annotation. **Test Plan** Existing unit tests (most notably `TestClassType.test_self_referential_method` which tests the main feature, self-referential class type annotations, that make use of string literal type annotations). **Fixes** This commit fixes #47570. [ghstack-poisoned]
SplitInfinity
pushed a commit
that referenced
this pull request
Nov 12, 2020
…eType` **Summary** This commit modifies `ScriptTypeParser::parseTypeFromExpr` so that string literal type annotations are resolved using `Resolver::resolveType`. At present, they are parsed in `parseBaseTypeName`, which inadvertently allows any key from `string_to_type_lut` to be used as a string literal type annotation. **Test Plan** Existing unit tests (most notably `TestClassType.test_self_referential_method` which tests the main feature, self-referential class type annotations, that make use of string literal type annotations). **Fixes** This commit fixes #47570. ghstack-source-id: 6323420f3446f75269e5d664450ee14be493b8f3 Pull Request resolved: #47731
|
ROCm CI failure due to CI script update. Will retest. |
|
@pytorchbot retest this please |
|
@SplitInfinity merged this pull request in 03d1978. |
|
@SplitInfinity merged this pull request in 03d1978. |
|
That's great! Didn't notice this fix is also coming |
tugsbayasgalan
pushed a commit
to tugsbayasgalan/pytorch
that referenced
this pull request
Nov 16, 2020
…eType` (pytorch#47731) Summary: Pull Request resolved: pytorch#47731 **Summary** This commit modifies `ScriptTypeParser::parseTypeFromExpr` so that string literal type annotations are resolved using `Resolver::resolveType`. At present, they are parsed in `parseBaseTypeName`, which inadvertently allows any key from `string_to_type_lut` to be used as a string literal type annotation. **Test Plan** Existing unit tests (most notably `TestClassType.test_self_referential_method` which tests the main feature, self-referential class type annotations, that make use of string literal type annotations). **Fixes** This commit fixes pytorch#47570. Test Plan: Imported from OSS Reviewed By: navahgar Differential Revision: D24934717 Pulled By: SplitInfinity fbshipit-source-id: b915b2c08272566b63b3cf5ff4a07ad43bdc381a
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.
Stack from ghstack:
Resolver::resolveType#47731 [JIT] Resolve string literal type annotations usingResolver::resolveTypeSummary
This commit modifies
ScriptTypeParser::parseTypeFromExprso thatstring literal type annotations are resolved using
Resolver::resolveType. At present, they are parsed inparseBaseTypeName, which inadvertently allows any key fromstring_to_type_lutto be used as a string literal type annotation.Test Plan
Existing unit tests (most notably
TestClassType.test_self_referential_methodwhich tests the mainfeature, self-referential class type annotations, that make use of
string literal type annotations).
Fixes
This commit fixes #47570.
Differential Revision: D24934717