[PATCH] D134379: [clangd] IncludeCleaner: handle using namespace

Aleksandr Platonov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 21 13:40:45 PDT 2022


ArcsinX added a comment.

In D134379#3806664 <https://reviews.llvm.org/D134379#3806664>, @sammccall wrote:

> Is test.h meaningfully used in that example?
> Yes, the code is going to fail to compile without it, but it seems like the "spirit" of IWYU would say to delete both the include and the using directive.

Yes, in this example both should be removed, but from IDE user point of view, I expect warning about unused `using namespace ...`, and after it will be removed, warning about unused include can appear.

Also:

- I think that in unsure cases, we should keep includes and avoid code with errors
- IWYU tool doesn't report that include can be removed

> My concern about marking it used is that namespaces are typically redeclared in *every* header, and this effectively disables the feature on large swaths of code:
>
>   // foo.h
>   namespace myproj { void foo(); }
>   
>   // bar.h
>   namespace myproj { void bar(); }
>   
>   // main.cc
>   #include "foo.h"
>   #include "bar.h" // not meaningfully used
>   
>   using namespace myproj;
>   int main() {
>    foo();
>   }

In this case I don't see a difference with other declaration types. E.g. function

  // foo.h
  void foo();
  
  // bar.h
  void foo();
  
  // main.cc
  #include "foo.h"
  #include "bar.h"
  
  int main() {
   foo();
  }

both foo.h and bar.h contains prototype of foo(), but bar.h can be removed


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134379/new/

https://reviews.llvm.org/D134379



More information about the cfe-commits mailing list