Skip to content

Apply modernize-use-starts-ends-with on llvm-project #89140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

nicovank
Copy link
Contributor

Run modernize-use-starts-ends-with on llvm-project. Two instances are flagged, minor readability improvements, extremely minor performance improvements.

python3 clang-tools-extra/clang-tidy/tool/run-clang-tidy.py \
    -clang-tidy-binary="build/bin/clang-tidy" \
    -clang-apply-replacements-binary="build/bin/clang-apply-replacements" \
    -checks="-*,modernize-use-starts-ends-with" \
    -header-filter=".*" \
    -fix -format

I am working on some additions to this check, but they don't seem to flag any additional cases anyway.

@llvmbot
Copy link
Member

llvmbot commented Apr 17, 2024

@llvm/pr-subscribers-lldb

@llvm/pr-subscribers-clang-tools-extra

Author: Nicolas van Kempen (nicovank)

Changes

Run modernize-use-starts-ends-with on llvm-project. Two instances are flagged, minor readability improvements, extremely minor performance improvements.

python3 clang-tools-extra/clang-tidy/tool/run-clang-tidy.py \
    -clang-tidy-binary="build/bin/clang-tidy" \
    -clang-apply-replacements-binary="build/bin/clang-apply-replacements" \
    -checks="-*,modernize-use-starts-ends-with" \
    -header-filter=".*" \
    -fix -format

I am working on some additions to this check, but they don't seem to flag any additional cases anyway.


Full diff: https://github.com/llvm/llvm-project/pull/89140.diff

2 Files Affected:

  • (modified) clang-tools-extra/clang-doc/Representation.cpp (+2-2)
  • (modified) llvm/unittests/Support/VirtualFileSystemTest.cpp (+1-1)
diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp
index 84233c36e15d98..2afff2929cf79c 100644
--- a/clang-tools-extra/clang-doc/Representation.cpp
+++ b/clang-tools-extra/clang-doc/Representation.cpp
@@ -380,8 +380,8 @@ ClangDocContext::ClangDocContext(tooling::ExecutionContext *ECtx,
   this->SourceRoot = std::string(SourceRootDir);
   if (!RepositoryUrl.empty()) {
     this->RepositoryUrl = std::string(RepositoryUrl);
-    if (!RepositoryUrl.empty() && RepositoryUrl.find("http://") != 0 &&
-        RepositoryUrl.find("https://") != 0)
+    if (!RepositoryUrl.empty() && !RepositoryUrl.starts_with("http://") &&
+        !RepositoryUrl.starts_with("https://"))
       this->RepositoryUrl->insert(0, "https://");
   }
 }
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index e9b4ac3d92e1dd..e9fd9671ea6ab5 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -101,7 +101,7 @@ class DummyFileSystem : public vfs::FileSystem {
     std::map<std::string, vfs::Status>::iterator I;
     std::string Path;
     bool isInPath(StringRef S) {
-      if (Path.size() < S.size() && S.find(Path) == 0) {
+      if (Path.size() < S.size() && S.starts_with(Path)) {
         auto LastSep = S.find_last_of('/');
         if (LastSep == Path.size() || LastSep == Path.size() - 1)
           return true;

@llvmbot
Copy link
Member

llvmbot commented Apr 17, 2024

@llvm/pr-subscribers-llvm-support

Author: Nicolas van Kempen (nicovank)

Changes

Run modernize-use-starts-ends-with on llvm-project. Two instances are flagged, minor readability improvements, extremely minor performance improvements.

python3 clang-tools-extra/clang-tidy/tool/run-clang-tidy.py \
    -clang-tidy-binary="build/bin/clang-tidy" \
    -clang-apply-replacements-binary="build/bin/clang-apply-replacements" \
    -checks="-*,modernize-use-starts-ends-with" \
    -header-filter=".*" \
    -fix -format

I am working on some additions to this check, but they don't seem to flag any additional cases anyway.


Full diff: https://github.com/llvm/llvm-project/pull/89140.diff

2 Files Affected:

  • (modified) clang-tools-extra/clang-doc/Representation.cpp (+2-2)
  • (modified) llvm/unittests/Support/VirtualFileSystemTest.cpp (+1-1)
diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp
index 84233c36e15d98..2afff2929cf79c 100644
--- a/clang-tools-extra/clang-doc/Representation.cpp
+++ b/clang-tools-extra/clang-doc/Representation.cpp
@@ -380,8 +380,8 @@ ClangDocContext::ClangDocContext(tooling::ExecutionContext *ECtx,
   this->SourceRoot = std::string(SourceRootDir);
   if (!RepositoryUrl.empty()) {
     this->RepositoryUrl = std::string(RepositoryUrl);
-    if (!RepositoryUrl.empty() && RepositoryUrl.find("http://") != 0 &&
-        RepositoryUrl.find("https://") != 0)
+    if (!RepositoryUrl.empty() && !RepositoryUrl.starts_with("http://") &&
+        !RepositoryUrl.starts_with("https://"))
       this->RepositoryUrl->insert(0, "https://");
   }
 }
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index e9b4ac3d92e1dd..e9fd9671ea6ab5 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -101,7 +101,7 @@ class DummyFileSystem : public vfs::FileSystem {
     std::map<std::string, vfs::Status>::iterator I;
     std::string Path;
     bool isInPath(StringRef S) {
-      if (Path.size() < S.size() && S.find(Path) == 0) {
+      if (Path.size() < S.size() && S.starts_with(Path)) {
         auto LastSep = S.find_last_of('/');
         if (LastSep == Path.size() || LastSep == Path.size() - 1)
           return true;

@nicovank
Copy link
Contributor Author

std::string::starts_with is only available from C++20, and LLVM is limited to C++17 as far as I know. However, the two objects here are StringRef so this is safe.

@nicovank nicovank force-pushed the apply-modernize-use-starts-ends-with branch from 4f28ecc to 2c82316 Compare April 19, 2024 19:54
@nicovank nicovank requested a review from JDevlieghere as a code owner April 19, 2024 19:54
@llvmbot llvmbot added the lldb label Apr 19, 2024
@nicovank
Copy link
Contributor Author

Added one missed instance due to CMake configuration options.

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LLDB & VFS change LGTM.

@nicovank
Copy link
Contributor Author

Maybe @kazutakahirata (or anyone else) can stamp the minor clang-doc change?

I don't have commit access, feel free to hit merge when everything is in order 👍 .

@JDevlieghere JDevlieghere merged commit 5232cec into llvm:main Apr 19, 2024
@nicovank nicovank deleted the apply-modernize-use-starts-ends-with branch April 19, 2024 22:30
aniplcc pushed a commit to aniplcc/llvm-project that referenced this pull request Apr 21, 2024
Run `modernize-use-starts-ends-with` on llvm-project. Two instances are
flagged, minor readability improvements, extremely minor performance
improvements.

```
python3 clang-tools-extra/clang-tidy/tool/run-clang-tidy.py \
    -clang-tidy-binary="build/bin/clang-tidy" \
    -clang-apply-replacements-binary="build/bin/clang-apply-replacements" \
    -checks="-*,modernize-use-starts-ends-with" \
    -header-filter=".*" \
    -fix -format
```

I am working on some additions to this check, but they don't seem to
flag any additional cases anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants