[PATCH] D66788: Refactor GlobList from an ad-hoc linked list to a vector

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 27 04:02:26 PDT 2019


gribozavr marked an inline comment as done.
gribozavr added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/GlobList.cpp:46
+GlobList::GlobList(StringRef Globs) {
+  do {
+    GlobListItem Item;
----------------
ilya-biryukov wrote:
> NIT: I suggest using `for (;!Globs.empty();) {}` to make the stop condition stand out more.
> Not a big deal, though, feel free to keep as is
It is actually important for the algorithm to use a do-while loop... The linked-list-based code always parsed at least one glob, even if the input string is empty. Therefore, new loop-based code should also parse at least one glob.

I added a doc comment:

```
+  /// An empty \p Globs string is interpreted as one glob that matches an empty
+  /// string.
   GlobList(StringRef Globs);
```

Please let me know if that is not sufficient. There are also tests for this behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66788





More information about the cfe-commits mailing list