Skip to content

Conversation

@firewave
Copy link
Collaborator

I only aligned Settings::library and Settings::libraries where necessary. That will be improved in #4798.

void initvar_smartptr() { // #10237
Settings s;
s.libraries.emplace_back("std");
// TODO: test shuld probably not pass without library
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@chrchr-github Please have a look at this. It was added in 9d6e5c2.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That should probably be LOAD_LIB_2(s.library, "std.cfg"); Since the fix was for a FP, the test is ineffective, but still passes with or without the library

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The changes in that commit check for smart pointers. But without the library it shouldn't know what is a smart pointer.

Let me guess - those are tracked hard-coded in the source instead of using the library.

void uninitVarInheritClassInit() {
Settings s;
s.libraries.emplace_back("vcl");
// TODO: test should probably not pass without library
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@danmar Please have a look at this. It was added in 03445c0.

Copy link
Owner

Choose a reason for hiding this comment

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

Loading a library with the name "vcl" triggers a bit of hard coded handling in cppcheck. That is unfortunate. But I think the code is intentional at least.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but the test never triggers that code since it does pass without the library being loaded.

Copy link
Owner

Choose a reason for hiding this comment

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

hmm I believe I wanted to test that there would be no FP. I am confused why you could remove the vcl library and still don't get false positives.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please take a look. It doesn't have to be tackled within this PR though.

@danmar danmar merged commit 3ec4da0 into danmar:main Mar 2, 2023
@firewave firewave deleted the test-lib branch March 2, 2023 20:25
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