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

Include compile_flags.txt in installed header directory #45348

Open
redbeard0531 mannequin opened this issue May 20, 2020 · 8 comments
Open

Include compile_flags.txt in installed header directory #45348

redbeard0531 mannequin opened this issue May 20, 2020 · 8 comments
Labels
bugzilla Issues migrated from bugzilla clangd libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@redbeard0531
Copy link
Mannequin

redbeard0531 mannequin commented May 20, 2020

Bugzilla Link 46003
Version unspecified
OS Linux
CC @mclow

Extended Description

Having a compile_commands.txt file that includes at least -stdlib=libc++ significantly improves the experience when using clangd and navigating to one of the libc++ headers on platforms where libc++ is not the default stdlib. It may also be worth setting -std=c++LATEST so that the maximum amount of code is parsed and usable, but that is less important than not having every file broken due to mixing stdlib headers.

@redbeard0531
Copy link
Mannequin Author

redbeard0531 mannequin commented May 20, 2020

Sorry, I combined compile_commands.json with compile_flags.txt. I meant the latter since that seems like the simpler solution than building a full compiledb listing flags for each header. OTOH, using a compiledb might be nice for headers that are only valid with specific flags, like TS language support headers.

I fixed the title, but couldn't edit my initial description comment.

@mclow
Copy link
Contributor

mclow commented May 21, 2020

This is problematic, because it has the possibility of colliding with a user-provided file named 'compile_commands.json' and having it be included inadvertently (since the libc++ include directory is on every user's include path).

Yeah, it's unlikely.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@sam-mccall
Copy link
Collaborator

(Sorry for finding this so late)

@mclow Regarding putting the file on the include path, can we put it in the parent directory instead?

I don't know about the general case (or much about libc++), but on my system <algorithm> is /usr/lib/llvm-11/include/c++/v1/algorithm. /usr/lib/llvm-11/include/c++/compile_flags.txt would not be on the include path.

@ldionne
Copy link
Member

ldionne commented Jan 7, 2022

The main problem I actually see is that the compile_flags.txt file would most likely not correctly represent the flags that the user is using when compiling, and a such it would be misleading (at least to some extent).

What exactly should go into compile_flags.txt?

@sam-mccall
Copy link
Collaborator

I think this would work well:

-stdlib=libc++
-stdlib++-isystemv1
-xc++-header

I think -stdlib=libc++ does nothing additional once -stdlib++-isystem is set, so it could be left out.
I don't have an opinion on whether -std should be unset or c++20 or something else.

As you point out it's not perfect, but it avoids the biggest practical issues:

  • prevents internal #includes from failing to resolve, or worse resolving to parts of libstdc++ or a different copy of libc++
  • prevents spurious errors causing us to hit -ferror-limit and suppressing real ones (e.g. '__config' file not found with <angled> include; use "quotes" instead)
  • allows the files to be parsed by tools even though they have no extension. We have a hack for this in clangd, but clang-query etc fail.

Our experience is that the value of having tools/features work on headers seems to outweigh the occasional confusion caused by picking the wrong configuration.

@sam-mccall
Copy link
Collaborator

(I've tested that locally and it indeed helps a lot)

@RedBeard0531
Copy link

@sam-mccall that is roughly what I was thinking, but is probably a better version. I would personally suggest -std=c++20 or even -std=c++2b (since gcc/clang don't have an equivalent of /std:c++-latest which would be ideal) in order to allow tooling to parse the most code. It would also be a better experience when using editors that use semantic highlighting to show #if-disabled code as commented out. This would allow them to at least read the whole header with normal syntax highlighting.

@ldionne
Copy link
Member

ldionne commented Jan 14, 2022

Regarding passing -std=c++20, the issue though is that it would arguably make editors highlight the wrong #if-#else block if you're not actually compiling your code with -std=c++20, which I could see creating a lot of confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clangd libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

4 participants