clangd / clangd

clangd language server
https://clangd.llvm.org
Apache License 2.0
1.53k stars 63 forks source link

Include Cleaner recommends removing header needed by template instantiation #2053

Open graveljp opened 4 months ago

graveljp commented 4 months ago

Consider this simplified assert-style Check class:

check.h

#pragma once

#include <iosfwd>

class Check {
 public:
  Check(bool condition);

  template <typename T>
  std::ostream& operator<<(T&& streamed_type) {
    return stream() << streamed_type;
  }

  std::ostream& stream();
};

Most users of the class will not use operator<<. Thus, we can reduce the include tree size for most users by letting those that actually print a message include <iostream>. clangd however recommends removing such header. Doing so would break the build:

main.cc

#include "check.h"
#include <ostream>  // Warns: "Included header ostream is not used directly"

int main() {
  Check(false) << "Error message";
  return 0;
}

include-what-you-use correctly handles this.

System information

Output of clangd --version:

clangd version 19.0.0git (https://github.com/llvm/llvm-project.git 55e59083cb610f30ad21fe8c8cdb9900534937ec)
Features: linux
Platform: x86_64-unknown-linux-gnu

Editor/LSP plugin: VSCode

Operating system: Linux

kadircet commented 2 months ago

hi @graveljp, sorry for late reply, but this is WAI per our include-what-you-spell policy, that's a core assumption that we should've document externally, sorry for lacking that.

This is what enables include-cleaner to be fast, simple and operate without humans. According to this policy, you don't have any spelling of an entity in your main.cc that requires inclusion of <ostream>. It's responsibility of check.h to make sure symbols it uses are defined in the compilation.

Recommended approach for such cases would look like: check_with_ostream.h:

#pragma once
#include "check.h" // IWYU pragma: export
#include <ostream>

template <typename T>
std::ostream& Check::operator<<(T&& streamed_type) {
    return stream() << streamed_type;
}

this way people that need to depend on the version of the symbol with streaming capabilities can include check_with_ostream.h, and include-cleaner will recognize it as an alternative provider for check.h and won't recommend deletion of it (but it also won't insert it if missing).

In addition to that you can also add // IWYU pragma: keep to relevant sources, but I believe that's much more expensive.