refs #10692 - added command-line option `--cpp-header-probe` to probe headers and extension-less files for Emacs C++ marker #6324

firewave · 2024-04-22T13:40:08Z

No description provided.

firewave · 2024-04-22T13:47:56Z

Still work in progress...

The standard library and LLVM are specifying this among others. Just running it on /usr/include will encounter lots of actual usages.

This will be disabled by default but I am considering enabling it by default in the release afterwards.

Will also add a commit which adds it to our headers. Will most likely do it in a separate PR.

firewave · 2024-04-22T15:37:46Z

Requires #6325 to be merged first.

After #6211 has landed (if even possible) and we finally upgraded to PCRE2 (see https://trac.cppcheck.net/ticket/10610) we should replace most of the messy parsing code with a regular expression.

chrchr-github · 2024-04-22T16:26:06Z

What is the canonical use case for this?

firewave · 2024-04-22T16:37:07Z

What is the canonical use case for this?

That is described in the ticket. It is mainly for IDE integrations. When you open a header by default it will be treated as C file and when it is actually C++ code it will error out. So you need to specify --language=c++ so the analysis is performed. But that would also force (and override the existing Path::identify() calls) all source files to be treated as C++ even though they might be C code.

For the CLI usage this should not have any impact at all since the probing would only be performed if you explicitly pass a file with the *.h or no extension (like the C++ standard headers). I probably should change the command-line option to --cpp-header-probe (or similar) indicate that.

This is based on the implementation of the CLion plugin which only has a single global configuration for the options being provided.

On a side note it already helped to detect two issues hard to spot issues. 😀

chrchr-github · 2024-04-22T16:43:39Z

What is the canonical use case for this?

That is described in the ticket. [...]

Thanks for the explanation. IDEs were mentioned in the ticket, but I didn't make the connection to IDE integration.

firewave · 2024-04-22T16:47:31Z

Thanks for the explanation. IDEs were mentioned in the ticket, but I didn't make the connection to IDE integration.

I just realized there is no upstream ticket for this. It is mentioned in the known issues though: https://github.com/johnthagen/clion-cppcheck?tab=readme-ov-file#analyzing-header-files.

firewave · 2024-04-22T17:00:36Z

I also encountered some performance issues with some headers even when I disable ValueFlow and all checks so it is probably related to the tokenizing or preprocessing. I have not looked into that yet.

firewave · 2024-04-22T17:07:50Z

I also encountered some performance issues with some headers even when I disable ValueFlow and all checks so it is probably related to the tokenizing or preprocessing. I have not looked into that yet.

Turns out this only occurs in a debug build so there is no issue.

firewave · 2024-04-24T18:10:49Z

To test this with existing headers I patched out some code for speed, enabled the logging in hasEmacsCppMarker() and ran find /usr/include -type f | xargs -n 1 bin/cppcheck -D__GNUC__ --cpp-header-probe -q 2> /dev/null (FYI that found 2812 markers).

It seems this uncovered bad markers in the LLVM headers which I reported upstream: llvm/llvm-project#89965.

firewave · 2024-04-24T18:58:50Z

This does not currently handle markers in /* */ comments. I will add support for that in a follow-up PR.

firewave · 2024-04-26T12:14:20Z

lib/tokenize.cpp

