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

Keep -iframework if present in parsed C/C++ flags #3057

Merged
merged 2 commits into from
Aug 19, 2020

Conversation

ts826848
Copy link
Contributor

-iframework appears to implicitly add some header search paths to the
set specified using the -isystem, -I, and -isysroot flags. This means
that leaving out -iframework can result in invalid errors being
displayed.

For example, given a CMake project with this CMakeLists.txt:

    project(test)
    set(Qt5_DIR "/usr/local/opt/qt5/lib/cmake/Qt5")
    find_package(Qt5 5.9 REQUIRED COMPONENTS Widgets)
    add_executable(foo foo.cpp)
    target_link_libraries(foo PRIVATE Qt5::Widgets)

and foo.cpp:

    #include <QWidget>
    int main() {}

If g:ale_c_parse_compile_commands is set, then ALE executes something
along these lines:

clang++ -S -x c++ -fsyntax-only -std=gnu++11 -isystem /usr/local/opt/qt5/lib/QtWidgets.framework/Headers - < foo.cpp

Resulting in this output:

In file included from <stdin>:1:
In file included from /usr/local/opt/qt5/lib/QtWidgets.framework/Headers/QWidget:1:
/usr/local/opt/qt5/lib/QtWidgets.framework/Headers/qwidget.h:43:10: fatal error: 'QtWidgets/qtwidgetsglobal.h' file not found
#include <QtWidgets/qtwidgetsglobal.h>
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Because the header is in a subdirectory of the path denoted by -isystem
(/usr/local/opt/qt5/lib/QtWidgets.framework/Headers/5.14.1/QtWidgets/qtwidgetsglobal.h),
clang fails to find it and exits.

If -iframework is included, ALE executes something along these lines:

clang++ -S -x c++ -fsyntax-only -std=gnu++11 -iframework /usr/local/opt/qt5/lib -isystem /usr/local/opt/qt5/lib/QtWidgets.framework/Headers - < foo.cpp

And compilation completes successfully.

Alex Wang added 2 commits March 17, 2020 18:47
-iframework appears to implicitly add some header search paths to the
set specified using the -isystem, -I, and -isysroot flags. This means
that leaving out -iframework can result in invalid errors being
displayed.

For example, given a CMake project with this CMakeLists.txt:

    project(test)
    set(Qt5_DIR "/usr/local/opt/qt5/lib/cmake/Qt5")
    find_package(Qt5 5.9 REQUIRED COMPONENTS Widgets)
    add_executable(foo foo.cpp)
    target_link_libraries(foo PRIVATE Qt5::Widgets)

and foo.cpp:

    #include <QWidget>
    int main() {}

If g:ale_c_parse_compile_commands is set, then ALE executes something
along these lines:

    clang++ -S -x c++ -fsyntax-only -std=gnu++11 -isystem /usr/local/opt/qt5/lib/QtWidgets.framework/Headers - < foo.cpp

Resulting in this output:

    In file included from <stdin>:1:
    In file included from /usr/local/opt/qt5/lib/QtWidgets.framework/Headers/QWidget:1:
    /usr/local/opt/qt5/lib/QtWidgets.framework/Headers/qwidget.h:43:10: fatal error: 'QtWidgets/qtwidgetsglobal.h' file not found
    #include <QtWidgets/qtwidgetsglobal.h>
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    1 error generated.

Because the header is in a subdirectory of the path denoted by -isystem
(/usr/local/opt/qt5/lib/QtWidgets.framework/Headers/5.14.1/QtWidgets/qtwidgetsglobal.h),
clang fails to find it and exits.

If -iframework is included, ALE executes something along these lines:

    clang++ -S -x c++ -fsyntax-only -std=gnu++11 -iframework /usr/local/opt/qt5/lib -isystem /usr/local/opt/qt5/lib/QtWidgets.framework/Headers - < foo.cpp

And compilation completes successfully.
@stale
Copy link

stale bot commented Aug 13, 2020

This pull request has been automatically marked as stale because it has not been updated recently. Make sure to write tests and document your changes. See :help ale-dev for information on writing tests.
If your pull request is good to merge, bother w0rp or another maintainer again, and get them to merge it.

@stale stale bot added the stale PRs/Issues no longer valid label Aug 13, 2020
@ts826848
Copy link
Contributor Author

I think this PR is ready to go? It has tests, though I'm not sure if additional documentation is required beyond that in the commit message.

@stale stale bot removed the stale PRs/Issues no longer valid label Aug 13, 2020
@w0rp w0rp merged commit 4d42ebc into dense-analysis:master Aug 19, 2020
@w0rp
Copy link
Member

w0rp commented Aug 19, 2020

Cheers! 🍻

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.

2 participants