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

Clang and warnings fixes #88

Merged
merged 9 commits into from
Apr 16, 2024
Merged

Conversation

iglesias
Copy link
Collaborator

I've started onboarding llvm tooling (off-topic, just for info, I am aiming at doing some toy exercise with AST matching), and started running experimenting with clang-check chain_interface.hpp.

The ones in the unit test were compilation warnings related to multi-dimensional indexing.

I didn't run it in other files yet. I'd say let's keep the PR open for some days to continue pushing small fixes like these one.

clang-check warned about depecrated use of implicitly defined copy
constructor since copy assignment existed. Rule of Five.
open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1161r3.html
@iglesias iglesias requested a review from lisitsyn April 15, 2024 13:54
ASSERT_EQ(static_cast<int>(pg[target_dimension]), td);
ASSERT_EQ(static_cast<int>(pg[num_neighbors]), k);
}

TEST(Interface, OneParameterParametersSet)
{
int td = 3;
tapkee::ParametersSet pg = tapkee::kwargs[target_dimension = td];
tapkee::ParametersSet pg = tapkee::kwargs[(target_dimension = td)];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oeps, I think I overlooked when changing this one ':-) without parentheses there should still be no warning in this one. I will modify it back.

@@ -220,7 +220,7 @@ TEST(Interface, NoReductionMethodSetFailPassThru)

TapkeeOutput output;
// should fail with missed parameter
ASSERT_THROW(output = embed(data.begin(), data.end(), kcb, dcb, fcb, tapkee::kwargs[eigen_method = Dense]),
ASSERT_THROW(output = embed(data.begin(), data.end(), kcb, dcb, fcb, tapkee::kwargs[(eigen_method = Dense)]),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same as in the other comment in this one.

Copy link
Owner

@lisitsyn lisitsyn left a comment

Choose a reason for hiding this comment

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

Good to go!

@iglesias iglesias marked this pull request as ready for review April 16, 2024 07:50
Copy link
Owner

@lisitsyn lisitsyn left a comment

Choose a reason for hiding this comment

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

Ready to go!

@lisitsyn lisitsyn merged commit 7226b75 into lisitsyn:main Apr 16, 2024
4 of 6 checks passed
@iglesias iglesias deleted the clang-and-warnings-fixes branch May 1, 2024 11:04
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.

2 participants