Skip to content

Suppress or fix clang-tidy-9 warnings#4366

Merged
aurianer merged 3 commits into
TheHPXProject:masterfrom
msimberg:clang-tidy-fixes
Feb 6, 2020
Merged

Suppress or fix clang-tidy-9 warnings#4366
aurianer merged 3 commits into
TheHPXProject:masterfrom
msimberg:clang-tidy-fixes

Conversation

@msimberg
Copy link
Copy Markdown
Contributor

@msimberg msimberg commented Feb 4, 2020

Attempting to fix new warnings after updating from 8 to 9.

biddisco
biddisco previously approved these changes Feb 4, 2020
Copy link
Copy Markdown
Contributor

@biddisco biddisco left a comment

Choose a reason for hiding this comment

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

LGTM

@msimberg msimberg force-pushed the clang-tidy-fixes branch 4 times, most recently from d57120e to de231aa Compare February 4, 2020 11:50
for (std::size_t d = 0; d < num_domains_; ++d)
{
std::uint16_t dom =
std::size_t dom =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think with this change all of the static_cast's can go away now as well.

Comment thread hpx/util/thread_aware_timer.hpp Outdated
@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Feb 4, 2020

Btw, I've suppressed quite a few warnings that to me look like false positives. If you see something that actually seems like a bug please let me know and I'll try to fix it.

@msimberg msimberg force-pushed the clang-tidy-fixes branch 11 times, most recently from ec70553 to 47d4396 Compare February 5, 2020 10:16
@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Feb 5, 2020

Related to the docker image upgrade, the python command isn't available anymore (only python3). We have a few Python scripts which explicitly use /usr/bin/env python which now fail on CircleCI. We can:

  • change python to python3,
  • symlink python to python3 in the docker image, or
  • generate the shebang line in those scripts like we do for scripts that get installed.

My preference is in the order they're listed. Opinions?

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Feb 5, 2020

Since python V2 is being phased out everywhere, I think we are safe to rename the command to python3. I don't know however if this would be correct on all platforms. Option 3 might be the safest way forward.

@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Feb 5, 2020

Since python V2 is being phased out everywhere, I think we are safe to rename the command to python3. I don't know however if this would be correct on all platforms. Option 3 might be the safest way forward.

That's my thinking as well. I see that we actually install many of the Python scripts. I'll generate them with the Python that we find during configuration. Scripts that aren't installed I'll leave with python3.

This is finally passing clang-tidy now. The spell checker is complaining about some changes that I missed earlier. I'll leave those out of this PR and look at them separately (or try to ignore them if possible).

@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Feb 5, 2020

I unfortunately don't have the time to update the scripts now until Monday. If someone wants to push to this branch feel free to do so so that we can get builds working for everyone, or we just merge it like this and I change the python shebangs separately next week.

@aurianer aurianer merged commit c668aa6 into TheHPXProject:master Feb 6, 2020
@msimberg msimberg deleted the clang-tidy-fixes branch February 10, 2020 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants