Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@bkonyi
Copy link
Contributor

@bkonyi bkonyi commented Dec 8, 2017

No description provided.

@bkonyi bkonyi merged commit c3f18b9 into master Dec 8, 2017
Copy link
Contributor

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

We should come up with a more maintainable solution to window compiler oddities. I dont think one off preprocessor guards are the way to go.

for (size_t i = 0; i < count; i++) {
loop.GetTaskRunner()->PostTask([&terminated, i, &current, count]() {
loop.GetTaskRunner()->PostTask([&terminated, i, &current
#if OS_WIN
Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea what issue this is trying to work around on Windows. This is not maintainable and I am sure the next time anyone tries to modify such TUs, he/she is going to miss one of these preprocessor guards.

@chinmaygarde
Copy link
Contributor

Also, please get PRs reviewed before landing. Adding a reviewer to the PR usually gets that persons attention pretty quickly.

@bkonyi
Copy link
Contributor Author

bkonyi commented Dec 9, 2017

@chinmaygarde my bad, I should have submitted for review but was trying to get the bots green. The change above is because MSVC requires count to be included in the lambda capture to not be considered a compilation error, but the toolchain on Linux complains that count is not used and the compilation fails. Do you have any suggestion on how this can be resolved?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants