-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[WIP] Avoid overriding parsed C/C++ -std=* flag #3056
[WIP] Avoid overriding parsed C/C++ -std=* flag #3056
Conversation
dfa011a
to
a5c552a
Compare
ALE appends flags from {c,cpp}_{clang,gcc}_options after those found by parsing compile_commands.json or Makefile output. If -std=* flags are present in both the ALE flags and parsed flags, the last one present (i.e., ALE's -std=* flag) will determine the mode the compiler works in. This can result in errors showing up in vim but not in the actual build or vice-versa. For example, say you have foo.cpp: #include <type_traits> int main() { return std::is_same_v<float, int>; } If cpp_clang_options contains -std=c++17 and -std=c++14 is parsed from compile_commands.json, then ALE would end up running something like: clang++ -S -x c++ -fsyntax-only -std=c++14 -std=c++17 - < foo.cpp This would result in no errors showing up in Vim, but the actual build would fail with: <stdin>:3:14: error: no template named 'is_same_v' in namespace 'std'; did you mean 'is_same'? return std::is_same_v<float, int>; ~~~~~^~~~~~~~~ is_same /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/type_traits:872:61: note: 'is_same' declared here template <class _Tp, class _Up> struct _LIBCPP_TEMPLATE_VIS is_same : public false_type {}; ^ <stdin>:3:35: error: expected '(' for function-style cast or type construction return std::is_same_v<float, int>; ~~~~~~~~~~~~~~~~~~~~~~~~~~^ 2 errors generated. as the actual build would not have the -std=c++17 flag added by ALE. If cpp_clang_options contains -std=c++14 and -std=c++17 is parsed from compile_commands.json, then the opposite problem would occur. ALE would end up running something like: clang++ -S -x c++ -fsyntax-only -std=c++17 -std=c++14 - < foo.cpp and would show an error on line 3 of foo.cpp: [clang] No template named 'is_same_v' in namespace 'std'; did you mean 'is_same'? (fix available) The actual build, on the other hand, would succeed without any complaints. Removing -std=* from ALE's flags if it is already present in the parsed flags ensures that the wrong -std=* flag is not used. An alternative would have been to switch the order in which parsed flags and ALE flags were concatenated when producing the command to execute, but that could prevent a user from intentionally using ALE's flags to override some other flags, e.g. -W* flags to enable/disable warnings in a project whose flags are not under the developer's control. -std=* flags are also present in cuda/nvcc.vim, objc/clang.vim, objcpp/clang.vim, and vhdl/ghdl.vim, but none of those linters appear to parse compile_commands.json or `make` output.
a5c552a
to
fc6677d
Compare
Because the flag munging is implemented in Is there a better way to either structure my changes or to write the tests? |
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 |
Should this be closed in favor of a more general solution for #3276? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, we just need some tests.
I have developed a roundabout way to mock functions with Vader I've used in a number of test files now. Here's how you can do it.
Before:
...
let g:get_cflags_return_value = ''
runtime autoload/ale/c.vim
function! ale#c#GetCFlags(buffer, output) abort
return g:get_cflags_return_value
endfunction
After:
...
unlet! g:get_cflags_return_value
runtime autoload/ale/c.vim
You load the autoload Vim script that contains the function you want to mock, replace the function with your own definition, and reload the Vim script to load the original function again. You can use this in each of the test files for each linter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just gonna merge this now and add in the tests, so I can cross off another checkbox on the list.
Cheers! 🍻 |
Ah, so that's how you do it. I'll try to remember that for the next time I need to make changes. Thanks! |
ALE appends flags from
{c,cpp}_{clang,gcc}_options
after those found byparsing compile_commands.json or Makefile output. If
-std=*
flags arepresent in both the ALE flags and parsed flags, the last one present
(i.e., ALE's
-std=*
flag) will determine the mode the compiler works in.This can result in errors showing up in vim but not in the actual build
or vice-versa.
For example, say you have foo.cpp:
If
cpp_clang_options
contains-std=c++17
and-std=c++14
is parsed fromcompile_commands.json, then ALE would end up running something like:
This would result in no errors showing up in Vim, but the actual build
would fail with:
as the actual build would not have the
-std=c++17
flag added by ALE.If
cpp_clang_options
contains-std=c++14
and-std=c++17
is parsed fromcompile_commands.json, then the opposite problem would occur. ALE would
end up running something like:
and would show an error on line 3 of foo.cpp:
The actual build, on the other hand, would succeed without any
complaints.
Removing -std=* from ALE's flags if it is already present in the parsed
flags ensures that the wrong -std=* flag is not used.
An alternative would have been to switch the order in which parsed flags
and ALE flags were concatenated when producing the command to execute,
but that could prevent a user from intentionally using ALE's flags to
override some other flags, e.g. -W* flags to enable/disable warnings in
a project whose flags are not under the developer's control.
-std=* flags are also present in cuda/nvcc.vim, objc/clang.vim,
objcpp/clang.vim, and vhdl/ghdl.vim, but none of those linters appear to
parse compile_commands.json or
make
output.