Skip to content
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

[libc] Add VISIT enum for search.h #132746

Merged
merged 1 commit into from
Mar 26, 2025
Merged

[libc] Add VISIT enum for search.h #132746

merged 1 commit into from
Mar 26, 2025

Conversation

c8ef
Copy link
Contributor

@c8ef c8ef commented Mar 24, 2025

This patch introduces the VISIT enum for tree search. Existing tests ensure the correct generation of headers.

@c8ef c8ef changed the title Draft [libc] Add VISIT enum for search.h Mar 24, 2025
@c8ef c8ef marked this pull request as ready for review March 24, 2025 14:43
@llvmbot llvmbot added the libc label Mar 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 24, 2025

@llvm/pr-subscribers-libc

Author: Connector Switch (c8ef)

Changes

This patch introduces the VISIT enum for tree search. Existing tests ensure the correct generation of headers.


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

4 Files Affected:

  • (modified) libc/include/CMakeLists.txt (+1)
  • (modified) libc/include/llvm-libc-types/CMakeLists.txt (+1)
  • (added) libc/include/llvm-libc-types/VISIT.h (+14)
  • (modified) libc/include/search.yaml (+1)
diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 409737762ac41..d1e116ac547d7 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -245,6 +245,7 @@ add_header_macro(
     .llvm-libc-types.ENTRY
     .llvm-libc-types.struct_hsearch_data
     .llvm-libc-types.size_t
+    .llvm-libc-types.VISIT
     .llvm-libc-types.__lsearchcompare_t
 )
 
diff --git a/libc/include/llvm-libc-types/CMakeLists.txt b/libc/include/llvm-libc-types/CMakeLists.txt
index bf8bdfe89943c..66e8527701873 100644
--- a/libc/include/llvm-libc-types/CMakeLists.txt
+++ b/libc/include/llvm-libc-types/CMakeLists.txt
@@ -128,6 +128,7 @@ add_header(struct_iovec HDR struct_iovec.h DEPENDS .size_t)
 add_header(struct_msghdr HDR struct_msghdr.h DEPENDS .size_t .socklen_t .struct_iovec)
 add_header(ACTION HDR ACTION.h)
 add_header(ENTRY HDR ENTRY.h)
+add_header(VISIT HDR VISIT.h)
 add_header(struct_hsearch_data HDR struct_hsearch_data.h)
 add_header(struct_epoll_event HDR struct_epoll_event.h)
 add_header(struct_epoll_data HDR struct_epoll_data.h)
diff --git a/libc/include/llvm-libc-types/VISIT.h b/libc/include/llvm-libc-types/VISIT.h
new file mode 100644
index 0000000000000..0a7ff6e50d469
--- /dev/null
+++ b/libc/include/llvm-libc-types/VISIT.h
@@ -0,0 +1,14 @@
+//===-- Definition of VISIT type ------------------------------------------===//
+//
+// 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_TYPES_VISIT_H
+#define LLVM_LIBC_TYPES_VISIT_H
+
+typedef enum { preorder, postorder, endorder, leaf } VISIT;
+
+#endif // LLVM_LIBC_TYPES_VISIT_H
diff --git a/libc/include/search.yaml b/libc/include/search.yaml
index f6f5d6cb062e5..e2e711cc93f4d 100644
--- a/libc/include/search.yaml
+++ b/libc/include/search.yaml
@@ -5,6 +5,7 @@ types:
   - type_name: struct_hsearch_data
   - type_name: ENTRY
   - type_name: ACTION
+  - type_name: VISIT
   - type_name: __lsearchcompare_t
 enums: []
 objects: []

@c8ef
Copy link
Contributor Author

c8ef commented Mar 25, 2025

Thanks for the review!

The only thing left for search.h is the tree-search related functions. I’d like to work on that, but first, I have a few questions about the basic design choices, specifically the data structure. As far as I can tell, glibc uses a red-black tree, while musl and FreeBSD use AVL trees. There are other tree structures that could be suitable as well, like AA trees. Which one would be more appropriate here? I’m wondering if any of them would work, or if there’s a clear preference based on benchmarks or specific use cases.

@michaelrj-google
Copy link
Contributor

Any of them would work. From what I can tell, the tsearch family of functions isn't widely used so it's not really performance critical.

@c8ef c8ef merged commit 7a5ce55 into llvm:main Mar 26, 2025
20 checks passed
@c8ef c8ef deleted the visit branch March 26, 2025 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants