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.

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

make clang-tidy recognize .H as a header #3832

Merged
merged 6 commits into from
Apr 2, 2024

Conversation

zingale
Copy link
Member

@zingale zingale commented Mar 21, 2024

Summary

the output of clang-tidy --dump-config shows that .H is not recognized as a header

Additional background

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

@zingale
Copy link
Member Author

zingale commented Mar 21, 2024

hmmm... the version of clang-tidy in the CI doesn't seem to like this. But it does fix things when I run locally with clang-tidy 17

@zingale
Copy link
Member Author

zingale commented Mar 21, 2024

indeed, this option was only added in clang-tidy 17:
https://releases.llvm.org/17.0.1/tools/clang/tools/extra/docs/ReleaseNotes.html

there must be another way to tell clang-tidy to recognize the headers for old versions. At the moment, clang-tidy is not doing all the checks cause it doesn't treat .H as a header

@WeiqunZhang
Copy link
Member

We can add this to the config file for now.

CheckOptions:
    - key:             bugprone-dynamic-static-initializers.HeaderFileExtensions
      value:           ';H;h;hh;hpp;hxx'
    - key:             bugprone-suspicious-include.HeaderFileExtensions
      value:           ';H;h;hh;hpp;hxx'
    - key:             google-global-names-in-headers.HeaderFileExtensions
      value:           ';H;h;hh;hpp;hxx'
    - key:             google-build-namespaces.HeaderFileExtensions
      value:           ';H;h;hh;hpp;hxx'
    - key:             misc-definitions-in-headers.HeaderFileExtensions
      value:           ';H;h;hh;hpp;hxx'
    - key:             misc-use-anonymous-namespace.HeaderFileExtensions
      value:           ';H;h;hh;hpp;hxx'

@WeiqunZhang
Copy link
Member

I tested the CheckOptions above on my local branch. it does seem to work. I now see
/home/runner/work/amrex/amrex/Src/Particle/AMReX_Particle.H:16:1: error: do not use unnamed namespaces in header files [google-build-namespaces,-warnings-as-errors]
namespace
^

@zingale
Copy link
Member Author

zingale commented Mar 21, 2024

okay. I'll update the PR

@WeiqunZhang
Copy link
Member

There will be many errors to fix. Do you want me to help?

@zingale
Copy link
Member Author

zingale commented Mar 21, 2024

I can help. But I'm working through Castro and Microphysics first. Maybe we hold this for a little bit until the other tidy PRs are all done.

@ax3l
Copy link
Member

ax3l commented Apr 2, 2024

fyi @lucafedeli88

@WeiqunZhang
Copy link
Member

After #3867, we can go back to the first commit of this PR.

@WeiqunZhang WeiqunZhang merged commit d78e241 into AMReX-Codes:development Apr 2, 2024
68 of 69 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants