subject:"\[PATCH\] D156650\: \[clangd\] Respect IWYU keep pragma for standard headers."

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

2023-07-31 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

In D156650#4545867 , @kadircet wrote:

> also we should get this cherry-picked too. `keep` pragmas on includes are not 
> common, but people do have export pragmas often enough to cause some 
> annoyance here.

Filed https://github.com/llvm/llvm-project/issues/64260.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156650

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

2023-07-31 Thread Haojian Wu via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdcb28244faa8: [clangd] Respect IWYU keep pragma for standard 
headers. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156650

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -78,6 +78,8 @@
   auto TU = TestTU::withCode(R"cpp(
 #include 
 #include 
+#include  // IWYU pragma: keep
+#include  // IWYU pragma: export
 std::list x;
   )cpp");
   // Layout of std library impl is not relevant.
@@ -86,10 +88,13 @@
 namespace std {
   template  class list {};
   template  class queue {};
+  template  class vector {};
 }
   )cpp";
   TU.AdditionalFiles["list"] = "#include ";
   TU.AdditionalFiles["queue"] = "#include ";
+  TU.AdditionalFiles["vector"] = "#include ";
+  TU.AdditionalFiles["string"] = "#include ";
   TU.ExtraArgs = {"-isystem", testRoot()};
   auto AST = TU.build();
   IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -71,6 +71,8 @@
 bool mayConsiderUnused(
 const Inclusion &Inc, ParsedAST &AST,
 const include_cleaner::PragmaIncludes *PI) {
+  if (PI && PI->shouldKeep(Inc.HashLine + 1))
+  return false;
   // FIXME(kirillbobyrev): We currently do not support the umbrella headers.
   // System headers are likely to be standard library headers.
   // Until we have good support for umbrella headers, don't warn about them.
@@ -82,8 +84,6 @@
   AST.getIncludeStructure().getRealPath(HID));
   assert(FE);
   if (PI) {
-if (PI->shouldKeep(Inc.HashLine + 1))
-  return false;
 // Check if main file is the public interface for a private header. If so 
we
 // shouldn't diagnose it as unused.
 if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -78,6 +78,8 @@
   auto TU = TestTU::withCode(R"cpp(
 #include 
 #include 
+#include  // IWYU pragma: keep
+#include  // IWYU pragma: export
 std::list x;
   )cpp");
   // Layout of std library impl is not relevant.
@@ -86,10 +88,13 @@
 namespace std {
   template  class list {};
   template  class queue {};
+  template  class vector {};
 }
   )cpp";
   TU.AdditionalFiles["list"] = "#include ";
   TU.AdditionalFiles["queue"] = "#include ";
+  TU.AdditionalFiles["vector"] = "#include ";
+  TU.AdditionalFiles["string"] = "#include ";
   TU.ExtraArgs = {"-isystem", testRoot()};
   auto AST = TU.build();
   IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -71,6 +71,8 @@
 bool mayConsiderUnused(
 const Inclusion &Inc, ParsedAST &AST,
 const include_cleaner::PragmaIncludes *PI) {
+  if (PI && PI->shouldKeep(Inc.HashLine + 1))
+  return false;
   // FIXME(kirillbobyrev): We currently do not support the umbrella headers.
   // System headers are likely to be standard library headers.
   // Until we have good support for umbrella headers, don't warn about them.
@@ -82,8 +84,6 @@
   AST.getIncludeStructure().getRealPath(HID));
   assert(FE);
   if (PI) {
-if (PI->shouldKeep(Inc.HashLine + 1))
-  return false;
 // Check if main file is the public interface for a private header. If so we
 // shouldn't diagnose it as unused.
 if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

2023-07-31 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 545589.
hokein marked an inline comment as done.
hokein added a comment.

address review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156650

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -78,6 +78,8 @@
   auto TU = TestTU::withCode(R"cpp(
 #include 
 #include 
+#include  // IWYU pragma: keep
+#include  // IWYU pragma: export
 std::list x;
   )cpp");
   // Layout of std library impl is not relevant.
@@ -86,10 +88,13 @@
 namespace std {
   template  class list {};
   template  class queue {};
+  template  class vector {};
 }
   )cpp";
   TU.AdditionalFiles["list"] = "#include ";
   TU.AdditionalFiles["queue"] = "#include ";
+  TU.AdditionalFiles["vector"] = "#include ";
+  TU.AdditionalFiles["string"] = "#include ";
   TU.ExtraArgs = {"-isystem", testRoot()};
   auto AST = TU.build();
   IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -71,6 +71,8 @@
 bool mayConsiderUnused(
 const Inclusion &Inc, ParsedAST &AST,
 const include_cleaner::PragmaIncludes *PI) {
+  if (PI && PI->shouldKeep(Inc.HashLine + 1))
+  return false;
   // FIXME(kirillbobyrev): We currently do not support the umbrella headers.
   // System headers are likely to be standard library headers.
   // Until we have good support for umbrella headers, don't warn about them.
@@ -82,8 +84,6 @@
   AST.getIncludeStructure().getRealPath(HID));
   assert(FE);
   if (PI) {
-if (PI->shouldKeep(Inc.HashLine + 1))
-  return false;
 // Check if main file is the public interface for a private header. If so 
we
 // shouldn't diagnose it as unused.
 if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -78,6 +78,8 @@
   auto TU = TestTU::withCode(R"cpp(
 #include 
 #include 
+#include  // IWYU pragma: keep
+#include  // IWYU pragma: export
 std::list x;
   )cpp");
   // Layout of std library impl is not relevant.
@@ -86,10 +88,13 @@
 namespace std {
   template  class list {};
   template  class queue {};
+  template  class vector {};
 }
   )cpp";
   TU.AdditionalFiles["list"] = "#include ";
   TU.AdditionalFiles["queue"] = "#include ";
