Skip to content

fix: split compilation database command strings with shell quoting rules - #10

Merged
cuinixam merged 2 commits into
cuinixam:mainfrom
avengineers:fix/compile-command-quoted-defines
Oct 12, 2026
Merged

cuinixam merged 2 commits into
cuinixam:mainfrom
avengineers:fix/compile-command-quoted-defines

Conversation

@code-steadfast

Copy link
Copy Markdown
Contributor

A compile_commands.json record may carry its flags as command, a single string the shell would have tokenized. Splitting it with str.split() kept the quotes, so an empty define written by CMake as -DMACRO="" reached libclang as the string literal "". A declaration starting with such a macro no longer parsed as a function and disappeared from docs output with no diagnostic.

shlex.split() is not a drop-in replacement: in POSIX mode it treats the backslashes in a Windows compiler path as escapes. Drive shlex directly with escaping disabled instead, so quotes are stripped and backslashes stay literal.

@cuinixam

cuinixam commented Oct 5, 2026

Copy link
Copy Markdown
Owner

I think escape = "" might break string defines on POSIX. If I'm right, CMake writes -DVERSION=\"1.0\", and this would turn it into -DVERSION=\1.0\ instead of -DVERSION="1.0". As far as I know, clang's own compile database reader only disables escapes on Windows. Would it make sense to do the same (if os.name == "nt": lexer.escape = "") and add a POSIX-only case -DVERSION=\"1.0\" expecting -DVERSION="1.0" (with the test cases marked windows-only)?

@code-steadfast
code-steadfast force-pushed the fix/compile-command-quoted-defines branch 2 times, most recently from cb076ef to 881a2d6 Compare October 5, 2026 19:54
@code-steadfast

Copy link
Copy Markdown
Contributor Author

You are right. Fixed it with if os.name == "nt" and added the POSIX test case for -DVERSION="1.0".

@cuinixam

cuinixam commented Oct 8, 2026

Copy link
Copy Markdown
Owner

I suggest we do not add the -isystem fix to this pull request. #9 already has the fix and it extends the compile options to support more than just -isystem option.

@cuinixam

Copy link
Copy Markdown
Owner

pr #9 was merged. you can remove the fix commit from this pr related to the -isystem option.

@code-steadfast
code-steadfast force-pushed the fix/compile-command-quoted-defines branch from f2efa65 to 881a2d6 Compare October 12, 2026 09:04

@cuinixam cuinixam left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

A compile_commands.json record may carry its flags as `command`, a single
string the shell would have tokenized. Splitting it with `str.split()` kept
the quotes, so an empty define written by CMake as `-DMACRO=""` reached
libclang as the string literal `""`. A declaration starting with such a macro
no longer parsed as a function and disappeared from `docs` output with no
diagnostic.

`shlex.split()` is not a drop-in replacement: in POSIX mode it treats the
backslashes in a Windows compiler path as escapes, so `-IC:\inc` arrives as
`-IC:inc`. Disabling escapes outright is wrong in the other direction, because
a POSIX string define is written `-DVERSION=\"1.0\"` and those backslashes
have to resolve. Drive `shlex` directly and disable escaping only on Windows,
so the backslash rule follows the shell that wrote the command.
@code-steadfast
code-steadfast force-pushed the fix/compile-command-quoted-defines branch from 881a2d6 to 299fd51 Compare October 12, 2026 09:38
@cuinixam
cuinixam merged commit d8632c9 into cuinixam:main Oct 12, 2026
7 checks passed
@code-steadfast
code-steadfast deleted the fix/compile-command-quoted-defines branch October 12, 2026 10:12
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.

3 participants