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

A better wrapper is needed #3

Closed
adah1972 opened this issue Jan 23, 2024 · 8 comments
Closed

A better wrapper is needed #3

adah1972 opened this issue Jan 23, 2024 · 8 comments

Comments

@adah1972
Copy link
Contributor

Running cppsafe directly on a file caused error messages, but since the integration tests seems to be OK, I checked and found that you already have scripts that gather the include directories. Users need them, not just the tests.

So I suggest you provide a wrapper script that can gather the include directories. Otherwise a lot of people might be frustrated.

@qqiangwu
Copy link
Owner

The integration tests only collect standard include directories.

For a real project with thirdparty dependencies, cppsafe should be used with compile_commands.json:

cppsafe -p <directory_of_compile_commands.json> file1 file2 --Wno-lifetime-null --Wno-lifetime-post

Later I will add a wrapper and more document for easy use

@adah1972
Copy link
Contributor Author

The integration tests only collect standard include directories.

This is exactly what I care about.

The problem is not about custom directories, but that cppsafe cannot use the standard include directories (on Mac). I am not sure about Linux.

Even with compile_commands.json, the problem should still surface.

@adah1972
Copy link
Contributor Author

Even with compile_commands.json, the problem should still surface.

Reproduced and confirmed. Using either the default compiler (Apple Clang 14) or Homebrew GCC 13, I still get error messages like:

$ ../cppsafe/build/cppsafe -p build test.cpp
In file included from /Users/yongwei/Programming/test/test.cpp:1:
In file included from /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/thread:86:
In file included from /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/__debug:14:
In file included from /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/iosfwd:98:
In file included from /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/__mbstate_t.h:29:
In file included from /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/wchar.h:123:
/Library/Developer/CommandLineTools/SDKs/MacOSX13.3.sdk/usr/include/wchar.h:89:10: fatal error: 'stdarg.h' file not found
   89 | #include <stdarg.h>
      |          ^~~~~~~~~~
1 error generated.
Error while processing /Users/yongwei/Programming/test/test.cpp.

I.e. cppsafe cannot find the compiler built-in header files.

Everything is OK if I use a simple wrapper script that adds the include directories.

@qqiangwu
Copy link
Owner

I'm thinking about how to solve the problem. Cppsafe is released standalone, if you put it with llvm binaries, it can found c includes provided by clang (That's why clang-tidy cannot be released alone and must be bundled with llvm)

cppsafe(just like other clang based tools, e.g. clang, clang-tidy) will infer c include directory as $(which cppsafe)/../lib/clang/17/include.

for compile_comands.json, maybe you can try:

if(CMAKE_EXPORT_COMPILE_COMMANDS)
    set(CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES ${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES})
endif()

put it in the bottom of your CMakeList.txt.

to get full include paths.

there is also a workaround for apple clang:

CPLUS_INCLUDE_PATH=/opt/homebrew/opt/llvm/lib/clang/17/include cppsafe example.cpp

or

CPLUS_INCLUDE_PATH=/Library/Developer/CommandLineTools/usr/lib/clang/13.1.6/include cppsafe example.cpp

you can use cppsafe <your-cpp-file> -- -v to see the infered includes.


for gcc, it seems that all clang-based tools inter a wrong path.


simply embed CPLUS_INCLUDE_PATH=/Library/Developer/CommandLineTools/usr/lib/clang/13.1.6/include inside cppsafe doesn't work since users might add -nostdinc in compile_commands.json


maybe I should bundle clang/xxx/include headers with cppsafe.

@qqiangwu
Copy link
Owner

copy cppsafe to /opt/homebrew/opt/llvm/bin/ solves the problem too.

@adah1972
Copy link
Contributor Author

adah1972 commented Jan 23, 2024

Setting CPLUS_INCLUDE_PATH as you did above does not work for me. Generating fuller include paths like your test script works. Copying cppsafe to the LLVM directory works as well.

@qqiangwu
Copy link
Owner

Fixed. Now the system header detection is incorporated in cppsafe itself.

@adah1972
Copy link
Contributor Author

Fixed. Now the system header detection is incorporated in cppsafe itself.

Fantastic! Thank you!

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

No branches or pull requests

2 participants