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 compiler error in ASLocking #trivial #1079

Merged
merged 2 commits into from
Aug 22, 2018
Merged

Fix compiler error in ASLocking #trivial #1079

merged 2 commits into from
Aug 22, 2018

Conversation

nguyenhuy
Copy link
Member

@nguyenhuy nguyenhuy commented Aug 21, 2018 •
edited
Loading

Hitting this error because we're compiling with a stricter set of build rules internally.

nslockset

@nguyenhuy nguyenhuy requested a review from Adlai-Holler August 21, 2018 05:35
maicki
maicki approved these changes Aug 21, 2018
View reviewed changes
Copy link
Contributor

@maicki maicki left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM!

@nguyenhuy nguyenhuy force-pushed the HN-Fix-Build-Failuree branch from 45e9cbb to af032b5 Compare August 21, 2018 17:08
@ghost
Copy link

ghost commented Aug 21, 2018

🚫 CI failed with log

Adlai-Holler
Adlai-Holler reviewed Aug 21, 2018
View reviewed changes
Copy link
Member

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

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

I don't think this is correct – if we don't initialize the variable it will contain garbage values. What's the specific compiler error we see?

@nguyenhuy
Copy link
Member Author

Good point, updated the diff. The compiler complaints about missing initialization for the lock set’s locks array (i.e locks.locks).

Adlai-Holler
Adlai-Holler approved these changes Aug 22, 2018
View reviewed changes
Copy link
Member

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

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

Good deal. I think that {0} is only valid for multi-member structs in C++. In C it may not be well-defined. Do we not also use that construct in ASPendingState?

@nguyenhuy
Copy link
Member Author

Looks like we do, but the engineer who's working on this project hasn't hit it yet (and I heard it's non-trivial for me to apply all of her changes). Will follow-up if need to.

@nguyenhuy
Copy link
Member Author

Thanks btw!

@nguyenhuy nguyenhuy merged commit 31cb65b into TextureGroup:master Aug 22, 2018
@nguyenhuy nguyenhuy deleted the HN-Fix-Build-Failuree branch August 22, 2018 23:14
mikezucc pushed a commit to mikezucc/Texture that referenced this pull request Oct 2, 2018
@nguyenhuy @mikezucc
Fix compiler error in ASLocking #trivial (TextureGroup#1079)
449f23b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Adlai-Holler Adlai-Holler Adlai-Holler approved these changes

@maicki maicki maicki approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nguyenhuy @maicki @Adlai-Holler