Fix #7458 (FP unusedStructMember with struct in union) #7844
Conversation
|
I am very sorry, but I am gonna be very blunt. Instead of playing with AI, you should rather engage with the existing contributions. Things to review keep piling up and take weeks or months to proceed (that was not the case in the past unless it was controversial - and all my drafts are not helping things at all). That is extremely bad with new/external contributors as it will frustrate them and potentially discourage them from further contribution (ultimately hurting the project - but is also true that some of the contributors never engage in feedback). It is also bad for the application as important fixes are being dragged out (like the major simplecpp issues the last release was shipped with). Also if a change is part of a series it is really hard to hold a thought on the next steps if things are processing as slowly as they do. Something really needs to change. |
I think we need to have more reviewers. I feel I am spending much time reviewing stuff. I was away at cppcon last week and now github says I have 97 PRs to review :-( |
The biggest bottleneck is with people which can review check-related stuff. As soon it goes beyond the most basic stuff or just programming logic I cannot be of any help.
You should not feel guilty/bad about going to cppcon at all. |
|
and well most PRs in my list are NOT about fixing defect tickets. There are way too many open defect tickets imho.. |
yes. unfortunately we can't provide much more resources right now but there has been a discussion to maybe hire one more full time developer.. then I hope he/she can take more reviews.. tommy and ludvig are great but they are full time students and not available constantly.. |
In my case I have an obscenely large backlog of local changes which I amounted over the years and try to reduce while I am waiting for the important stuff (suppressions, builddir, executor, performance, ...) to proceed. That adds to the overall growth especially if it turns out it needs more work and cannot be immediately be reviewed. At least I have most of the stuff in branches now and not just sitting in my filesystem. |
|
|
Re AI: It seems we are already getting AI-generated PRs, which are not disclosed as such and of poor quality. We should not encourage that kind of thing ourselves. |
I don't understand this critisism.. I am not blindly using the output from the AI. And I don't see these 2 PRs I created today as poor quality? |
If people already see PRs tagged as AI-generated, they might feel encouraged to create one themselves, and those might be of poor quality. E.g. I suspect this was LLM output: |
| if (scope.type == ScopeType::eStruct && scope.nestedIn && scope.nestedIn->type == ScopeType::eUnion) { | ||
| bool structMemberUsed = false; | ||
|
|
||
| for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) { |
There was a problem hiding this comment.
Shouldn't we go through the known variables and start from their nameToken() instead of searching all tokens?
There was a problem hiding this comment.
in c++ a variable can be used before it's declaration.. but well I guess we could look up the bodyStart of the parent struct/union/class scope and start from there..
This should be reviewed very carefully. I used AI to generate the fix.
I have not reviewed this yet => this is a draft for now