Skip to content

add missing #line directive to misra.py - #8879

Open
driftregion wants to merge 1 commit into
cppcheck-opensource:mainfrom
driftregion:misra_line
Open

driftregion wants to merge 1 commit into
cppcheck-opensource:mainfrom
driftregion:misra_line

Conversation

@driftregion

Copy link
Copy Markdown

Hello cppcheck maintainer. Thank you for your work.

The #line preprocessor directive (standard since C90) was missing from misra.py, leading to false positives in my analysis. I have added it here.

See:
https://en.cppreference.com/c/preprocessor/line

Comment thread addons/misra.py
dir = mo.group(1)
if dir not in ['define', 'elif', 'else', 'endif', 'error', 'if', 'ifdef', 'ifndef', 'include',
'pragma', 'undef', 'warning']:
'pragma', 'undef', 'warning', 'line']:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is an AI review. Take it with a grain of salt and feel free to reject it by resolving the comment.

The fix looks right to me: #line is a standard directive, and with this change the 20.13 false positive is gone.

Could you add a regression test? One option that doesn't shift any line numbers in addons/test/misra/misra-test.c is to replace the empty line after #else1 // 20.13 (line 1946) with:

#line 1947

I tried it with the same commands CI uses (cppcheck --dump -DDUMMY --suppress=uninitvar --inline-suppr misra/misra-test.c --std=c89 --platform=unix64 + misra.py -verify). Without your fix it reports Not expected: misra/misra-test.c:1946 20.13, and with your fix it passes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi Daniel, thanks for your work maintaining cppcheck. I had a look at the AI reviews on #8879, #8876, and cppcheck-opensource/simplecpp#704 just now. They seem reasonable. Before I go about addressing them, I'd like to request your human opinion on whether you endorse the AI-proposed changes. This statement makes it clear that the review was AI-generated (great idea btw), but leaves me with some confusion about whether the human in the loop endorses its content:

This is an AI review. Take it with a grain of salt and feel free to reject it by resolving the comment.

Thanks!

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