microsoft / vscode-cpptools

Official repository for the Microsoft C/C++ extension for VS Code.
Other
5.53k stars 1.56k forks source link

IntelliSense: option to provide a type/path of toolchain; read default macros from it #1293

Closed ivankravets closed 6 years ago

ivankravets commented 6 years ago

See related issue https://github.com/platformio/platformio-vscode-ide/issues/54

How to reproduce this issue?

  1. Please install https://marketplace.visualstudio.com/items?itemName=platformio.platformio-ide
  2. PIO Home > New Project > Arduino Due (Programming Port)
  3. Open src/main.cpp and paste
    
    #include <Arduino.h>

void setup() { pinMode(LED_BUILTIN, OUTPUT); }

void loop() { digitalWrite(LED_BUILTIN, HIGH); delay(1000); digitalWrite(LED_BUILTIN, LOW); delay(1000); }


4. 100% CPU, a lot of errors are guaranted.

--------

There were no issues with previous releases. A build of a project works very well, all INCLUDES are provided.
sean-mcmanus commented 6 years ago

It says Unable to open 'PIO Home" Could not start PIO Home server: Error: timeout.. Does it not work on Windows?

Are you saying this didn't repro with 0.14.2? I don't know of any recent changes we made that could have changed this.

ivankravets commented 6 years ago

It says Unable to open 'PIO Home" Could not start PIO Home server: Error: timeout.. Does it not work on Windows?

Just press Home icon again. Does it work now?

sean-mcmanus commented 6 years ago

No, it doesn't work after pressing the Home icon again -- it just shows a loading bar for 30 seconds and then the timeout message. However, I got it working on Linux...

sean-mcmanus commented 6 years ago

I don't get any 100% CPU for more than a few seconds. I see the error squiggles though...

ivankravets commented 6 years ago

No, it doesn't work after pressing the Home icon again -- it just shows a loading bar for 30 seconds and then the timeout message

Could you open Terminal via clicking on Terminal icon on bottom toolbar and type pio home. Does it work in your browser?

ivankravets commented 6 years ago

I don't get any 100% CPU for more than a few seconds. I see the error squiggles though...

On macOS it uses 100% all time, I see C/C++ extension in processes list. OK, and how about " I see the error squiggles though"? We should not see any errors here. If you press "Build" button or "Task > Build" you will see that there are no errors. What is more, you can type in Terminal pio run -v (verbose mode) and check that we use the same Includes/Flags for building and for c_cpp_properties.json.

It seems that CMSIS for Atmel SAM breaks your parser.

sean-mcmanus commented 6 years ago

The problem is that something is causing uint32_t to be re-defined to be "static". I'm still investigating. Our parser code hasn't changed since October 19th. Do you know which version of our extension this worked on?

On Windows, I get: pio home Traceback (most recent call last): File "main.py", line 12, in init main File "c:\users\user\.platformio\penv\lib\site-packages\click\__init__.py", line 18, in <module> from .core import Context, BaseCommand, Command, MultiCommand, Group, \ File "c:\users\user\.platformio\penv\lib\site-packages\click\core.py", line 8, in <module> from .types import convert_type, IntRange, BOOL File "c:\users\user\.platformio\penv\lib\site-packages\click\types.py", line 5, in <module> from ._compat import open_stream, text_type, filename_to_ui, \ File "c:\users\user\.platformio\penv\lib\site-packages\click\_compat.py", line 6, in <module> from weakref import WeakKeyDictionary File "c:\users\user\.platformio\python27\Lib\weakref.py", line 14, in <module> from _weakref import ( ImportError: cannot import name _remove_dead_weakref

How do I invoke it from the browser?

I'll try Mac.

sean-mcmanus commented 6 years ago

You're missing a compiler define, such as __GNUC__. Adding that define fixes the problem. This exact same issue has been hit by another user. It'd be nice if we made it easier to diagnose the problem. In the future, we may automatically use the defines set by the compiler we find and possibly add a path to the compiler to use for obtaining the defaults.

The problem location was /.platformio/packages/framework-arduinosam/system/sam/CMSIS/CMSIS/Include/core_cm3.h lines 82 to 98, where it checks the compiler defines for defining ASM and INLINE. Without those defined, it causes a cascade of failures for our parser.

ivankravets commented 6 years ago

Is __GNUC__ default macro for ALL GCC toolchains?

sean-mcmanus commented 6 years ago

__GNUC__ appears to be defined for gcc compilers, see https://stackoverflow.com/questions/17572519/what-means-cc-arm-iccarm-gnuc-tasking . I don't know what compilers your users are using, but in this particular case, having any of the 4 defines would have fixed the issue (or adding a #else clause in the header).

ivankravets commented 6 years ago

All our toolchains are GCC-based

ivankravets commented 6 years ago

I recommend reopening this issue. Would be good to have an option c_cpp_properties.json a marker of toolchain, where we will specify a full path to our gcc or g++ binary or just a type gcc. What do you think?

P.S: Yes, I can add temporary __GNUC__ for VSCode.

sean-mcmanus commented 6 years ago

Sure. Also, I'm not getting anything stuck with 100% CPU on Mac. I have witnessed our extension hitting some sort of 0% CPU deadlock, but I don't have a repro yet.

ivankravets commented 6 years ago

File "c:\users\user.platformio\python27\Lib\weakref.py", line 14, in from _weakref import ( ImportError: cannot import name _remove_dead_weakref

What is your Python version?

c:\Users\User\.platformio\python27\python --version
sean-mcmanus commented 6 years ago

The version under the .platformio path says 2.7.14, and my global version on my path is 2.7.6.

ivankravets commented 6 years ago

@sean-mcmanus Please upgrade local 2.7.6 (2013 year) to minimum 2.7.9 (2014). 2.7.6 has critical bugs and was replaced by 2.7.9. Or even better, install the latest 2.7.14

sean-mcmanus commented 6 years ago

Ah, yeah, my PATH had a location with the old 2.7.6 Python, so I just replaced that with the location of my existing 2.7.13 Python. However, pio home still gets the same error message, and running python --version from the terminal gives 2.7.14, so it appears to be using the platformio version of python and not the version on my system path.

ivankravets commented 6 years ago

Now please uninstall PlatformIO's 2.7.14 version (it is located in an isolated folder but should be visible in programs manager).

After that, please remove c:\Users\User\.platformio\python27 and c:\Users\User\.platformio\penv folder. Now all should work.

P.S: I fixed this issue with Python 2.7.6. With a next update, we will use own Python's version.

sean-mcmanus commented 6 years ago

Thanks -- pio home works on Windows now.

sean-mcmanus commented 6 years ago

Yeah, we're going to query the compiler we find for the system defines and add a setting to be able to set the compiler path to use (i.e. if the automatically found one is not the one desired).

BrandonShockley commented 6 years ago

How would I get rid of intellisense auto-completing the queried system defines (on the preview build)? I'm just experimenting with writing a small c program and don't need the massive list of defines every time I type a character, but I would still like autocomplete for local/global variables.

sean-mcmanus commented 6 years ago

@BrandonShockley There's currently no way to filter out stuff, such as system defines, but due to the fact that the system defines start with __ they should be appearing at the end of the list, right?

BrandonShockley commented 6 years ago

@sean-mcmanus They do appear at the end of the list, so I can live with it. It's just a minor annoyance. No worries.

sean-mcmanus commented 6 years ago

@ivankravets We've added a compilerPath, cStandard, and cppStandard properties with https://github.com/Microsoft/vscode-cpptools/releases/tag/v0.16.0-insiders2 (the official release is expected early next week). Let us know if there any problem with that in regards to the platformIO extension.

ivankravets commented 6 years ago

@sean-mcmanus Thank you so much! What is better, if we specify all compiler macros in configuration files to switch to new features with compilerPath?

We have a lot of issues with IntelliSense parser. It still claims that includes are missing but that is not true. What is interesting, that it works good on macOS. I attached a simple project which has issues on Windows 180216-133905-mbed-blink.zip

P.S: Just install our PIO IDE extension >=0.14 and open this project. Wait a few seconds to allow us to rebuild an index and generate data for your extension.

sean-mcmanus commented 6 years ago

I don't understand your question about "what is better". If the compilerPath is set to a clang/gcc-based compiler then we'll attempt to query it for the default include path and defines. On Linux/Mac, we'll automatically attempt use a guessed compiler even if it's not set. You can add defines too. The compiler can generate hundreds of defines, so we keep those hidden from the c_cpp_properties.json. We haven't added support for cl.exe or WSL compiler querying on Windows yet though. I'm trying that repro project now...

ivankravets commented 6 years ago

screen shot 2018-03-23 at 23 32 27

Could you provide in c_cpp_properties.json option to hide this warning? Or VSCode has better API? The problem is that we need a few seconds to generate c_cpp_properties.json and this extension claims that there are no includes :(

sean-mcmanus commented 6 years ago

Our code that implements that is in our open source https://github.com/Microsoft/vscode-cpptools/blob/master/Extension/src/LanguageServer/client.ts . Adding a user-facing setting for that seems too strange, but some sort of hidden setting or checking for something else might work, such as if isTagParsing(), but that assumes it takes long enough to parse the browse.path.

sean-mcmanus commented 6 years ago

PlatformIO isn't generating a c_cpp_properties.json with 0.15.0 or 0.16.0-insiders2. Is that supposed to happen? I'm on Windows, but I could try Linux if that's better. It says "cannot open source file "mbed.h"".

ivankravets commented 6 years ago

Oh, sorry. Could you point me what is wrong with our template?

sean-mcmanus commented 6 years ago

That looks good to me. It's odd that it isn't working with 0.15.0, since I don't recall any changes that could break it. With 0.16.0-insiders2 we will detect a missing compilerPath/cStandard/cppStandard and add it. I'll try the repro on Linux...

ivankravets commented 6 years ago

we will detect a missing compilerPath/cStandard/cppStandard and add it

There are no missing paths. If you go down to mbed.h you will find that header in toolchain's includes...

Nevertheless, VSCode does not see it under Windows. It is all OK with macOS. I hope with Linux too.

sean-mcmanus commented 6 years ago

Where is mbed.h? Do I need to do something to install a "toolchain"?

image

sean-mcmanus commented 6 years ago

Oh, it's working on Linux. I remember last time I tried platformIO on Windows I had an issue too, but I thought it got resolved. I got the files generated...checking the results...

ivankravets commented 6 years ago

Please install https://marketplace.visualstudio.com/items?itemName=platformio.platformio-ide

Then, restart VSCode. It will rebuild project index automatically.

sean-mcmanus commented 6 years ago

Yeah, I did install that -- I think last time I had some issue with my python version on Windows. I don't see any error form platformIO. Is there a way to get more logging? But if you guys aren't getting any complaints from Windows users then maybe my scenario isn't important and my Linux repro should be good enough to investigate the repro...

ivankravets commented 6 years ago

It seems that you have very old Python in a system. Could you try to uninstall all Python from a system and install the latest?

sean-mcmanus commented 6 years ago

On Linux with 0.16.0-insiders2 I'm not getting any errors. Oh, but you said the bug repro was with Windows. Yeah, I think I uninstalled the newer version of python -- I'll reinstall it.

One potential breaking issue with your c_cpp_properties.json generation is that the defines from compilerPath will overwrite any conflicting ones set in the defines, which may cause problems if the compilerPath is incorrect. We want users to set the compilerPath correctly and we weren't aware of a scenario it would be important for users to manually override those in the defines.

ivankravets commented 6 years ago

I think we will not provide compilerPath at all. We will use the current workflow when we populate all info to c_cpp_properties.json.

Will it work without compilerPath?

ivankravets commented 6 years ago

Yeah, I think I uninstalled the newer version of python -- I'll reinstall it.

try to remove HOME_DIR/.platformio folder after that. Restart VSCode.

sean-mcmanus commented 6 years ago

On Windows, we don't try to guess a default compiler so it shouldn't matter, but for Linux it will most likely default to the default clang or gcc path (even if the setting is cleared out), so if there are any conflicting defines generated by those executables, it will overwrite the values platformio generates in the defines, which could result in incorrect IntelliSense. Setting the compilerPath is expected to be able to fix this (assuming the compiler and args used are queryable without errors)... @bobbrow maybe we should provide some way to enable the user defines to overwrite the compilerPath ones?

ivankravets commented 6 years ago

We should have option to disable compilerPath, fir example setting it to null, undefined or false.

We can’t pass our compilers to this extension because you don’t spawn them correctly. That is why we don’t use your Debugger :( However, we want to use it.

Instead of correct subprocessing you place compiler/GDB to separate AppleScript which raises security error that binaries of toolchain are not signed.

We use NativeDebug temporary while you fix this issue.

sean-mcmanus commented 6 years ago

The debugger code is a different implementation. We just use the compilerPath to query for the compiler's include and defines, so we believe you should be able to set it correctly, assuming your compiler can be launched/queried like gcc/clang can be. How can we determine if the compilerPath can be set correctly?

Bypassing our compilerPath quering will not give correct results because your implementation doesn't appear to set all the compiler defines, such as __cplusplus.

sean-mcmanus commented 6 years ago

I still can't get platformio to work on Windows. I have 3.6.4 Python installed (at "F:/Program Files/Python"), but the version under .platformio/penv/Scripts is 2.7.13 (same as another version under .windows-build-tools/python27). I'm seeing "Error: Could not install 'framework-mbed' with version requirements '~4.50707.0' for your system 'windows_x86'." PlatformIO: IntelliSense Index failed: Error: Could not install 'framework-mbed' with version requirements '~4.50707.0' for your system 'windows_x86'. Please try this solution -> http://bit.ly/faq-package-manager. This doesn't appear to be a Python issue, because PIO Home works (the last time I had a python issue it didn't work).

UPDATE: I'm running the "platformio run --target upload", which seems to be working...

sean-mcmanus commented 6 years ago

Good news -- I got the platformio working via that command, and I reproed the bug with the missing include and I fixed it via setting the compilerPath to clang or /.platformio/packages/toolchain-gccarmnoneeabi/bin/arm-none-eabi-gcc.exe. However, after I do a reload window, platformio is recreating the c_cpp_properties.json and removing the compilerPath, causing the the bug to repro again. The preferred way to interact with our json files is to use a json library to update the document instead of doing a replace operation (which should also work after we switch to using settings.json).

image

sean-mcmanus commented 6 years ago

I'm thinking we should swap the priority such that user defines take precedence over the compilerPath ones. That would prevent users from experiencing a regression if they have the defines set already. However, the fact that platformIO clobbers the compilerPath on extension reload will prevent users from setting the compilerPath to fix issues, such as the missing stdio.h issue I saw on Windows.

ivankravets commented 6 years ago

because your implementation doesn't appear to set all the compiler defines, such as __cplusplus.

Thanks! Switched dump to C++ https://github.com/platformio/platformio-core/commit/622e4033c1b17a55ce7dc3cf0758571cfe96f0ae

So, now we have __cplusplus=201402L record in c_cpp_properties.json but problem still presists. You can open VSCode Terminal and type pio upgrade --dev. You will be upgraded to the latest GIT version and reload VSCode Window to rebuild c_cpp_properties.json.

screen shot 2018-03-24 at 13 02 57

ivankravets commented 6 years ago

We just use the compilerPath to query for the compiler's include and defines

Could you point me the source code which handles these tasks?

How can we determine if the compilerPath can be set correctly?

It will fail if set invalid path. For example, compilerPath: "/dev/null' or `compilerPath: "non-existing-path".

You need to be ready that someone will set an incorrect path to a compiler. In this case, you DO NOT need to use default compilerPath.

This feature is cool for the users of this extension. We already did this part of work on our side and we have all VALID information about project includes and defines. What is more, calling again GGC multiple times will consume a lot of CPU. No need to do that when we know when a project was modified.

The preferred way to interact with our json files is to use a json library to update the document instead of doing a replace operation

We have an opened issue in this case. However, it's not important. There is no need to edit c_cpp_properties.json manually. All build flags and includes should be configured with http://docs.platformio.org/en/latest/projectconf/section_env_build.html#build-flags

As only you make a change to the project's platformio.ini, we regenerate c_cpp_properties.json with updated values.

ivankravets commented 6 years ago

The debugger code is a different implementation.

I see a ton of issues on the Internet when people can't use this extension for embedded development as for this issue with "security". They use other IDEs of NativeDebug extension which works well.

In embedded, we don't have signed toolchains. Anyone can build own toolchain and use it.

Do you plan to resolve this issue finally? We would be glad to switch to your extension for debugging. Or, are you support the only desktop debugging?

Also, I remember the issue with incorrect cwd before you start GDB. You don't change CURRENT DIRECTORY before launching GDB even when a user asks you in launch.json via cwd option.

sean-mcmanus commented 6 years ago

I don't understand why you switched to dump C++. We use "-x c" or "-x c++" depending on the type of the translation unit. The args will also vary based on the -std flag (or the cStandard/cppStandard settings). Do you see the include errors go away when the compilerPath was set? That's what I observed. I'm not sure what else we're doing differently. I'll try with your newer version later today. Maybe we can change it so that an empty string compilerPath forces it add no system includes/defines.

The code that queries the compiler for args is closed source.

If someone sets a compilerPath that fails we fall back to the compile commands one, or a guessed one if it exists.

Are there issues tracking the debugger issues you mentioned? @pieandcakes and @WardenGnaw work on the debugger so I usually ignore those issues. I don't think the debugger issues would prevent our language server process from querying the compiler.