jacobdufault / cquery

C/C++ language server supporting multi-million line code base, powered by libclang. Emacs, Vim, VSCode, and others with language server protocol support. Cross references, completion, diagnostics, semantic highlighting and more
MIT License
2.35k stars 163 forks source link

cquery always passes -fms-compatibility [on windows], which breaks cross-compiled projects #509

Closed HotelCalifornia closed 6 years ago

HotelCalifornia commented 6 years ago

https://github.com/cquery-project/cquery/blob/eadc53abe9af1e28d0a1abba40da7a16864de919/src/platform_win.cc#L132-L144

I'm working on a project that cross-compiles for Arm, and after a bit of hair-pulling, we were able to attribute our issues to the -fms-compatibility flag. Is there some way this could be made configurable?

My initial thoughts were to either:

The real issue may also be a confusion between the platform and the target

HotelCalifornia commented 6 years ago
Some output (with `-fms-compatibility`):
    clang++ -c -working-directory=C:/Users/usr/Desktop/driver-demo -target armv7a-xilinx-none-eabi -IC:/pros-toolchain/usr/arm-none-eabi/include/c++/7.2.1 -IC:/pros-toolchain/usr/arm-none-eabi/include/c++/7.2.1/thumb/v7-ar/fpv3/sof
tfp -IC:/pros-toolchain/usr/arm-none-eabi/include/c++/7.2.1/backward -IC:/pros-toolchain/usr/arm-none-eabi/include/c++/7.2.1/arm-none-eabi -IC:/pros-toolchain/usr/lib/gcc/arm-none-eabi/7.2.1/include -IC:/pros-toolchain/usr/lib/gcc/arm-none-eabi/7.2.1/include-fixed -IC:/p
ros-toolchain/usr/arm-none-eabi/include -iquote./include -iquote./include/./ -march=armv7-a -mfpu=neon-fp16 -mfloat-abi=softfp -Os -fshort-enums -std=gnu++14 C:\Users\usr\Desktop\driver-demo\src\initialize.cpp -resource-dir=C:/Users/Elliot/Downloads/cquery/build/rele
ase/LLVM-6.0.0-win64/lib/clang/6.0.0 -Wno-unknown-warning-option -fparse-all-comments -fms-extensions -fms-compatibility -fdelayed-template-parsing
In file included from C:\Users\usr\Desktop\driver-demo\src\initialize.cpp:1:
In file included from C:/Users/usr/Desktop/driver-demo\./include\api.h:24:
In file included from C:/pros-toolchain/usr/arm-none-eabi/include/c++/7.2.1\cmath:42:
C:/pros-toolchain/usr/arm-none-eabi/include/c++/7.2.1\bits/cpp_type_traits.h:176:25: error: use of undeclared identifier 'char16_t'
struct __is_integer
^
C:/pros-toolchain/usr/arm-none-eabi/include/c++/7.2.1\bits/cpp_type_traits.h:183:25: error: use of undeclared identifier 'char32_t'
struct __is_integer
^
In file included from C:\Users\usr\Desktop\driver-demo\src\initialize.cpp:1:
In file included from C:/Users/usr/Desktop/driver-demo\./include\api.h:24:
In file included from C:/pros-toolchain/usr/arm-none-eabi/include/c++/7.2.1\cmath:45:
In file included from C:/pros-toolchain/usr/arm-none-eabi/include\math.h:5:
C:/pros-toolchain/usr/arm-none-eabi/include\sys/reent.h:197:36: error: C++ requires a type specifier for all declarations
_READ_WRITE_RETURN_TYPE _EXFNPTR(_read, (struct _reent *, _PTR,
^
C:/pros-toolchain/usr/arm-none-eabi/include\sys/reent.h:197:44: error: C++ requires a type specifier for all declarations
_READ_WRITE_RETURN_TYPE _EXFNPTR(_read, (struct _reent *, _PTR,
^
C:/pros-toolchain/usr/arm-none-eabi/include\sys/reent.h:199:36: error: C++ requires a type specifier for all declarations
_READ_WRITE_RETURN_TYPE _EXFNPTR(_write, (struct _reent *, _PTR,
^
C:/pros-toolchain/usr/arm-none-eabi/include\sys/reent.h:199:45: error: C++ requires a type specifier for all declarations
_READ_WRITE_RETURN_TYPE _EXFNPTR(_write, (struct _reent *, _PTR,
^
C:/pros-toolchain/usr/arm-none-eabi/include\sys/reent.h:202:20: error: C++ requires a type specifier for all declarations
_fpos_t _EXFNPTR(_seek, (struct _reent *, _PTR, _fpos_t, int));
^
C:/pros-toolchain/usr/arm-none-eabi/include\sys/reent.h:202:28: error: C++ requires a type specifier for all declarations
_fpos_t _EXFNPTR(_seek, (struct _reent *, _PTR, _fpos_t, int));
^
C:/pros-toolchain/usr/arm-none-eabi/include\sys/reent.h:203:16: error: C++ requires a type specifier for all declarations
int _EXFNPTR(_close, (struct _reent *, _PTR));
^
C:/pros-toolchain/usr/arm-none-eabi/include\sys/reent.h:203:25: error: C++ requires a type specifier for all declarations
int _EXFNPTR(_close, (struct _reent *, _PTR));
^
C:/pros-toolchain/usr/arm-none-eabi/include\sys/reent.h:587:17: error: C++ requires a type specifier for all declarations
void _EXFNPTR(__cleanup, (struct _reent *));
^
C:/pros-toolchain/usr/arm-none-eabi/include\sys/reent.h:587:29: error: C++ requires a type specifier for all declarations
void _EXFNPTR(__cleanup, (struct _reent *));
^
In file included from C:\Users\usr\Desktop\driver-demo\src\initialize.cpp:1:
In file included from C:/Users/usr/Desktop/driver-demo\./include\api.h:29:
C:/pros-toolchain/usr/arm-none-eabi/include/c++/7.2.1\cstdlib:171:36: error: no matching function for call to 'ldiv'
div(long __i, long __j) { return ldiv(__i, __j); }
^~~~
C:/pros-toolchain/usr/arm-none-eabi/include\stdlib.h:103:15: note: candidate function not viable: requires 0 arguments, but 2 were provided
ldiv_t _EXFUN(ldiv,(long __numer, long __denom));
^
13 errors generated.
  

(none of these errors occur after removing the flag in question)

MaskRay commented 6 years ago

Can you verify the three MS compatibility options are indeed included in clang++ -E -x c++ - -v output?

You can disable the behavior by setting initialization option extraClangArgs to -fno-ms-compatibility -fno-delayed-template-parsing. They come last on the libclang command line. Can you please verify if this works and add this to https://github.com/cquery-project/cquery/wiki/Initialization-options ?

Cross compiling on Windows is an interesting case. What have you done to make include path correct? Can you also add the example (simplified) to https://github.com/cquery-project/cquery/wiki/Project-setup-examples ?

MaskRay commented 6 years ago

OT, where do the https://github.com/LazoCoder/Pokemon-Terminal pictures come from?

HotelCalifornia commented 6 years ago

Alright, it looks like those options are included in the output of clang++ -E -x c++ - -v.

checking on the init options now

ps: https://github.com/LazoCoder/Pokemon-Terminal#notes--credits

HotelCalifornia commented 6 years ago

so, I'm not sure whether I'm doing this right, but this is the output I get in the log file (working with rianneogi/ide-cquery):

arguments: cquery --log-file cquery_log.txt --language-server --init=\'{\"extraClangArguments\":[\"-fno-ms-compatibility\",\ \"-fno-delayed-template-parsing\"]}\'
Current dir: D:\Desktop\coading\sigbots\maketest
File verbosity level: 9
date       time         ( uptime  ) [ thread name/id]              file:line     v| 
2018-03-11 23:22:47.243 (   0.002s) [main thread  ]        loguru.hpp:2014     0| Logging to 'cquery_log.txt', mode: 'w', verbosity: 9
2018-03-11 23:22:47.243 (   0.002s) [main thread  ]        loguru.hpp:1736     0| atexit

(are those quotes supposed to be escaped like that?)

MaskRay commented 6 years ago

If you use a wrapper

#!/bin/zsh
exec ~/Dev/Util/cquery/build/release/bin/cquery --log-file=/tmp/cquery_log.txt '--init={"extraClangArguments":["-fno-ms-compatibility", "-fno-delayed-template-parsing"]}'

Better approach is to set initialization option in the language client, e.g. emacs-cquery#initialization-options

HotelCalifornia commented 6 years ago

Better approach is to set initialization option in the language client, e.g. emacs-cquery#initialization-options

Yeah, the problem is it doesn't appear the client I'm using supports arbitrary initialization options just yet, so I've had to hack it in (basically I wasn't sure whether the arguments are supposed to specified when the server starts or when the initializationOptions packet gets sent)

and of course, when I do send them on server startup, it looks as though the server is exiting instantly.

MaskRay commented 6 years ago

Updated https://github.com/cquery-project/cquery/wiki/Initialization-options

What's your editor?

HotelCalifornia commented 6 years ago

Using atom, with rianneogi/ide-cquery as the editor. I'm not really sure what's going wrong, other than it looks like the arguments are being passed incorrectly to the server (it seemed to be escaping all non-alpha characters, including spaces)

edjubuh commented 6 years ago

Also working with @HotelCalifornia on this project... I'm taking a look at the log output right now. I got the following message:

2018-03-12 00:29:13.536 ( 0.409s) [indexer2 ]clang_translation_unit.cc:103 libclang generic failure for C:/Users/me/Desktop/driver/src/file.c. Please try running the following, identify which flag causes the issue, and report a bug. cquery will then filter the flag for you automatically:

with the following command (added newlines for clarity)

clang -working-directory=C:\Users\me\Desktop\demo 
--target='armv7a-xilinx-none-eabi' 
-IC:/toolchain/usr/lib/gcc/arm-none-eabi/7.2.1/include 
-IC:/toolchain/usr/lib/gcc/arm-none-eabi/7.2.1/include-fixed 
-IC:/toolchain/usr/arm-none-eabi/include 
-iquote./include -iquote./include/./ 
-march=armv7-a -mfpu=neon-fp16 -mfloat-abi=softfp 
-Os -Wall 
-ffunction-sections -fdata-sections -fdiagnostics-color 
--std=gnu11 C:/Users/me/Desktop/demo/src/file.c 
-fno-ms-compatibility 
-fno-delayed-template-parsing 
-resource-dir=C:/Users/me/cquery/build/release/lib/LLVM-5.0.1-win64/lib/clang/5.0.1 
-Wno-unknown-warning-option 
-fparse-all-comments 
-fms-extensions 
-fms-compatibility 
-fdelayed-template-parsing 
-fsyntax-only

-fno-ms-compatibility -fno-delayed-template-parsing comes before -fms-extensions -fms-compatibility -fdelayed-template-parsing

MaskRay commented 6 years ago

Fixed in 0c695ad50d873a8910ed69ada58b8ea126291aeb

edjubuh commented 6 years ago

Err... that makes it

-fms-extensions
-fms-compatability
-fdelayed-template-parsing
clang
<rest of arguments>

Which'll fail since -fms-extensions and friends are before the actual command to execute. Might be OK to insert the first argument of the passed in arguments in the front of the vector, and the rest at the end with push_back?

MaskRay commented 6 years ago

Fixed.........

That is why I hate the goma clang compiler driver.......

Riatre commented 6 years ago

That works well as a workaround.

I think it is possible to make this automatic by introducing better support for -target, these MS compatibility flags is only needed for projects using clang-cl driver or i686-pc-windows-msvc-like target.

edjubuh commented 6 years ago

I'd be willing to take a whack at letting cquery add flags based on matching the target triple (or falling back to the system default). I'm not sure what the best way to determine the target triple is though. I'm not familiar with libclang. The most naive way I can think would be to scan through all the arguments and fall back to a default target triple based on Windows/POSIX as it is today, but that may not be robust against all the different scenarios of changing the target triple. Does libclang have some sort of call to help figure this info out based on the provided arguments? Would it be too expensive?

MaskRay commented 6 years ago
-fms-extensions
-fms-compatability
-fdelayed-template-parsing

Are the three options only added by clang or also by libclang, with --driver-mode=cl?

If it is too tricky, I'd like to remove the platform arguments

HotelCalifornia commented 6 years ago

also, re: what did we do to make the include path correct:

all it really took was running riszotto/scan-build (intercept-build make all) on our Makefile to generate a compile_commands.json. the include paths are effectively the same as what one would put into VSCode's .c_cpp_properties.json for intellisense support (which makes sense, as that is effectively a compilation database for their language server). I can update the wiki with some of that if you want.

MaskRay commented 6 years ago

pls update the wiki for posterity, thx!

jacobdufault commented 6 years ago

FYI: The -fms-extensions, f-ms-compatability, -fdelayed-template-parsing are needed because libclang will use the MSVC STL which relies on these extensions. If not present any indexing related to STL will be broken.

HotelCalifornia commented 6 years ago

FYI: The -fms-extensions, f-ms-compatability, -fdelayed-template-parsing are needed because libclang will use the MSVC STL which relies on these extensions. If not present any indexing related to STL will be broken.

That may be true, but certain projects (for example, the one we're working on) link to their own versions of the STL (through newlib, etc). In our case, our standard libraries come from the GNU Arm toolchain and the only thing we have to make sure of is that the include paths are correct (and that the ms extensions are not present)

MaskRay commented 6 years ago

FYI: The -fms-extensions, f-ms-compatability, -fdelayed-template-parsing are needed because libclang will use the MSVC STL which relies on these extensions. If not present any indexing related to STL will be broken.

Which of the following uses MSVC STL and will break if we remove -fms-extensions, f-ms-compatability, -fdelayed-template-parsing from platform clang arguments?

"arguments": ["clang.exe", "a.cc"]
"arguments": ["clang.exe", "--driver-mode=cl", "a.cc"]
"arguments": ["clang-cl.exe", "a.cc"]
jacobdufault commented 6 years ago

"arguments": ["clang.exe", "a.cc"]

at the very least this, I have not tried the others. I suspect all will break.

I have a feeling either we ship a separate stdlib and use that, or we add these flags on Wnidows so clang can parse the MSVC STL.

MaskRay commented 6 years ago

"arguments": ["clang.exe", "a.cc"]

at the very least this, I have not tried the others. I suspect all will break.

Then I agree we should keep fms-extensions, f-ms-compatability, -fdelayed-template-parsing

Riatre commented 6 years ago

because libclang will use the MSVC STL which relies on these extensions

Not with -target x86_64-pc-windows-gnu. With -target x86_64-pc-windows-gnu, clang tries to find a GCC installation and uses its STL instead, which is why I suggested appending these arguments only when working with -target *-pc-windows-msvc.

Clang shipped by release.llvm.org just defaults to x86_64-pc-windows-msvc, other clang distributions may have different default target triplet (For example, MSYS2 ships a libclang package which defaults to gnu). Keeping these flags breaks everything except a libclang in MSVC configuration, and vice versa.

Edit1: Format. Edit2: Elaborate.

edjubuh commented 6 years ago

I'm not sure if there's a way for us to extract a target triple from the provided arguments using libclang, but I did manage to find how LLVM represents triples (and a few methods that would be convenient for detecting MSVC) http://llvm.org/doxygen/classllvm_1_1Triple.html

On a slightly different note, why doesn't clang add -fms-extensions and friends automatically? Why wouldn't a compile_commands.json or .cquery file include them?

MaskRay commented 6 years ago

I want to remove the three options and let user add them explicitly. If I have time, I'll see if it can be fixed in libclang upstream. clang.exe is an emulation and using "extraClangArgs" or passing -fms-extensions -fms-compatability -fdelayed-template-parsing doesn't put huge burden on the user but makes the code much simpler.

MaskRay commented 6 years ago

Can someone sort out this intricacy (clang-cl.exe clang.exe) and contribute to https://github.com/cquery-project/cquery/wiki/Getting-started#windows ? The wiki is publicly editable