@@ -8052,7 +8052,8 @@ void Tokenizer::unmatchedToken(const Token *tok) const
 void Tokenizer::syntaxErrorC(const Token *tok, const std::string &what) const
 {
    printDebugOutput(0);
-    throw InternalError(tok, "Code '"+what+"' is invalid C code. Use --std or --language to configure the language.", InternalError::SYNTAX);
+    // TODO
+    throw InternalError(tok, "Code '"+what+"' is invalid C code.\nUse --std or --language to configure the language or --cpp-header-probe to detect C++ headers via the Emacs marker.", InternalError::SYNTAX);


This obviously needs to be reworded but I couldn't come up with something better yet.

I also moved the references to the options to the verbose part of the message. That should be done for a lot more messages but will be tackled at a future date.

I already re-worded it before I made it ready for review.

lib/path.cpp

lib/settings.h

firewave · 2024-05-23T09:18:12Z

lib/path.cpp

+    findAndReplace(marker, "Mode:", "");
+    marker = trim(marker);
+    if (marker == "C++" || marker == "c++") {
+        // NOLINTNEXTLINE(readability-simplify-boolean-expr) - TODO: FP


Upstream issue: llvm/llvm-project#93157.

cli/cmdlineparser.cpp

… headers and extension-less files for Emacs C++ marker

firewave force-pushed the cpp-probe branch from e3a42cf to 73a5bb4 Compare April 22, 2024 15:33

firewave mentioned this pull request Apr 22, 2024

TokenList: avoid Path::identify() being called with empty filename from determineCppC() #6326

Merged

firewave mentioned this pull request Apr 22, 2024

extracted trim() and findAndReplace() to utils.{cpp|h} #6325

Merged

firewave changed the title ~~fixed #10692 - added command-line option --cpp-probe to probe headers and extension-less files for Emacs marker~~ fixed #10692 - added command-line option --cpp-header-probe to probe headers and extension-less files for Emacs marker Apr 22, 2024

firewave force-pushed the cpp-probe branch from a9b01c3 to 36c6877 Compare April 22, 2024 16:59

firewave changed the title ~~fixed #10692 - added command-line option --cpp-header-probe to probe headers and extension-less files for Emacs marker~~ fixed #10692 - added command-line option --cpp-header-probe to probe headers and extension-less files for Emacs C++ marker Apr 22, 2024

firewave force-pushed the cpp-probe branch 7 times, most recently from 36708cf to 78807ce Compare April 24, 2024 08:47

firewave changed the title ~~fixed #10692 - added command-line option --cpp-header-probe to probe headers and extension-less files for Emacs C++ marker~~ refs #10692 - added command-line option --cpp-header-probe to probe headers and extension-less files for Emacs C++ marker Apr 24, 2024

firewave force-pushed the cpp-probe branch 6 times, most recently from 799b1aa to 56f257d Compare April 25, 2024 21:08

firewave force-pushed the cpp-probe branch 4 times, most recently from 0439266 to 3cc9090 Compare April 26, 2024 09:22

firewave commented Apr 26, 2024

View reviewed changes

firewave force-pushed the cpp-probe branch from 3cc9090 to 6125c8b Compare April 26, 2024 12:24

firewave force-pushed the cpp-probe branch from 6125c8b to 9b4cd7b Compare May 7, 2024 11:53

firewave marked this pull request as ready for review May 7, 2024 11:53

danmar reviewed May 21, 2024

View reviewed changes

lib/path.cpp Show resolved Hide resolved

danmar reviewed May 22, 2024

View reviewed changes

lib/settings.h Show resolved Hide resolved

firewave commented May 23, 2024

View reviewed changes

danmar reviewed May 23, 2024

View reviewed changes

cli/cmdlineparser.cpp Show resolved Hide resolved

danmar approved these changes May 23, 2024

View reviewed changes

firewave added 2 commits May 23, 2024 18:22

refs #10692 - added command-line option --cpp-header-probe to probe…

aaab7aa

… headers and extension-less files for Emacs C++ marker

path.cpp: mitigated readability-simplify-boolean-expr false positive

a9ad6b6

firewave force-pushed the cpp-probe branch from ec75f29 to a9ad6b6 Compare May 23, 2024 16:25

firewave merged commit 2263d30 into danmar:main May 23, 2024
63 checks passed

firewave deleted the cpp-probe branch May 23, 2024 18:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refs #10692 - added command-line option `--cpp-header-probe` to probe headers and extension-less files for Emacs C++ marker #6324

refs #10692 - added command-line option `--cpp-header-probe` to probe headers and extension-less files for Emacs C++ marker #6324

firewave commented Apr 22, 2024

firewave commented Apr 22, 2024

firewave commented Apr 22, 2024

chrchr-github commented Apr 22, 2024

firewave commented Apr 22, 2024

chrchr-github commented Apr 22, 2024

firewave commented Apr 22, 2024 •

edited

Loading

firewave commented Apr 22, 2024

firewave commented Apr 22, 2024

firewave commented Apr 24, 2024 •

edited

Loading

firewave commented Apr 24, 2024

firewave Apr 26, 2024

firewave May 21, 2024

firewave May 23, 2024 •

edited

Loading

refs #10692 - added command-line option --cpp-header-probe to probe headers and extension-less files for Emacs C++ marker #6324

refs #10692 - added command-line option --cpp-header-probe to probe headers and extension-less files for Emacs C++ marker #6324

Conversation

firewave commented Apr 22, 2024

firewave commented Apr 22, 2024

firewave commented Apr 22, 2024

chrchr-github commented Apr 22, 2024

firewave commented Apr 22, 2024

chrchr-github commented Apr 22, 2024

firewave commented Apr 22, 2024 • edited Loading

firewave commented Apr 22, 2024

firewave commented Apr 22, 2024

firewave commented Apr 24, 2024 • edited Loading

firewave commented Apr 24, 2024

firewave Apr 26, 2024

Choose a reason for hiding this comment

firewave May 21, 2024

Choose a reason for hiding this comment

firewave May 23, 2024 • edited Loading

Choose a reason for hiding this comment

refs #10692 - added command-line option `--cpp-header-probe` to probe headers and extension-less files for Emacs C++ marker #6324

refs #10692 - added command-line option `--cpp-header-probe` to probe headers and extension-less files for Emacs C++ marker #6324

firewave commented Apr 22, 2024 •

edited

Loading

firewave commented Apr 24, 2024 •

edited

Loading

firewave May 23, 2024 •

edited

Loading