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

clangd-indexer should use or document -extra-arg="-Wno-everything" #24

Closed
smhc opened this issue Mar 22, 2019 · 6 comments
Closed

clangd-indexer should use or document -extra-arg="-Wno-everything" #24

smhc opened this issue Mar 22, 2019 · 6 comments
Assignees

Comments

@smhc
Copy link

smhc commented Mar 22, 2019

Projects that use g++ may not necessarily compile cleanly with clang due to several warnings treated as errors. The following option to clangd-indexer significantly helps in creating a useful index for these projects:

-extra-arg="-Wno-everything"

Given the purpose of the index, perhaps this should be the default option?
If not, maybe it could at least be promoted more in documentation/usage as it results in a much more useful index for the purposes of symbol lookups etc.

@smhc
Copy link
Author

smhc commented Mar 22, 2019

After more thought - does the background indexer for clangd have an equivalent option for:

-extra-arg="-Wno-everything"

I assume clangd would fail to index code that clang would fail to compile if it hits an -Werror type of warning. Bearing in mind that g++ wouldn't fail to compile the same code, hence the compiler options do not specify to ignore the problematic warnings.

@smhc
Copy link
Author

smhc commented Mar 23, 2019

For some more context:
We have a project compiled with g++ that uses "-Wall -Werror". This compiles fine with g++, but fails in clang++ due to the addition of other warnings (e.g -Wunused-function) that g++ doesn't have.

Effectively what we need is for clangd to honour the fact the compile_commands.json specifies 'g++' and turn off all clang++ specific warnings. This could be done automatically, or via a command line option to provide additional args to the compiler, similar to what clangd-indexer supports.

It may be necessary have two sets of 'extra-args'. One for the indexer, which would be safe to turn off all warnings, and another setting for the diagnostics where you'd likely want to leave on all the g++ warnings.

@sam-mccall
Copy link
Member

Yeah there are a few different ways to address your problem, but i think the cleanest is to disable all warnings during indexing as you suggest.

This should happen for all the different types of indexing we do (static, dynamic, background, it's a mess...).

Futzing with compile commands is always fiddly (it can be a clang-cl style command line too!) but that's just a better reason to do this in one place.

@smhc
Copy link
Author

smhc commented Mar 27, 2019

I can't help but think clang should have a gcc version x compatible mode where it disables additional warnings that gcc version x doesn't have. Or at the very least doesn't convert them into errors even if asked to. I assume it hasn't been asked for often enough to warrant doing.

@sam-mccall sam-mccall self-assigned this Mar 28, 2019
@sam-mccall
Copy link
Member

I assume clangd would fail to index code that clang would fail to compile if it hits an -Werror type of warning. Bearing in mind that g++ wouldn't fail to compile the same code, hence the compiler options do not specify to ignore the problematic warnings.

Turns out this assumption doesn't hold :-) Non-fatal errors don't stop indexing, and all symbols are still collected.

However there are still reasons to do this:

  • -ferror-limit (which has a default) can cause warnings with -Werror to create a fatal error, that does stop indexing
  • the warning spam from clangd-indexer isn't helpful
  • some warnings are expensive, we should avoid computing them at all

llvm-git-migration pushed a commit to llvm/llvm-project that referenced this issue Mar 28, 2019
Summary:
- we don't record the warnings at all
- we don't want to stop indexing if we hit error-limit due to warnings
- this allows some analyses to be skipped which can save some CPU

clangd/clangd#24

Reviewers: hokein

Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D59935

llvm-svn: 357186
dtzWill pushed a commit to llvm-mirror/clang-tools-extra that referenced this issue Mar 28, 2019
Summary:
- we don't record the warnings at all
- we don't want to stop indexing if we hit error-limit due to warnings
- this allows some analyses to be skipped which can save some CPU

clangd/clangd#24

Reviewers: hokein

Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D59935

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@357186 91177308-0d34-0410-b5e6-96231b3b80d8
@sam-mccall
Copy link
Member

Fixed in https://reviews.llvm.org/rL357186
(Though I suspect this wouldn't have prevented an index from building in most cases)

I can't help but think clang should have a gcc version x compatible mode where it disables additional warnings that gcc version x doesn't have. Or at the very least doesn't convert them into errors even if asked to. I assume it hasn't been asked for often enough to warrant doing.

I can see how this would be desirable, though it also sounds hard to keep in sync. In any case that's indeed a FR for clang.

arichardson pushed a commit to arichardson/llvm-project that referenced this issue Apr 8, 2019
Summary:
- we don't record the warnings at all
- we don't want to stop indexing if we hit error-limit due to warnings
- this allows some analyses to be skipped which can save some CPU

clangd/clangd#24

Reviewers: hokein

Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D59935

llvm-svn: 357186
shrouxm pushed a commit to sourcegraph/lsif-clang that referenced this issue Jul 12, 2020
Summary:
- we don't record the warnings at all
- we don't want to stop indexing if we hit error-limit due to warnings
- this allows some analyses to be skipped which can save some CPU

clangd/clangd#24

Reviewers: hokein

Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D59935

llvm-svn: 357186
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants