[PATCH] D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7c8b9c102f38: [clang-format] [NFC] Restructure 
getLineCommentIndentPrefix (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95081

Files:
  clang/lib/Format/BreakableToken.cpp


Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -42,24 +42,26 @@
 static StringRef getLineCommentIndentPrefix(StringRef Comment,
 const FormatStyle ) {
   static constexpr StringRef KnownCStylePrefixes[] = {"///<", "//!<", "///",
-  "//",   "//!",  "//:"};
-  static constexpr StringRef KnownTextProtoPrefixes[] = {"//", "#", "##", 
"###",
- ""};
+  "//!",  "//:",  "//"};
+  static constexpr StringRef KnownTextProtoPrefixes[] = {"", "###", "##",
+ "//", "#"};
   ArrayRef KnownPrefixes(KnownCStylePrefixes);
   if (Style.Language == FormatStyle::LK_TextProto)
 KnownPrefixes = KnownTextProtoPrefixes;
 
-  StringRef LongestPrefix;
+  assert(std::is_sorted(KnownPrefixes.begin(), KnownPrefixes.end(),
+[](StringRef Lhs, StringRef Rhs) noexcept {
+  return Lhs.size() > Rhs.size();
+}));
+
   for (StringRef KnownPrefix : KnownPrefixes) {
 if (Comment.startswith(KnownPrefix)) {
-  size_t PrefixLength = KnownPrefix.size();
-  while (PrefixLength < Comment.size() && Comment[PrefixLength] == ' ')
-++PrefixLength;
-  if (PrefixLength > LongestPrefix.size())
-LongestPrefix = Comment.substr(0, PrefixLength);
+  const auto PrefixLength =
+  Comment.find_first_not_of(' ', KnownPrefix.size());
+  return Comment.substr(0, PrefixLength);
 }
   }
-  return LongestPrefix;
+  return {};
 }
 
 static BreakableToken::Split


Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -42,24 +42,26 @@
 static StringRef getLineCommentIndentPrefix(StringRef Comment,
 const FormatStyle ) {
   static constexpr StringRef KnownCStylePrefixes[] = {"///<", "//!<", "///",
-  "//",   "//!",  "//:"};
-  static constexpr StringRef KnownTextProtoPrefixes[] = {"//", "#", "##", "###",
- ""};
+  "//!",  "//:",  "//"};
+  static constexpr StringRef KnownTextProtoPrefixes[] = {"", "###", "##",
+ "//", "#"};
   ArrayRef KnownPrefixes(KnownCStylePrefixes);
   if (Style.Language == FormatStyle::LK_TextProto)
 KnownPrefixes = KnownTextProtoPrefixes;
 
-  StringRef LongestPrefix;
+  assert(std::is_sorted(KnownPrefixes.begin(), KnownPrefixes.end(),
+[](StringRef Lhs, StringRef Rhs) noexcept {
+  return Lhs.size() > Rhs.size();
+}));
+
   for (StringRef KnownPrefix : KnownPrefixes) {
 if (Comment.startswith(KnownPrefix)) {
-  size_t PrefixLength = KnownPrefix.size();
-  while (PrefixLength < Comment.size() && Comment[PrefixLength] == ' ')
-++PrefixLength;
-  if (PrefixLength > LongestPrefix.size())
-LongestPrefix = Comment.substr(0, PrefixLength);
+  const auto PrefixLength =
+  Comment.find_first_not_of(' ', KnownPrefix.size());
+  return Comment.substr(0, PrefixLength);
 }
   }
-  return LongestPrefix;
+  return {};
 }
 
 static BreakableToken::Split
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix

[Marek Kurdej via Phabricator via cfe-commits]

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

Just an assert is ok IMO. We may fix it when LLVM will be compiled with C++20 
but this code may change before it happens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95081

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

[PATCH] D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix

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

HazardyKnusperkeks updated this revision to Diff 318156.
HazardyKnusperkeks added a comment.

Empty lines removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95081

Files:
  clang/lib/Format/BreakableToken.cpp


Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -42,24 +42,26 @@
 static StringRef getLineCommentIndentPrefix(StringRef Comment,
 const FormatStyle ) {
   static constexpr StringRef KnownCStylePrefixes[] = {"///<", "//!<", "///",
-  "//",   "//!",  "//:"};
-  static constexpr StringRef KnownTextProtoPrefixes[] = {"//", "#", "##", 
"###",
- ""};
+  "//!",  "//:",  "//"};
+  static constexpr StringRef KnownTextProtoPrefixes[] = {"", "###", "##",
+ "//", "#"};
   ArrayRef KnownPrefixes(KnownCStylePrefixes);
   if (Style.Language == FormatStyle::LK_TextProto)
 KnownPrefixes = KnownTextProtoPrefixes;
 
-  StringRef LongestPrefix;
+  assert(std::is_sorted(KnownPrefixes.begin(), KnownPrefixes.end(),
+[](StringRef Lhs, StringRef Rhs) noexcept {
+  return Lhs.size() > Rhs.size();
+}));
+
   for (StringRef KnownPrefix : KnownPrefixes) {
 if (Comment.startswith(KnownPrefix)) {
-  size_t PrefixLength = KnownPrefix.size();
-  while (PrefixLength < Comment.size() && Comment[PrefixLength] == ' ')
-++PrefixLength;
-  if (PrefixLength > LongestPrefix.size())
-LongestPrefix = Comment.substr(0, PrefixLength);
+  const auto PrefixLength =
+  Comment.find_first_not_of(' ', KnownPrefix.size());
+  return Comment.substr(0, PrefixLength);
 }
   }
-  return LongestPrefix;
+  return {};
 }
 
 static BreakableToken::Split


Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -42,24 +42,26 @@
 static StringRef getLineCommentIndentPrefix(StringRef Comment,
 const FormatStyle ) {
   static constexpr StringRef KnownCStylePrefixes[] = {"///<", "//!<", "///",
-  "//",   "//!",  "//:"};
-  static constexpr StringRef KnownTextProtoPrefixes[] = {"//", "#", "##", "###",
- ""};
+  "//!",  "//:",  "//"};
+  static constexpr StringRef KnownTextProtoPrefixes[] = {"", "###", "##",
+ "//", "#"};
   ArrayRef KnownPrefixes(KnownCStylePrefixes);
   if (Style.Language == FormatStyle::LK_TextProto)
 KnownPrefixes = KnownTextProtoPrefixes;
 
-  StringRef LongestPrefix;
+  assert(std::is_sorted(KnownPrefixes.begin(), KnownPrefixes.end(),
+[](StringRef Lhs, StringRef Rhs) noexcept {
+  return Lhs.size() > Rhs.size();
+}));
+
   for (StringRef KnownPrefix : KnownPrefixes) {
 if (Comment.startswith(KnownPrefix)) {
-  size_t PrefixLength = KnownPrefix.size();
-  while (PrefixLength < Comment.size() && Comment[PrefixLength] == ' ')
-++PrefixLength;
-  if (PrefixLength > LongestPrefix.size())
-LongestPrefix = Comment.substr(0, PrefixLength);
+  const auto PrefixLength =
+  Comment.find_first_not_of(' ', KnownPrefix.size());
+  return Comment.substr(0, PrefixLength);
 }
   }
-  return LongestPrefix;
+  return {};
 }
 
 static BreakableToken::Split
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix

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

HazardyKnusperkeks updated this revision to Diff 318151.
HazardyKnusperkeks added a comment.

Now with `assert`.

We could do a `static_assert`, but then we would have to write our own 
`is_sorted`, since the `std` is only `constexpr` in C++20.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95081

Files:
  clang/lib/Format/BreakableToken.cpp


Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -42,24 +42,28 @@
 static StringRef getLineCommentIndentPrefix(StringRef Comment,
 const FormatStyle ) {
   static constexpr StringRef KnownCStylePrefixes[] = {"///<", "//!<", "///",
-  "//",   "//!",  "//:"};
-  static constexpr StringRef KnownTextProtoPrefixes[] = {"//", "#", "##", 
"###",
- ""};
+  "//!",  "//:",  "//"};
+  static constexpr StringRef KnownTextProtoPrefixes[] = {"", "###", "##",
+ "//", "#"};
+
+
   ArrayRef KnownPrefixes(KnownCStylePrefixes);
   if (Style.Language == FormatStyle::LK_TextProto)
 KnownPrefixes = KnownTextProtoPrefixes;
 
-  StringRef LongestPrefix;
+  assert(std::is_sorted(KnownPrefixes.begin(), KnownPrefixes.end(),
+[](StringRef Lhs, StringRef Rhs) noexcept {
+  return Lhs.size() > Rhs.size();
+}));
+
   for (StringRef KnownPrefix : KnownPrefixes) {
 if (Comment.startswith(KnownPrefix)) {
-  size_t PrefixLength = KnownPrefix.size();
-  while (PrefixLength < Comment.size() && Comment[PrefixLength] == ' ')
-++PrefixLength;
-  if (PrefixLength > LongestPrefix.size())
-LongestPrefix = Comment.substr(0, PrefixLength);
+  const auto PrefixLength =
+  Comment.find_first_not_of(' ', KnownPrefix.size());
+  return Comment.substr(0, PrefixLength);
 }
   }
-  return LongestPrefix;
+  return {};
 }
 
 static BreakableToken::Split


Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -42,24 +42,28 @@
 static StringRef getLineCommentIndentPrefix(StringRef Comment,
 const FormatStyle ) {
   static constexpr StringRef KnownCStylePrefixes[] = {"///<", "//!<", "///",
-  "//",   "//!",  "//:"};
-  static constexpr StringRef KnownTextProtoPrefixes[] = {"//", "#", "##", "###",
- ""};
+  "//!",  "//:",  "//"};
+  static constexpr StringRef KnownTextProtoPrefixes[] = {"", "###", "##",
+ "//", "#"};
+
+
   ArrayRef KnownPrefixes(KnownCStylePrefixes);
   if (Style.Language == FormatStyle::LK_TextProto)
 KnownPrefixes = KnownTextProtoPrefixes;
 
-  StringRef LongestPrefix;
+  assert(std::is_sorted(KnownPrefixes.begin(), KnownPrefixes.end(),
+[](StringRef Lhs, StringRef Rhs) noexcept {
+  return Lhs.size() > Rhs.size();
+}));
+
   for (StringRef KnownPrefix : KnownPrefixes) {
 if (Comment.startswith(KnownPrefix)) {
-  size_t PrefixLength = KnownPrefix.size();
-  while (PrefixLength < Comment.size() && Comment[PrefixLength] == ' ')
-++PrefixLength;
-  if (PrefixLength > LongestPrefix.size())
-LongestPrefix = Comment.substr(0, PrefixLength);
+  const auto PrefixLength =
+  Comment.find_first_not_of(' ', KnownPrefix.size());
+  return Comment.substr(0, PrefixLength);
 }
   }
-  return LongestPrefix;
+  return {};
 }
 
 static BreakableToken::Split
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/BreakableToken.cpp:44
 const FormatStyle ) {
+  // Keep these prefixes sorted after length.
   static constexpr StringRef KnownCStylePrefixes[] = {"///<", "//!<", "///",

I'd rather put asserts instead of this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95081

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

[PATCH] D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix

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

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When sorting the known prefixes after length the if in the loop will hit at 
most once, so we can return from there.

Also replace the inner loop with an algorithm, that makes it more readable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95081

Files:
  clang/lib/Format/BreakableToken.cpp


Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -41,25 +41,23 @@
 
 static StringRef getLineCommentIndentPrefix(StringRef Comment,
 const FormatStyle ) {
+  // Keep these prefixes sorted after length.
   static constexpr StringRef KnownCStylePrefixes[] = {"///<", "//!<", "///",
-  "//",   "//!",  "//:"};
-  static constexpr StringRef KnownTextProtoPrefixes[] = {"//", "#", "##", 
"###",
- ""};
+  "//!",  "//:",  "//"};
+  static constexpr StringRef KnownTextProtoPrefixes[] = {"//", "", "###",
+ "##", "#"};
   ArrayRef KnownPrefixes(KnownCStylePrefixes);
   if (Style.Language == FormatStyle::LK_TextProto)
 KnownPrefixes = KnownTextProtoPrefixes;
 
-  StringRef LongestPrefix;
   for (StringRef KnownPrefix : KnownPrefixes) {
 if (Comment.startswith(KnownPrefix)) {
-  size_t PrefixLength = KnownPrefix.size();
-  while (PrefixLength < Comment.size() && Comment[PrefixLength] == ' ')
-++PrefixLength;
-  if (PrefixLength > LongestPrefix.size())
-LongestPrefix = Comment.substr(0, PrefixLength);
+  const auto PrefixLength =
+  Comment.find_first_not_of(' ', KnownPrefix.size());
+  return Comment.substr(0, PrefixLength);
 }
   }
-  return LongestPrefix;
+  return {};
 }
 
 static BreakableToken::Split


Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -41,25 +41,23 @@
 
 static StringRef getLineCommentIndentPrefix(StringRef Comment,
 const FormatStyle ) {
+  // Keep these prefixes sorted after length.
   static constexpr StringRef KnownCStylePrefixes[] = {"///<", "//!<", "///",
-  "//",   "//!",  "//:"};
-  static constexpr StringRef KnownTextProtoPrefixes[] = {"//", "#", "##", "###",
- ""};
+  "//!",  "//:",  "//"};
+  static constexpr StringRef KnownTextProtoPrefixes[] = {"//", "", "###",
+ "##", "#"};
   ArrayRef KnownPrefixes(KnownCStylePrefixes);
   if (Style.Language == FormatStyle::LK_TextProto)
 KnownPrefixes = KnownTextProtoPrefixes;
 
-  StringRef LongestPrefix;
   for (StringRef KnownPrefix : KnownPrefixes) {
 if (Comment.startswith(KnownPrefix)) {
-  size_t PrefixLength = KnownPrefix.size();
-  while (PrefixLength < Comment.size() && Comment[PrefixLength] == ' ')
-++PrefixLength;
-  if (PrefixLength > LongestPrefix.size())
-LongestPrefix = Comment.substr(0, PrefixLength);
+  const auto PrefixLength =
+  Comment.find_first_not_of(' ', KnownPrefix.size());
+  return Comment.substr(0, PrefixLength);
 }
   }
-  return LongestPrefix;
+  return {};
 }
 
 static BreakableToken::Split
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits