[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG89878e8c966a: [clang-format] Find main include after block 
ended with #pragma hdrstop (authored by rjelonek, committed by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D94217?vs=315509=315719#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94217

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -900,6 +900,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\n"
+ "#include \"b.h\"\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\n"
+ "\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"e.h\"\n";
+
+  Expected = "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\n"
+ "\n"
+ "#include \"e.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 TEST_F(SortIncludesTest, skipUTF8ByteOrderMarkMerge) {
   Style.IncludeBlocks = Style.IBS_Merge;
   std::string Code = "\xEF\xBB\xBF#include \"d.h\"\r\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2308,7 +2308,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // Precompiled headers.
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
 }
 if (Pos == StringRef::npos || Pos + 1 == Code.size())


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -900,6 +900,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\n"
+ "#include \"b.h\"\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\n"
+ "\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"e.h\"\n";
+
+  Expected = "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\n"
+ "\n"
+ "#include \"e.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 TEST_F(SortIncludesTest, skipUTF8ByteOrderMarkMerge) {
   Style.IncludeBlocks = Style.IBS_Merge;
   std::string Code = "\xEF\xBB\xBF#include \"d.h\"\r\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2308,7 +2308,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, 

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Rafał Jelonek via Phabricator via cfe-commits]

rjelonek updated this revision to Diff 315509.
rjelonek added a comment.

upload diff via arc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94217

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\n"
+ "#include \"b.h\"\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\n"
+ "\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"e.h\"\n";
+
+  Expected = "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\n"
+ "\n"
+ "#include \"e.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\n"
+ "#include \"b.h\"\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\n"
+ "\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"e.h\"\n";
+
+  Expected = "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\n"
+ "\n"
+ "#include \"e.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }
___
cfe-commits mailing list

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Rafał Jelonek via Phabricator via cfe-commits]

rjelonek updated this revision to Diff 315431.
rjelonek added a comment.

rebase patch on master/main


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

https://reviews.llvm.org/D94217

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\n"
+ "#include \"b.h\"\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\n"
+ "\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"e.h\"\n";
+
+  Expected = "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\n"
+ "\n"
+ "#include \"e.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\n"
+ "#include \"b.h\"\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\n"
+ "\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"e.h\"\n";
+
+  Expected = "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\n"
+ "\n"
+ "#include \"e.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

I've set the repository to rG  
LLVM. Could you please rebase and update the patch so that the CI gets 
triggered?

I hope it is enough to just reupload a patch. I have heard that sometimes you 
might need to reupload the diff twice, once removing the repo and then setting 
it back, but I'm don't know what the correct recipe is (I always use arc).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Rafał Jelonek via Phabricator via cfe-commits]

rjelonek added a comment.

no, i upload diff file


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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

I like what you did with tests. And thank you for the patch.
Before accepting this formally, I'd like to see the premerge tests pass. How do 
you upload the diff? Do you use arcanist?


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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Rafał Jelonek via Phabricator via cfe-commits]

rjelonek updated this revision to Diff 315236.
rjelonek added a comment.

Use LF instead of CRLF for case with extended `#pragma hdrstop`


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

https://reviews.llvm.org/D94217

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\n"
+ "#include \"b.h\"\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\n"
+ "\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"e.h\"\n";
+
+  Expected = "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\n"
+ "\n"
+ "#include \"e.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\n"
+ "#include \"b.h\"\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\n"
+ "\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"e.h\"\n";
+
+  Expected = "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\n"
+ "\n"
+ "#include \"e.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }
___
cfe-commits mailing list

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Rafał Jelonek via Phabricator via cfe-commits]

rjelonek added a comment.

In D94217#2485178 , @curdeius wrote:

> LGTM if you add a test for hdrstop(filename) and possibly with LF newline (as 
> the test you've already added tests CRLF).

You would like to 2 unit tests called 
`DoNotTreatPrecompiledHeadersAsFirstBlockCRLF` and 
'DoNotTreatPrecompiledHeadersAsFirstBlockLF`?


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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Rafał Jelonek via Phabricator via cfe-commits]

rjelonek updated this revision to Diff 315223.
rjelonek added a comment.

Add test to check extended form of `#pragma hdrstop`


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

https://reviews.llvm.org/D94217

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

LGTM if you add a test for hdrstop(filename) and possibly with LF newline (as 
the test you've already added tests CRLF).


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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/Format.cpp:2308
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;

would you now please add a unit test that handles the new case


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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Rafał Jelonek via Phabricator via cfe-commits]

rjelonek updated this revision to Diff 315169.
rjelonek added a comment.

Add support to `#pragma hdrstop( "c:\\projects\\include\\myinc.pch" )'


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

https://reviews.llvm.org/D94217

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,27 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,27 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Rafał Jelonek via Phabricator via cfe-commits]

rjelonek added a comment.

heh, maybe you seen unexpectedly closed IDE, errors while expanding list in 
source editor or blinking gui, etc ;D
I made plugin to use clang-format in Builder.

After reading the msvc documentation, I agree with you.  `#pragma hdrstop( 
"c:\\projects\\include\\myinc.pch" )` should be supported, also. I will change 
this soon.
Builder just ignore everything after `#pragma hdrstop` to the end of line. I 
check it on Builder ver. XE7




Comment at: clang/lib/Format/Format.cpp:2308
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed == "#pragma hdrstop") // precompiled headers
+  FirstIncludeBlock = true;

MyDeveloperDay wrote:
> Should this handle this form?
> 
> ```
> #pragma hdrstop( "c:\\projects\\include\\myinc.pch" )
> ```
No


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D94217#2483925 , @rjelonek wrote:

> Currently at work we use Embarcadero Builder C++

I'm very sorry for you! (As someone who has (had?) also to use it.)

> In builder all above #pragma hdrstop is a precompiled header. And we have 
> this #pragma at almost all cpp files, so mainInclude is ignored because is in 
> second include block.
>
> `#pragma hdrstop( "c:\\projects\\include\\myinc.pch" )`
> This form shouldn't be handled

Why not? I don't know if `bcc` does support it, but `MSVC` does. So I think it 
definitely should support both, or none.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Rafał Jelonek via Phabricator via cfe-commits]

rjelonek added a comment.

Currently at work we use Embarcadero Builder C++
In builder all above #pragma hdrstop is a precompiled header. And we have this 
#pragma at almost all cpp files, so mainInclude is ignored because is in second 
include block.

`#pragma hdrstop( "c:\\projects\\include\\myinc.pch" )`
This form shoudnt be handled


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I've not used this before, but you seem to be suggesting that `#pragma hdrstop` 
should be used to determine when the files main header should now be placed?

Could you explain a little more as to why you think that should be the case 
(i.e. is there a technical reason as to why, I'm happy to be educated).




Comment at: clang/lib/Format/Format.cpp:2308
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed == "#pragma hdrstop") // precompiled headers
+  FirstIncludeBlock = true;

Should this handle this form?

```
#pragma hdrstop( "c:\\projects\\include\\myinc.pch" )
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94217

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

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

[Rafał Jelonek via Phabricator via cfe-commits]

rjelonek created this revision.
rjelonek added reviewers: rsmith, rnk.
rjelonek created this object with visibility "All Users".
rjelonek created this object with edit policy "Members of Project: clang".
rjelonek added a project: clang-format.
rjelonek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Find main include in first include block not ended with #pragma hdrstop


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94217

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,27 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed == "#pragma hdrstop") // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,27 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed == "#pragma hdrstop") // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits