[libc-commits] [clang-tools-extra] [libc] clang-tidy: readability-redundant-smartptr-get does not remove -> (#97964) (PR #98757)
via libc-commits
libc-commits at lists.llvm.org
Wed Jul 17 10:53:51 PDT 2024
https://github.com/akshaykumars614 updated https://github.com/llvm/llvm-project/pull/98757
>From 283ec53fe19f0008c3c04210ea5c9b20c3d9781c Mon Sep 17 00:00:00 2001
From: akshaykumars614 <akshaykumars614 at gmail.com>
Date: Sat, 13 Jul 2024 14:14:53 -0400
Subject: [PATCH 1/6] clang-tidy: readability-redundant-smartptr-get does not
remove -> (#97964)
added a check to remove '->' if exists
---
.../clang-tidy/readability/RedundantSmartptrGetCheck.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
index 8837ac16e8828..be52af77ae0a5 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -164,6 +164,10 @@ void RedundantSmartptrGetCheck::check(const MatchFinder::MatchResult &Result) {
StringRef SmartptrText = Lexer::getSourceText(
CharSourceRange::getTokenRange(Smartptr->getSourceRange()),
*Result.SourceManager, getLangOpts());
+ // Check if the last two characters are "->" and remove them
+ if (SmartptrText.ends_with("->")) {
+ SmartptrText = SmartptrText.drop_back(2);
+ }
// Replace foo->get() with *foo, and foo.get() with foo.
std::string Replacement = Twine(IsPtrToPtr ? "*" : "", SmartptrText).str();
diag(GetCall->getBeginLoc(), "redundant get() call on smart pointer")
>From 1c0a96b23bfe407b12eadd76d264f44a43b99630 Mon Sep 17 00:00:00 2001
From: Petr Hosek <phosek at google.com>
Date: Sat, 13 Jul 2024 11:23:32 -0700
Subject: [PATCH 2/6] [libc] Remove src/errno/errno.h (#98759)
This addresses the build error introduced in #98287 where
src/errno/errno.h is included instead of the system errno.h.
We instead move the declaration to libc_errno.h.
---
libc/src/errno/CMakeLists.txt | 1 -
libc/src/errno/errno.h | 14 --------------
libc/src/errno/libc_errno.cpp | 15 +++++----------
libc/src/errno/libc_errno.h | 2 ++
4 files changed, 7 insertions(+), 25 deletions(-)
delete mode 100644 libc/src/errno/errno.h
diff --git a/libc/src/errno/CMakeLists.txt b/libc/src/errno/CMakeLists.txt
index b05fd4e31ff68..1d78a5eedff96 100644
--- a/libc/src/errno/CMakeLists.txt
+++ b/libc/src/errno/CMakeLists.txt
@@ -18,7 +18,6 @@ add_entrypoint_object(
SRCS
libc_errno.cpp
HDRS
- errno.h
libc_errno.h # Include this
COMPILE_OPTIONS
${full_build_flag}
diff --git a/libc/src/errno/errno.h b/libc/src/errno/errno.h
deleted file mode 100644
index a2df93513ec62..0000000000000
--- a/libc/src/errno/errno.h
+++ /dev/null
@@ -1,14 +0,0 @@
-//===-- Implementation header for errno -------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_LIBC_SRC_ERRNO_ERRNO_H
-#define LLVM_LIBC_SRC_ERRNO_ERRNO_H
-
-extern "C" int *__llvm_libc_errno();
-
-#endif // LLVM_LIBC_SRC_ERRNO_ERRNO_H
diff --git a/libc/src/errno/libc_errno.cpp b/libc/src/errno/libc_errno.cpp
index f7bd3a3b9eb4b..7b28a62c786b0 100644
--- a/libc/src/errno/libc_errno.cpp
+++ b/libc/src/errno/libc_errno.cpp
@@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//
#include "libc_errno.h"
-#include "src/errno/errno.h"
#include "src/__support/macros/config.h"
// libc never stores a value; `errno` macro uses get link-time failure.
@@ -47,9 +46,6 @@ LIBC_ERRNO_MODE_SYSTEM
namespace LIBC_NAMESPACE_DECL {
-// Define the global `libc_errno` instance.
-Errno libc_errno;
-
#if LIBC_ERRNO_MODE == LIBC_ERRNO_MODE_UNDEFINED
void Errno::operator=(int) {}
@@ -61,9 +57,7 @@ namespace {
LIBC_THREAD_LOCAL int thread_errno;
}
-extern "C" {
-int *__llvm_libc_errno() { return &thread_errno; }
-}
+extern "C" int *__llvm_libc_errno() { return &thread_errno; }
void Errno::operator=(int a) { thread_errno = a; }
Errno::operator int() { return thread_errno; }
@@ -74,9 +68,7 @@ namespace {
int shared_errno;
}
-extern "C" {
-int *__llvm_libc_errno() { return &shared_errno; }
-}
+extern "C" int *__llvm_libc_errno() { return &shared_errno; }
void Errno::operator=(int a) { shared_errno = a; }
Errno::operator int() { return shared_errno; }
@@ -93,4 +85,7 @@ Errno::operator int() { return errno; }
#endif
+// Define the global `libc_errno` instance.
+Errno libc_errno;
+
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/errno/libc_errno.h b/libc/src/errno/libc_errno.h
index c6c6b20b48251..82c65f5a0b7f6 100644
--- a/libc/src/errno/libc_errno.h
+++ b/libc/src/errno/libc_errno.h
@@ -33,6 +33,8 @@
namespace LIBC_NAMESPACE_DECL {
+extern "C" int *__llvm_libc_errno();
+
struct Errno {
void operator=(int);
operator int();
>From 984de29a776bcf6c760fc5a036396bb865efcb24 Mon Sep 17 00:00:00 2001
From: Petr Hosek <phosek at google.com>
Date: Sat, 13 Jul 2024 11:29:12 -0700
Subject: [PATCH 3/6] [libc] Mark internal __llvm_libc_errno as noexcept
(#98760)
The declaration must match the previous declaration in errno.h.
---
libc/src/errno/libc_errno.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libc/src/errno/libc_errno.h b/libc/src/errno/libc_errno.h
index 82c65f5a0b7f6..44ee2714843ba 100644
--- a/libc/src/errno/libc_errno.h
+++ b/libc/src/errno/libc_errno.h
@@ -33,7 +33,7 @@
namespace LIBC_NAMESPACE_DECL {
-extern "C" int *__llvm_libc_errno();
+extern "C" int *__llvm_libc_errno() noexcept;
struct Errno {
void operator=(int);
>From b95f7b81c0368b49855e27bcb62dae17643f9f57 Mon Sep 17 00:00:00 2001
From: Petr Hosek <phosek at google.com>
Date: Sat, 13 Jul 2024 11:40:33 -0700
Subject: [PATCH 4/6] [libc] Mark all __llvm_libc_errno definitions as noexcept
(#98762)
The definitions must match the previous declaration in errno.h.
---
libc/src/errno/libc_errno.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libc/src/errno/libc_errno.cpp b/libc/src/errno/libc_errno.cpp
index 7b28a62c786b0..7a17a5a8217c7 100644
--- a/libc/src/errno/libc_errno.cpp
+++ b/libc/src/errno/libc_errno.cpp
@@ -57,7 +57,7 @@ namespace {
LIBC_THREAD_LOCAL int thread_errno;
}
-extern "C" int *__llvm_libc_errno() { return &thread_errno; }
+extern "C" int *__llvm_libc_errno() noexcept { return &thread_errno; }
void Errno::operator=(int a) { thread_errno = a; }
Errno::operator int() { return thread_errno; }
@@ -68,7 +68,7 @@ namespace {
int shared_errno;
}
-extern "C" int *__llvm_libc_errno() { return &shared_errno; }
+extern "C" int *__llvm_libc_errno() noexcept { return &shared_errno; }
void Errno::operator=(int a) { shared_errno = a; }
Errno::operator int() { return shared_errno; }
>From d8d4de2e5b942459d6057fb2d77ebab9a0cbe229 Mon Sep 17 00:00:00 2001
From: akshaykumars614 <akshaykumars614 at gmail.com>
Date: Mon, 15 Jul 2024 12:56:16 -0400
Subject: [PATCH 5/6] clang-tidy: readability-redundant-smartptr-get does not
remove (#97964)
Added test case and updated Release Notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 6 +++
.../readability/redundant-smartptr-get.cpp | 51 +++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c1fa502534ea5..ef895b866a5b1 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -502,6 +502,12 @@ Changes in existing checks
usages of ``std::string_view::compare``. Added a `StringLikeClasses` option
to detect usages of ``compare`` method in custom string-like classes.
+- Improved :doc:`readability-redundant-smartptr-get
+ <clang-tidy/checks/readability/readability-redundant-smartptr-get>` identify
+ and fix redundant calls to the `get()` method on smart pointers. This
+ improves code readability and efficiency by removing unnecessary
+ dereferences.
+
Removed checks
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp
index 01f12b6bfe6ea..95ec583500a5c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp
@@ -1,5 +1,6 @@
// RUN: %check_clang_tidy %s readability-redundant-smartptr-get %t
+
#define NULL __null
namespace std {
@@ -20,6 +21,46 @@ struct shared_ptr {
explicit operator bool() const noexcept;
};
+template <typename T>
+struct vector {
+ vector();
+ bool operator==(const vector<T>& other) const;
+ bool operator!=(const vector<T>& other) const;
+ unsigned long size() const;
+ bool empty() const;
+
+ // Basic iterator implementation for testing
+ struct iterator {
+ T* ptr;
+ iterator(T* p) : ptr(p) {}
+ T& operator*() { return *ptr; }
+ T* operator->() { return ptr; }
+ iterator& operator++() {
+ ++ptr;
+ return *this;
+ }
+ bool operator!=(const iterator& other) const { return ptr != other.ptr; }
+ };
+
+ iterator begin();
+ iterator end();
+
+ T* data;
+ unsigned long sz;
+};
+
+template <typename T>
+vector<T>::vector() : data(nullptr), sz(0) {}
+
+template <typename T>
+typename vector<T>::iterator vector<T>::begin() {
+ return iterator(data);
+}
+
+template <typename T>
+typename vector<T>::iterator vector<T>::end() {
+ return iterator(data + sz);
+}
} // namespace std
struct Bar {
@@ -235,3 +276,13 @@ void Negative() {
if (MACRO(x) == nullptr)
;
}
+
+void test_redundant_get() {
+ std::vector<std::shared_ptr<int>> v;
+ auto f = [](int) {};
+ for (auto i = v.begin(); i != v.end(); ++i) {
+ f(*i->get());
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-FIXES: f(**i);
+ }
+}
>From 8f4524f0bc9b072f26717147ebca7c0b9abf2cb0 Mon Sep 17 00:00:00 2001
From: akshaykumars614 <akshaykumars614 at gmail.com>
Date: Wed, 17 Jul 2024 13:52:01 -0400
Subject: [PATCH 6/6] clang-tidy: readability-redundant-smartptr-get does not
remove (#97964)
Added test case and updated Release Notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ef895b866a5b1..71afec40a5105 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -482,6 +482,11 @@ Changes in existing checks
false-positives when type of the member does not match the type of the
initializer.
+- Improved :doc:`readability-redundant-smartptr-get
+ <clang-tidy/checks/readability/readability-redundant-smartptr-get>` check to
+ improves code readability and efficiency by removing unnecessary
+ dereferences.
+
- Improved :doc:`readability-static-accessed-through-instance
<clang-tidy/checks/readability/static-accessed-through-instance>` check to
support calls to overloaded operators as base expression and provide fixes to
More information about the libc-commits
mailing list