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.

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

Fix MSVC compiler error in basic_ops.h #93322

Closed
wants to merge 1 commit into from
Closed

Fix MSVC compiler error in basic_ops.h #93322

wants to merge 1 commit into from

Conversation

osalpekar
Copy link
Member

@osalpekar osalpekar commented Jan 31, 2023 •
edited

#93069 introduces a compiler error in some internal Windows builds using MSVC:

stderr: d:\full-fbsource\xplat\caffe2\torch\csrc\autograd\functions\basic_ops.h(43): fatal error C1001: An internal error has occurred in the compiler.

This may be related to older versions of MSVC not recognizing the [[maybe-unused]] attribute: https://developercommunity.visualstudio.com/t/compiler-bug-on-parsing-maybe-unused-in-range-base/209488. This PR reverts the changes in basic_ops.h that resolves those errors.

Verified this fixes the internal jobs, and landed as D42854205.

@pytorch-bot
Copy link

pytorch-bot bot commented Jan 31, 2023 •
edited

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/93322

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Failures

As of commit a48a34f:

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Skylion007
Skylion007 approved these changes Jan 31, 2023
View reviewed changes
Copy link
Collaborator

@Skylion007 Skylion007 left a comment

Choose a reason for hiding this comment

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

Yeah, this is actually jut another C++17 feature sneaking into the codebase, not MSVC's fault. Let's hope all the internal downstream libraries can get on C++17 soon.

albanD
albanD approved these changes Jan 31, 2023
View reviewed changes
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

SGTM!

@albanD
Copy link
Collaborator

albanD commented Jan 31, 2023

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 31, 2023
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / win-vs2019-cuda11.6-py3 / test (default, 1, 5, windows.g5.4xlarge.nvidia.gpu)

Details for Dev Infra team Raised by workflow job

@albanD
Copy link
Collaborator

albanD commented Jan 31, 2023

@pytorchbot merge -f "Infra issue"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Skylion007 Skylion007 Skylion007 approved these changes

@albanD albanD albanD approved these changes

@soulitzer soulitzer Awaiting requested review from soulitzer

Assignees
No one assigned
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@osalpekar @albanD @pytorchmergebot @Skylion007