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

Change C-style casts to static_cast and remove unnecessary semicolons #668

Merged
merged 11 commits into from
Sep 10, 2024

Conversation

jmcarcell
Copy link
Member

I have been playing a bit with some warnings in Key4hep and I get some from included files from podio. The C-style cast may be useful. I guess these don't hurt anyway.

BEGINRELEASENOTES

  • Change C-style casts to static_cast and remove unnecessary semicolons

ENDRELEASENOTES

Copy link
Collaborator

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Looks like there were quite a few. Thanks for cleaning up. Minor comment for the ones where we could also use a .f possibly.

tests/read_test.h Outdated Show resolved Hide resolved
tests/root_io/read_and_write_associated.cpp Outdated Show resolved Hide resolved
jmcarcell and others added 2 commits September 9, 2024 11:29
Co-authored-by: Thomas Madlener <thomas.madlener@desy.de>
Co-authored-by: Thomas Madlener <thomas.madlener@desy.de>
Copy link
Collaborator

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Thanks.

@m-fila
Copy link
Contributor

m-fila commented Sep 9, 2024

Here is another semicolon to be removed

std::string AsString() override {
return "Podio data source";
};

There is a clang-format check for this https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removesemicolon but there is a disclaimer that it may produce incorrect code so probably we don't want to add it

Also there is a clang-tidy check for casts but it misses some cases and flags the code in MurmurHash so I guess there is little reason to add it

@jmcarcell
Copy link
Member Author

For casts there is -Wold-style-cast. I added a pragma for Murmurhash3 so that it doesn't complain.

@m-fila
Copy link
Contributor

m-fila commented Sep 9, 2024

Are these flags and pragmas supported by all the compilers we have?

@jmcarcell
Copy link
Member Author

With all the compilers we have being GCC yes, but they also work fine with Clang.

@tmadlener tmadlener merged commit 0777022 into AIDASoft:master Sep 10, 2024
18 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