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

Fix error using Clang option '-stdlib=libc++' #3808

Merged

Conversation

TotalNormal2
Copy link
Contributor

Using Clang option "-stdlib=libc++" failed with:
cppcheck: error: unrecognized command line option: "--std=libc++"

Using Clang option "-stdlib=libc++" failed with:
cppcheck: error: unrecognized command line option: "--std=libc++"
# BUT NOT:
# * -std c99
# * -stdlib=libc++
std_regex = re.compile("-?-std[ |=].*")
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, it's a nice catch!

[ |=] matches any of the three characters between square brackets, because | is not a separator in this context. Also, the build_action.analyzer_option doesn't contain a space character if it starts with --std, because command line arguments are already split here.

What do you think of this regex?

Suggested change
std_regex = re.compile("-?-std[ |=].*")
std_regex = re.compile(r"-?-std\b.*")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for looking into this. You're right about the split and |.

I had to extend my regex knowledge first to understand your nice idea. I did some testing using an online regex evaluator (selected Python) and found:

  • the pattern needs not to be at the start (solvable with ^)
  • if a - (or any other special char) is following std anything afterwards will also be accepted

Example: ThisCanBeAnything-std-AlsoHere=AndMatch

I've tested other solutions like "^-(-std$)|(-?std=.*)" or "^--?std($)|(=.*)" (the latter still accepts -std c99) to avoid any undesired patterns. I compared them to "^-?-std\b.*" which requires only up to 10 steps to evaluate a string while the others require up to 30. On the downside it still accepts -std c99 or -std-Anything... but I vote for speed:

Suggested change
std_regex = re.compile("-?-std[ |=].*")
std_regex = re.compile(r"^-?-std\b.*")

Looking now into interesting_option (got that | from there) I'm wondering if it should be interesting_option = re.compile(r"^-[IUD].*") ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, it's a very nice analysis of the issue, it's good that we consider all corner cases. As a next step what do you think of this?

Suggested change
std_regex = re.compile("-?-std[ |=].*")
std_regex = re.compile(r"-?-std[\b=].*")

It means that after "std" it is either the end of the word or = follows. The ^is not necessarily needed, because function match() in line 109 finds the pattern at the beginning of the string.

Yes, the interesting_option should really be fixed to IUD instead of I|U|D.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems [\b] is matching backspace and [\b=] is matching backspace or = but not word boundary.
You're right about ^ not being required using match().

I've tested the following strings (1-3 are the correct ones) in an online-evaluator...

  1. --std
  2. -std=xxx
  3. --std=xxx
  4. -std
  5. -stdlib=libc++
  6. --stdä=hnlich
  7. --std-not
  8. -stdio=xxx
  9. --stdio=xxx
  10. -std-99
  11. --std-C99
  12. -std-99=xxx
  13. --std-C99=xxx

...with these expressions (matching strings mentioned)

  1. -(-std$|-?std=.*) matching: 1,2,3
  2. -?-std($|=.*) matching: 1,2,3,4
  3. -?-std(\b|=.*) matching: 1,2,3,4,(6),7,10,11,12,13 ; (6) had different results on different evaluators
  4. -?-std[$=].* matching: 2,3
  5. -?-std[\b=].* matching: 2,3

Only expression 1 has zero false-pos and false-neg.
I prefer avoiding \b because using - within a name is not complete out of the world but it's not a boundary (see 11-13).

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, thanks, it's a very nice research and test cases :)
I start the tests and we can merge it. Thank you, well done!

@bruntib bruntib merged commit 09f7c1b into Ericsson:master Jan 12, 2023
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.

None yet

2 participants