search

jbenden / vscode-c-cpp-flylint

A VS Code extension for advanced, modern, static analysis of C/C++ that supports a number of back-end analyzer programs.
MIT License
152 stars 29 forks source link

Clang path bad "/" prefix on Windows #179

Closed dzid26 closed 1 year ago

dzid26 commented 1 year ago

https://github.com/jbenden/vscode-c-cpp-flylint/issues/144
I noticed again that my clang is ignoring included paths and failing to find headers because the drive letter is prefixed with /.

C:\msys64\usr\bin\clang.EXE -fsyntax-only -fno-color-diagnostics -fno-caret-diagnostics -fno-diagnostics-show-option -fdiagnostics-show-category=name -ferror-limit=200 --std=c99 -fblocks -I /c:/Users/dzidm/Repos/firmware/src/ -x c c:\Users\dzidm\Repos\firmware\src\control_api.c

Is this line needed? https://github.com/jbenden/vscode-c-cpp-flylint/blob/a1bac0a7c1b9cf3bb913297599ad0b5442cae498/server/src/server.ts#L288-L291 When I comment it out, the / disappears and clang works correctly.

jbenden commented 1 year ago

Hi,

The problems run deep!

The problem with Windows (OK, there ARE more!) is that many environments could be used with the extension. These environments all use different conventions for the file paths. The environments are: native, cygwin, and msys. So native wants drive letters first, Cygwin and MSYS want slashes at the beginning and use different methods of accessing a specific drive letter. Plus, all file names passed to the tools need to be in correct form.

But, the extension does not have a way of knowing in advance.

Thoughts, comments, and suggestions welcome, -Joe

dzid26 commented 1 year ago

I see.

So:

  1. we could not add / and be compatible with native and MSYS2 - but maybe not with Cygwin, or
  2. we could fix the UNIX path by removing the colon and being compatible with MSYS2 and Cygwin, but not native, or
  3. we could remove the drive letter altogether and be compatible with everything, but the drive would implicit.
jbenden commented 1 year ago

How about a configuration switch to force the use of UNIX paths, which defaults to off? This could solve the issue you face, while also leaving an escape-hatch for anyone depending on the current functionality?

Alternatively, it could be removed to see if anyone actually complains... sigh...

Thoughts, comments, and suggestions are always welcomed, -Joe

dzid26 commented 1 year ago

I am not sure if you are leaning toward having UNIX or Windows style. I tested that for me both styles work.

The problem is that currently, we have something that is neither Unix nor Windows style: -I /c:/Users/...
Note the colon :.

jbenden commented 1 year ago

Actually, thinking about this more... In truth, there must be a bug in absolute path detection - when a drive letter and colon exists, this is an absolute path...

update: doesn't apply in this case.

jbenden commented 1 year ago

Would you mind testing the PR #183 ? It hopefully resolves this problem...

dzid26 commented 1 year ago

Ok, thank you. Yes, it works for me now. The colon : is gone.

executing:  C:\msys64\usr\bin\clang.EXE -fsyntax-only-DHSE_VALUE=16000000 -I /c/Users/dzidm/Repos/firmware/src/  -x c c:\Users\dzidm\Repos\firmware\src\spi.c

Similarly as before the includes follow Unix convention and the source file at the end is Windows style. For my MSYS it doesn't matter though.

jbenden commented 1 year ago

Hi, would you mind testing again? I made a few changes to process all filenames used; so hopefully all parameters to clang.exe are now in Unix convention. PS: Ignore CICD failing on the PR; GitHub is having connection reset problems...

dzid26 commented 1 year ago

Yikes, Cppcheck didn't like it. 

executing:  C:\Program Files\Cppcheck\cppcheck.EXE --inline-suppr --addon=misra.json --addon=naming.json 
-I c:/Users/dzidm/Repos/firmware/src/APP/ -I c:/Users/dzidm/Repos/firmware/src/BSP/ - --cppcheck-build-dir=.vscode 
/c/Users/dzidm/Repos/firmware/src/BSP/tle5012.c
[
  'cppcheck: error: could not find or open any of the paths given.',
  ''

Lizard also ignored Unix path. Quietly, with no error.

jbenden commented 1 year ago

Darn! Ok, so I changed it to produce Windows-only paths (but with Unix path terminator). Maybe it'll work better now? Thanks so much!

dzid26 commented 1 year ago

Yes, both dcb21d0 (c:/Users/..) and [f2d97f4 ] (/Users/..) worked for me. If we decide on skipping the drive letter for includes, we should probably skip it for source file. Otherwise, I imagine, cygwin [clang] will fail. 

executing:  C:\msys64\usr\bin\clang.EXE -fsyntax-only -fno-color-diagnostics -fno-caret-diagnostics -DHSE_VALUE=16000
-I /Users/dzidm/Repos/firmware/src/BSP -x c c:/Users/dzidm/Repos/MKS-SERVO42B/firmware/src/BSP/tle5012.c
executing:  C:\Program Files\Cppcheck\cppcheck.EXE --inline-suppr --enable=warning,--addon=misra.json --
addon=naming.json -I /Users/dzidm/Repos/firmware/src/APP DHSE_VALUE=16000000 --language=c --platform=native --
template="{file}  {line}  {severity} {id}: {message}" --cppcheck-build-dir=.vscode 
c:/Users/dzidm/Repos/firmware/src/BSP/tle5012.c
[
  'Checking c:\\Users\\dzidm\\Repos\\firmware\\src\\BSP\\tle5012.c ...',
  ''

I must say though, no drive letter looks weird to the eye. I kinda liked pure windows paths the most from dcb21d0. :)

dzid26 commented 1 year ago

I just noticed that I am not getting code squiggles. with any of these.
Last one squiggles were working was 78c48ffe0a27111ba731e2f930b99d56cde8e566.

jbenden commented 1 year ago

It should now always use Windows paths, with drive letters. I additionally added unit-tests to catch more edge cases in conversion.

If we can get all file names to be proper and actual tools to run with them (ie: cppcheck and clang); I'll be able to see why squiggles are not working. I suspect I'm storing the original filename instead of the normalized version.

Thanks so much! -Joe