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

Completion fails in templates on windows due to implicit -fdelayed-template-parsing #302

Closed
jabra11 opened this issue Mar 5, 2020 · 5 comments
Labels
enhancement New feature or request windows Mostly or entirely affects windows users

Comments

@jabra11
Copy link

jabra11 commented Mar 5, 2020

Hello, it seems like the auto completion provided by the vscode clangd extension (for windows) stops working when writing templates. Here a short clip of this behavior: https://streamable.com/6qri0
The version of my clangd application is 9.0.1 (x64 windows). Is there anything I can do?

@sam-mccall
Copy link
Member

Can you try adding the flag -fno-delayed-template-parsing to your compile_commands.json?
(Or if you don't have one, create compile_flags.txt in the same dir with just that string)

MS compilers have a nonstandard way of handling templates, clang has an option to emulate it and turns it on by default on windows (maybe it's needed to parse common libraries?). I was vaguely aware of this but hadn't realized it might break completion entirely in templates!

@jabra11
Copy link
Author

jabra11 commented Mar 5, 2020

Yes, that seemed to fix it. Is there a way to build that into the extension? Otherwise I'd have to add that flag to every of my projects CMakeLists to rely on the build system to generate it

@sam-mccall
Copy link
Member

Long-term fix is to make this work by default in clangd, either by turning off delayed template parsing in general or just in code completion.

Short-term, I think you should be able to add -fno-delayed-template-parsing to CMAKE_CXX_FLAGS and rebuild (unless your build actually relies on this behavior)

@jabra11 jabra11 closed this as completed Mar 12, 2020
@sam-mccall
Copy link
Member

I'd actually like to keep this open and do a fairly limited fix.

We discussed this and I don't think we can just turn delayed template parsing off in clangd, we'll break people including slightly old windows headers.
However the file the user's editing is unlikely to be broken, and clang will let us build the preamble (headers) with delayed template parsing and then the main-file without it.

So the most-limited fix: turn it off for the code-completion parse only. In most cases of invalid code clang will realize "missing typename" and recover, only in pathological cases will it actually parse the code differently. Users will probably never notice.
The more ambitious fix: turn it off for the main-file parse as well as code-completion, while keeping it on for preamble builds. That means that users will see errors for non-standard code, but only in the file they're actually editing.

@sam-mccall sam-mccall reopened this Mar 12, 2020
@sam-mccall sam-mccall added the windows Mostly or entirely affects windows users label Mar 12, 2020
@sam-mccall sam-mccall changed the title Auto completion not working when writing C++ templates Completion fails in templates on windows due to implicit -fdelayed-template-parsing Mar 12, 2020
@kadircet kadircet added the enhancement New feature or request label Mar 18, 2020
arichardson pushed a commit to arichardson/llvm-project that referenced this issue Apr 2, 2020
Summary:
It prevents code completion entirely in affected method bodies.
The main reason it's turned on is for compatibility with headers, so we turn it
off for the main file only. This is allowed because it's a compatible langopt.

Fixes clangd/clangd#302

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D77176
@sam-mccall
Copy link
Member

sam-mccall commented Apr 24, 2020

@kadircet this came up again (code completion works now but other features don't).

Maybe it's safe to flip for everything except background index, since we (almost always) don't parse function bodies? WDYT?

@sam-mccall sam-mccall reopened this Apr 24, 2020
arichardson pushed a commit to arichardson/llvm-project that referenced this issue Jul 2, 2020
Summary:
This is on by default in windows and breaks most features in template bodies.
We'd already disabled it in code completion, now disable it for building ASTs.

Potential regressions:
 - we may give spurious errors where files with templates relying on delayed
   parsing are directly opened
 - we may misparse such template bodies that are instantiated (and therefore
   *were* previously parsed)

Still *probably* a win overall. Avoiding the regressions entirely would be
substantial work and we don't have plans for it now.

Fixes clangd/clangd#302 (again)

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D78848
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
Summary:
It prevents code completion entirely in affected method bodies.
The main reason it's turned on is for compatibility with headers, so we turn it
off for the main file only. This is allowed because it's a compatible langopt.

Fixes clangd/clangd#302

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D77176
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
Summary:
This is on by default in windows and breaks most features in template bodies.
We'd already disabled it in code completion, now disable it for building ASTs.

Potential regressions:
 - we may give spurious errors where files with templates relying on delayed
   parsing are directly opened
 - we may misparse such template bodies that are instantiated (and therefore
   *were* previously parsed)

Still *probably* a win overall. Avoiding the regressions entirely would be
substantial work and we don't have plans for it now.

Fixes clangd/clangd#302 (again)

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D78848
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request windows Mostly or entirely affects windows users
Projects
None yet
Development

No branches or pull requests

3 participants