+  TU.AdditionalFiles["vector"] = "#include ";
+  TU.AdditionalFiles["string"] = "#include ";
   TU.ExtraArgs = {"-isystem", testRoot()};
   auto AST = TU.build();
   IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -71,6 +71,8 @@
 bool mayConsiderUnused(
 const Inclusion &Inc, ParsedAST &AST,
 const include_cleaner::PragmaIncludes *PI) {
+  if (PI && PI->shouldKeep(Inc.HashLine + 1))
+  return false;
   // FIXME(kirillbobyrev): We currently do not support the umbrella headers.
   // System headers are likely to be standard library headers.
   // Until we have good support for umbrella headers, don't warn about them.
@@ -82,8 +84,6 @@
   AST.getIncludeStructure().getRealPath(HID));
   assert(FE);
   if (PI) {
-if (PI->shouldKeep(Inc.HashLine + 1))
-  return false;
 // Check if main file is the public interface for a private header. If so we
 // shouldn't diagnose it as unused.
 if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

2023-07-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added a comment.

also we should get this cherry-picked too. `keep` pragmas on includes are not 
common, but people do have export pragmas often enough to cause some annoyance 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156650

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

2023-07-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks!




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:85-86
   if (PI) {
 if (PI->shouldKeep(Inc.HashLine + 1))
   return false;
 // Check if main file is the public interface for a private header. If so 
we

can you unify this and the previous check and just have a single:
```
if (PI && PI->shouldKeep(Inc.HashLine + 1))
  return false;
```

at the top of the function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156650

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

2023-07-31 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

see the issue https://github.com/llvm/llvm-project/issues/64191


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156650

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -76,6 +76,8 @@
   auto TU = TestTU::withCode(R"cpp(
 #include 
 #include 
+#include  // IWYU pragma: keep
+#include  // IWYU pragma: export
 std::list x;
   )cpp");
   // Layout of std library impl is not relevant.
@@ -84,10 +86,13 @@
 namespace std {
   template  class list {};
   template  class queue {};
+  template  class vector {};
 }
   )cpp";
   TU.AdditionalFiles["list"] = "#include ";
   TU.AdditionalFiles["queue"] = "#include ";
+  TU.AdditionalFiles["vector"] = "#include ";
+  TU.AdditionalFiles["string"] = "#include ";
   TU.ExtraArgs = {"-isystem", testRoot()};
   auto AST = TU.build();
   IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -74,7 +74,8 @@
   // System headers are likely to be standard library headers.
   // Until we have good support for umbrella headers, don't warn about them.
   if (Inc.Written.front() == '<')
-return tooling::stdlib::Header::named(Inc.Written).has_value();
+return tooling::stdlib::Header::named(Inc.Written).has_value() &&
+   !(PI && PI->shouldKeep(Inc.HashLine + 1));
   assert(Inc.HeaderID);
   auto HID = static_cast(*Inc.HeaderID);
   auto FE = AST.getSourceManager().getFileManager().getFileRef(


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -76,6 +76,8 @@
   auto TU = TestTU::withCode(R"cpp(
 #include 
 #include 
+#include  // IWYU pragma: keep
+#include  // IWYU pragma: export
 std::list x;
   )cpp");
   // Layout of std library impl is not relevant.
@@ -84,10 +86,13 @@
 namespace std {
   template  class list {};
   template  class queue {};
+  template  class vector {};
 }
   )cpp";
   TU.AdditionalFiles["list"] = "#include ";
   TU.AdditionalFiles["queue"] = "#include ";
+  TU.AdditionalFiles["vector"] = "#include ";
+  TU.AdditionalFiles["string"] = "#include ";
   TU.ExtraArgs = {"-isystem", testRoot()};
   auto AST = TU.build();
   IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -74,7 +74,8 @@
   // System headers are likely to be standard library headers.
   // Until we have good support for umbrella headers, don't warn about them.
   if (Inc.Written.front() == '<')
-return tooling::stdlib::Header::named(Inc.Written).has_value();
+return tooling::stdlib::Header::named(Inc.Written).has_value() &&
+   !(PI && PI->shouldKeep(Inc.HashLine + 1));
   assert(Inc.HeaderID);
   auto HID = static_cast(*Inc.HeaderID);
   auto FE = AST.getSourceManager().getFileManager().getFileRef(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

6 matches

Site Navigation

Mail list logo

Footer information