Skip to content

Add support for nested var names in parser #75124

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
Closed
eellison wants to merge 3 commits into from
Closed

Add support for nested var names in parser #75124

eellison wants to merge 3 commits into from

Conversation

@eellison
Copy link
Contributor

@eellison eellison commented Apr 1, 2022 •
edited
Loading

This was referenced Apr 1, 2022
Closed
Closed
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Apr 1, 2022 •
edited
Loading

🔗 Helpful links

💊 CI failures summary and remediations

As of commit c4efd89 (more details on the Dr. CI page):

  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages

See GitHub Actions build Lint / clang-tidy (1/1)

Step: "Check for warnings" (full log | diagnosis details | 🔁 rerun)

2022-04-05T20:45:45.0969001Z /__w/pytorch/pytor...coreguidelines-init-variables,-warnings-as-errors] 
2022-04-05T20:45:44.8698179Z �[36;1mcd "${GITHUB_WORKSPACE}"�[0m
2022-04-05T20:45:44.8698374Z �[36;1mset -eu�[0m
2022-04-05T20:45:44.8698590Z �[36;1mcat "${GITHUB_WORKSPACE}"/clang-tidy-output.txt�[0m
2022-04-05T20:45:44.8698916Z �[36;1mif grep -Fq "Warnings detected!" "${GITHUB_WORKSPACE}"/clang-tidy-output.txt; then�[0m
2022-04-05T20:45:44.8699234Z �[36;1m  echo 'Please fix the above clang-tidy warnings.'�[0m
2022-04-05T20:45:44.8699437Z �[36;1m  false�[0m
2022-04-05T20:45:44.8699600Z �[36;1mfi�[0m
2022-04-05T20:45:44.8699809Z shell: sh -e {0}
2022-04-05T20:45:44.8699972Z ##[endgroup]
2022-04-05T20:45:45.0968140Z Processing 1 clang-tidy jobs
2022-04-05T20:45:45.0969001Z /__w/pytorch/pytorch/torch/csrc/jit/ir/irparser.cpp:138:8: error: variable 'continue_parsing' is not initialized [cppcoreguidelines-init-variables,-warnings-as-errors]
2022-04-05T20:45:45.0971390Z   bool continue_parsing;
2022-04-05T20:45:45.0971671Z        ^
2022-04-05T20:45:45.0971928Z                         = 0
2022-04-05T20:45:45.0972295Z Warnings detected!
2022-04-05T20:45:45.0972588Z Summary:
2022-04-05T20:45:45.0973031Z [cppcoreguidelines-init-variables] occurred 1 times
2022-04-05T20:45:45.0975208Z     /__w/pytorch/pytorch/torch/csrc/jit/ir/irparser.cpp:138
2022-04-05T20:45:45.0975563Z 
2022-04-05T20:45:45.0975826Z Processing 59 clang-tidy jobs
2022-04-05T20:45:45.0976301Z Please fix the above clang-tidy warnings.
This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Apr 1, 2022
eellison pushed a commit that referenced this pull request Apr 1, 2022
Add support for nested var names in parser
f2af986 
ghstack-source-id: 980fe9c
Pull Request resolved: #75124
@eellison eellison requested a review from davidberard98 April 1, 2022 19:10
Update on "Add support for nested var names in parser"
55fa470 
These occur with freezing cc @Krovatkin 

[ghstack-poisoned]
eellison pushed a commit that referenced this pull request Apr 4, 2022
Add support for nested var names in parser
931616a 
ghstack-source-id: f4592e5
Pull Request resolved: #75124
davidberard98
davidberard98 approved these changes Apr 4, 2022
View reviewed changes
Copy link
Contributor

@davidberard98 davidberard98 left a comment

Choose a reason for hiding this comment

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

I think one of the tests from the previous PR ended up in this one.

Otherwise, looks good!

@eellison
Copy link
Contributor Author

eellison commented Apr 4, 2022

@eellison has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Update on "Add support for nested var names in parser"
c4efd89 
These occur with freezing cc @Krovatkin

Differential Revision: [D35373998](https://our.internmc.facebook.com/intern/diff/D35373998)

[ghstack-poisoned]
eellison pushed a commit that referenced this pull request Apr 5, 2022
Add support for nested var names in parser
eb58305 
ghstack-source-id: f79dd82
Pull Request resolved: #75124
@eellison
Copy link
Contributor Author

eellison commented Apr 5, 2022

@eellison has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Apr 6, 2022
@facebook-github-bot
Add support for nested var names in parser (#75124)
89dc618 
Summary:
Pull Request resolved: #75124

These occur with freezing cc Krovatkin

Test Plan: Imported from OSS

Reviewed By: ejguan

Differential Revision: D35373998

Pulled By: eellison

fbshipit-source-id: c043d728900f833b8d027ff75b088f9d3eb389e0
@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2022

Hey @eellison.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@facebook-github-bot facebook-github-bot deleted the gh/eellison/291/head branch April 10, 2022 14:17
@WBobby WBobby mentioned this pull request Aug 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@davidberard98 davidberard98 davidberard98 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla signed oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@eellison @facebook-github-bot @davidberard98