[PATCH] D109880: [clangd] PreamblePatch should be no-op if includes arent patched

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Don't create a useless functional patch with only filename in it when
there is only include directives to be patched but they're not
requested.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109880

Files:
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -554,6 +554,20 @@
   auto AST = createPatchedAST(Baseline, Modified);
   ASSERT_TRUE(AST);
 }
+
+TEST(PreamblePatch, NoopWhenNotRequested) {
+  llvm::StringLiteral Baseline = "#define M\nint num = M;";
+  llvm::StringLiteral Modified = "#define M\n#include <foo.h>\nint num = M;";
+  auto TU = TestTU::withCode(Baseline);
+  auto BaselinePreamble = TU.preamble();
+  ASSERT_TRUE(BaselinePreamble);
+
+  TU.Code = Modified.str();
+  MockFS FS;
+  auto PP = PreamblePatch::createMacroPatch(testPath(TU.Filename),
+                                            TU.inputs(FS), *BaselinePreamble);
+  EXPECT_TRUE(PP.text().empty());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -459,7 +459,7 @@
   bool IncludesChanged = BaselineScan->Includes != ModifiedScan->Includes;
   bool DirectivesChanged =
       BaselineScan->TextualDirectives != ModifiedScan->TextualDirectives;
-  if (!IncludesChanged && !DirectivesChanged)
+  if ((PatchType != PatchType::All || !IncludesChanged) && !DirectivesChanged)
     return PreamblePatch::unmodified(Baseline);
 
   PreamblePatch PP;



Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -554,6 +554,20 @@
   auto AST = createPatchedAST(Baseline, Modified);
   ASSERT_TRUE(AST);
 }
+
+TEST(PreamblePatch, NoopWhenNotRequested) {
+  llvm::StringLiteral Baseline = "#define M\nint num = M;";
+  llvm::StringLiteral Modified = "#define M\n#include <foo.h>\nint num = M;";
+  auto TU = TestTU::withCode(Baseline);
+  auto BaselinePreamble = TU.preamble();
+  ASSERT_TRUE(BaselinePreamble);
+
+  TU.Code = Modified.str();
+  MockFS FS;
+  auto PP = PreamblePatch::createMacroPatch(testPath(TU.Filename),
+                                            TU.inputs(FS), *BaselinePreamble);
+  EXPECT_TRUE(PP.text().empty());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -459,7 +459,7 @@
   bool IncludesChanged = BaselineScan->Includes != ModifiedScan->Includes;
   bool DirectivesChanged =
       BaselineScan->TextualDirectives != ModifiedScan->TextualDirectives;
-  if (!IncludesChanged && !DirectivesChanged)
+  if ((PatchType != PatchType::All || !IncludesChanged) && !DirectivesChanged)
     return PreamblePatch::unmodified(Baseline);
 
   PreamblePatch PP;

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