Skip to content

Conversation

@danmar
Copy link
Owner

@danmar danmar commented Sep 22, 2025

This should be reviewed extra carefully; I used ai to fix the ticket.

@danmar danmar requested a review from Copilot September 22, 2025 13:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a false positive in the unused struct member checker where members referenced in alignas attributes were incorrectly flagged as unused. The fix adds detection for member usage within alignas expressions.

  • Added logic to detect struct member references in alignas attributes
  • Added test case to verify the fix for issue #14075

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/testunusedvar.cpp Added test case structmember29 to verify alignas member usage detection
lib/checkunusedvar.cpp Added code to check for member references in alignas attributes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@danmar danmar marked this pull request as draft September 22, 2025 14:51
@sonarqubecloud
Copy link

@danmar
Copy link
Owner Author

danmar commented Sep 23, 2025

@firewave you reacted with a thumb down, do you see any problem here that I don't see?

@danmar
Copy link
Owner Author

danmar commented Sep 23, 2025

This should be reviewed extra carefully; I used ai to fix the ticket.

@firewave
Copy link
Collaborator

@firewave you reacted with a thumb down, do you see any problem here that I don't see?

Yes, I read the summary and I had to double-check that what it wrote actually matches what is contained in the PR. It turned out it is basically just the title and the comment blown up, so it adds nothing at all and is just a waste of time and resources.

This should be reviewed extra carefully; I used ai to fix the ticket.

In this case I would like to have that stated somewhere very clearly and I would even like to have such commits annotated with something which can be searched for in the commit log. As Cppcheck is a security-related tool which handles confidential data I think that might be worth pointing out.

FYI I am 100% and totally against any usage of it at all. I think it should never be allowed to write any code which will make it into production. I do see the potential in the area of testing (heck, fuzzing is still controversial in this project and it is essentially the same) but it is not worth any of the cost it implies - at least for me. And that is all I am gonna to say about it.

@danmar
Copy link
Owner Author

danmar commented Sep 23, 2025

Yes, I read the summary and I had to double-check that what it wrote actually matches what is contained in the PR.

yes the summary is pointless but sometimes copilot has good comments about the changes.. I invoked it to get a review.

I do see the potential in the area of testing

I agree it can be used for different things and we do not have to rule out all usages.

@danmar danmar added the AI-generated AI generated code, review extra carefully label Sep 23, 2025
@danmar danmar marked this pull request as ready for review September 23, 2025 12:56
@danmar danmar merged commit 376d7f1 into danmar:main Sep 23, 2025
51 checks passed
@danmar danmar deleted the fix-14075 branch November 1, 2025 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-generated AI generated code, review extra carefully

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants