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

Nested namespaces (or C++1z) support #699

Closed
mdziekon opened this issue May 7, 2017 · 14 comments
Closed

Nested namespaces (or C++1z) support #699

mdziekon opened this issue May 7, 2017 · 14 comments
Labels
Feature Request fixed Check the Milestone for the release in which the fix is or will be available. Language Service

Comments

@mdziekon
Copy link

mdziekon commented May 7, 2017

Hi, I'm currently trying to configure VSCode to "support" nested namespaces in C++ code. My code builds just fine when using clang++ (ver. 3.8.0) with flag -std=c++1z, so the only thing I need is working code completion inside VSC itself.

Is there a way to enable VSCode to use that "experimental" standard by switching some kind of configuration flag, or is this currently out of scope?

@sean-mcmanus sean-mcmanus added Language Service more info needed The issue report is not actionable in its current state labels May 8, 2017
@sean-mcmanus
Copy link
Contributor

What type of support are you looking for from VS Code? What feature is not working correctly? Are you referring to C++11 inline namespaces?

@mdziekon
Copy link
Author

mdziekon commented May 8, 2017

Consider the following code piece:

namespace my::nested::space {
    class some_class {
        ...
    };
}

In C++17, this is a valid definition of a nested namespace.

Currently, VSC with cpptools will throw the following error at the first line of this snippet:

severity: 'Error'
message: 'qualified name is not allowed'

... and of course I'm not able to autocomplete this namespace, nor the class name.


I guess VSC should be able to properly recognize this as a valid syntax, and properly detect some_class as a member of my::nested::space when trying to use autocompletion.

I'm not sure if this is something that is implemented inside the cpptools or provided by an external tool (like clang), but I hope this clarifies what I was trying to "achieve".

@sean-mcmanus sean-mcmanus added Feature Request and removed more info needed The issue report is not actionable in its current state labels May 8, 2017
@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented May 8, 2017

The C++17 nested namespace feature doesn't even work with the Visual Studio 2017 IntelliSense, although the cl.exe compiler seems to support it. Our goal is to support C++17, but currently our level of support is around C++14. I'm not sure yet what the timeframe is.

@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented May 8, 2017

My previous statement is incorrect. Nested namespace definitions work in VS 2017 when the /std:c++latest compile flag is used, and it works in VS Code on Windows -- I'm reproing the error you're seeing on Linux. So it looks like a bug.

@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented May 8, 2017

The bug is "fixed" by adding , "-D_MSC_EXTENSIONS" to the end of the "defaults" array in ~/.vscode/extensions/ms-vscode.cpptools-0.11.0/bin/msvc.json . However, I'm not sure yet if that change has any other unwanted consequences (i.e. incorrect error squiggles) so we need to investigate it more. If you can test that change and let us know if you encounter any regressions that would help.

@mdziekon
Copy link
Author

mdziekon commented May 8, 2017

@sean-mcmanus Thank you for your response. I can confirm that this quick "fix" allowed me to use nested namespaces. However (just a note for other people struggling with the same problem), I had to apply this change to the msvc.json file to make it work:

{
    "match": "^/lang_cpp_",
    "replace": "--c++latest"
}

(previous value of replace key was c++14).

I'll test that out and report any problems once found.

@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented May 8, 2017

Using "c++latest" as described should not work -- hovering should give an error message about "No IL available". It probably looks like it worked because the error disappeared because the invalid flag causes the errors to go away. Can you try the other , "-D_MSC_EXTENSIONS" fix? The c++14 is the latest flag that is supported by us and the c++17 flag behaves identically. From what I understand, the Microsoft compilers added the nested namespace definition feature in VS 2015 as an extension before C++17 was finished.

@mdziekon
Copy link
Author

mdziekon commented May 8, 2017

At first, I've applied only the , "-D_MSC_EXTENSIONS" fix, and unfortunately nothing has changed.
You're probably right about the --c++latest being a placebo. I was under the impression that this indeed did work, because I saw working autocompletion, which turned out to work even without all these changes.

Btw, I forgot to mention that I'm using VSCode Insiders, here is the exact version number:

Version 1.13.0-insider
Commit 674200fd49ad8c1557f884903fd09647975270f8
Date 2017-05-08T06:08:43.536Z
Shell 1.6.6
Renderer 56.0.2924.87
Node 7.4.0

@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented May 8, 2017

Yeah, I was mistaken. For some strange reason, simply adding a comma to the end of the "defaults" list causes it to become "fixed" (not the "-D_MSC_EXTENSIONS" mentioned earlier, e.g. "-D_USE_DECLSPECS_FOR_SAL=1",). I'm not able to detect a negative side effect from this change yet though (I'm getting correct IntelliSense errors and hover results). And the change needs to be made in the .vscode-insiders folder for insiders.

@mdziekon
Copy link
Author

mdziekon commented May 8, 2017

Simply placing , at the end of the array breaks standard libraries includes - VSCode cannot resolve includes like #include <vector> (which were previously working just fine).
It also throws 'name followed by '::' must be a class or namespace name' when trying to use a class from that nested namespace, but that might be broken because of other problems (mentioned includes) inside that file.

@sean-mcmanus
Copy link
Contributor

Okay, you're right, I'm able to repro the problem with vector. We need to investigate further why this is working on Windows and not Linux.

@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented May 8, 2017

Changing "replace": "--c++14" to "replace": "--c++17" in the msvc.json fixes the issue. I didn't think we had any C++17 support, but it looks like we support namespace attributes, nested namespace definitions, enumerator attributes, terse static assert, and possibly utf8 char literals and relaxed range-based for (not sure about the last two). But more C++17 is expected later.

@mdziekon
Copy link
Author

mdziekon commented May 8, 2017

I can confirm it works as expected (this time, introducing any kind of error in other places of the code properly displays that, so the language server works fine). Thank you for your response and investigation.

@sean-mcmanus sean-mcmanus added the fixed Check the Milestone for the release in which the fix is or will be available. label May 9, 2017
@bobbrow
Copy link
Member

bobbrow commented May 23, 2017

This was fixed in version 0.11.1. Please reopen if you continue to have this issue.

@bobbrow bobbrow closed this as completed May 23, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Oct 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Feature Request fixed Check the Milestone for the release in which the fix is or will be available. Language Service
Projects
None yet
Development

No branches or pull requests

3 participants