Skip to content

[libc] implement strings/str{n}casecmp_l #130407

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
merged 6 commits into from
Mar 12, 2025
Merged

[libc] implement strings/str{n}casecmp_l #130407

merged 6 commits into from
Mar 12, 2025

Conversation

c8ef
Copy link
Contributor

@c8ef c8ef commented Mar 8, 2025

ref: https://pubs.opengroup.org/onlinepubs/9799919799/functions/strcasecmp_l.html

This patch introduces the strcasecmp_l function. At present, the locale parameter is ignored, making it a stub implementation. This is consistent with how other locale-related functions, such as islower_l, are treated in our codebase as well as in musl and bionic.

c8ef added 3 commits March 8, 2025 10:02
.
@c8ef c8ef changed the title Draft [libc] implement strings/str{n}casecmp_l Mar 8, 2025
@c8ef c8ef marked this pull request as ready for review March 8, 2025 11:30
@llvmbot llvmbot added the libc label Mar 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 8, 2025

@llvm/pr-subscribers-libc

Author: Connector Switch (c8ef)

Changes

ref: https://pubs.opengroup.org/onlinepubs/9799919799/functions/strcasecmp_l.html

This patch introduces the strcasecmp_l function. At present, the locale parameter is ignored, making it a stub implementation. This is consistent with how other locale-related functions, such as islower_l, are treated in our codebase as well as in musl and bionic.


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

11 Files Affected:

  • (modified) libc/config/linux/aarch64/entrypoints.txt (+4)
  • (modified) libc/config/linux/x86_64/entrypoints.txt (+4)
  • (modified) libc/include/strings.yaml (+18)
  • (modified) libc/src/strings/CMakeLists.txt (+24)
  • (added) libc/src/strings/strcasecmp_l.cpp (+27)
  • (added) libc/src/strings/strcasecmp_l.h (+21)
  • (added) libc/src/strings/strncasecmp_l.cpp (+27)
  • (added) libc/src/strings/strncasecmp_l.h (+23)
  • (modified) libc/test/src/strings/CMakeLists.txt (+26)
  • (added) libc/test/src/strings/strcasecmp_l_test.cpp (+21)
  • (added) libc/test/src/strings/strncasecmp_l_test.cpp (+22)
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index c7beb3ef3fdfc..ab1917259519b 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -911,6 +911,10 @@ if(LLVM_LIBC_FULL_BUILD)
     # sched.h entrypoints
     libc.src.sched.__sched_getcpucount
 
+    # strings.h entrypoints
+    libc.src.strings.strcasecmp_l
+    libc.src.strings.strncasecmp_l
+
     # setjmp.h entrypoints
     libc.src.setjmp.longjmp
     libc.src.setjmp.setjmp
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 12dc87bf945fd..a29478898fe70 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -938,6 +938,10 @@ if(LLVM_LIBC_FULL_BUILD)
     libc.src.string.strcoll_l
     libc.src.string.strxfrm_l
 
+    # strings.h entrypoints
+    libc.src.strings.strcasecmp_l
+    libc.src.strings.strncasecmp_l
+
     # assert.h entrypoints
     libc.src.assert.__assert_fail
 
diff --git a/libc/include/strings.yaml b/libc/include/strings.yaml
index 802f6533585f8..855800d9dbc3d 100644
--- a/libc/include/strings.yaml
+++ b/libc/include/strings.yaml
@@ -3,6 +3,7 @@ header_template: strings.h.def
 macros: []
 types:
   - type_name: size_t
+  - type_name: locale_t
 enums: []
 objects: []
 functions:
@@ -68,6 +69,14 @@ functions:
     arguments:
       - type: const char *
       - type: const char *
+  - name: strcasecmp_l
+    standards:
+      - BSDExtensions
+    return_type: int
+    arguments:
+      - type: const char *
+      - type: const char *
+      - type: locale_t
   - name: strncasecmp
     standards:
       - BSDExtensions
@@ -76,3 +85,12 @@ functions:
       - type: const char *
       - type: const char *
       - type: size_t
+  - name: strncasecmp_l
+    standards:
+      - BSDExtensions
+    return_type: int
+    arguments:
+      - type: const char *
+      - type: const char *
+      - type: size_t
+      - type: locale_t
diff --git a/libc/src/strings/CMakeLists.txt b/libc/src/strings/CMakeLists.txt
index 6d86680e8e71f..d312495ae8d91 100644
--- a/libc/src/strings/CMakeLists.txt
+++ b/libc/src/strings/CMakeLists.txt
@@ -115,6 +115,18 @@ add_entrypoint_object(
     libc.src.string.memory_utils.inline_strcmp
 )
 
+add_entrypoint_object(
+  strcasecmp_l
+  SRCS
+    strcasecmp_l.cpp
+  HDRS
+    strcasecmp_l.h
+  DEPENDS
+    libc.hdr.types.locale_t
+    libc.src.__support.ctype_utils
+    libc.src.string.memory_utils.inline_strcmp
+)
+
 add_entrypoint_object(
   strncasecmp
   SRCS
@@ -125,3 +137,15 @@ add_entrypoint_object(
     libc.src.__support.ctype_utils
     libc.src.string.memory_utils.inline_strcmp
 )
+
+add_entrypoint_object(
+  strncasecmp_l
+  SRCS
+    strncasecmp_l.cpp
+  HDRS
+    strncasecmp_l.h
+  DEPENDS
+    libc.hdr.types.locale_t
+    libc.src.__support.ctype_utils
+    libc.src.string.memory_utils.inline_strcmp
+)
diff --git a/libc/src/strings/strcasecmp_l.cpp b/libc/src/strings/strcasecmp_l.cpp
new file mode 100644
index 0000000000000..95117cb27a564
--- /dev/null
+++ b/libc/src/strings/strcasecmp_l.cpp
@@ -0,0 +1,27 @@
+//===-- Implementation of strcasecmp_l ------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/strings/strcasecmp_l.h"
+
+#include "src/__support/common.h"
+#include "src/__support/ctype_utils.h"
+#include "src/__support/macros/config.h"
+#include "src/string/memory_utils/inline_strcmp.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, strcasecmp_l,
+                   (const char *left, const char *right, locale_t)) {
+  auto case_cmp = [](char a, char b) {
+    return LIBC_NAMESPACE::internal::tolower(a) -
+           LIBC_NAMESPACE::internal::tolower(b);
+  };
+  return inline_strcmp(left, right, case_cmp);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/strings/strcasecmp_l.h b/libc/src/strings/strcasecmp_l.h
new file mode 100644
index 0000000000000..48074460205ce
--- /dev/null
+++ b/libc/src/strings/strcasecmp_l.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for strcasecmp_l ------------------*- 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_STRINGS_STRCASECMP_L_H
+#define LLVM_LIBC_SRC_STRINGS_STRCASECMP_L_H
+
+#include "hdr/types/locale_t.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+int strcasecmp_l(const char *left, const char *right, locale_t locale);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STRINGS_STRCASECMP_L_H
diff --git a/libc/src/strings/strncasecmp_l.cpp b/libc/src/strings/strncasecmp_l.cpp
new file mode 100644
index 0000000000000..91ac7e5e89107
--- /dev/null
+++ b/libc/src/strings/strncasecmp_l.cpp
@@ -0,0 +1,27 @@
+//===-- Implementation of strncasecmp_l -----------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/strings/strncasecmp_l.h"
+
+#include "src/__support/common.h"
+#include "src/__support/ctype_utils.h"
+#include "src/__support/macros/config.h"
+#include "src/string/memory_utils/inline_strcmp.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, strncasecmp_l,
+                   (const char *left, const char *right, size_t n, locale_t)) {
+  auto case_cmp = [](char a, char b) {
+    return LIBC_NAMESPACE::internal::tolower(a) -
+           LIBC_NAMESPACE::internal::tolower(b);
+  };
+  return inline_strncmp(left, right, n, case_cmp);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/strings/strncasecmp_l.h b/libc/src/strings/strncasecmp_l.h
new file mode 100644
index 0000000000000..598eab879f884
--- /dev/null
+++ b/libc/src/strings/strncasecmp_l.h
@@ -0,0 +1,23 @@
+//===-- Implementation header for strcasecmp_l ------------------*- 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_STRINGS_STRNCASECMP_L_H
+#define LLVM_LIBC_SRC_STRINGS_STRNCASECMP_L_H
+
+#include "hdr/types/locale_t.h"
+#include "src/__support/macros/config.h"
+#include <stddef.h>
+
+namespace LIBC_NAMESPACE_DECL {
+
+int strncasecmp_l(const char *left, const char *right, size_t n,
+                  locale_t locale);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STRINGS_STRNCASECMP_L_H
diff --git a/libc/test/src/strings/CMakeLists.txt b/libc/test/src/strings/CMakeLists.txt
index baa22bb449c6c..a1bf5e82d7ea8 100644
--- a/libc/test/src/strings/CMakeLists.txt
+++ b/libc/test/src/strings/CMakeLists.txt
@@ -74,6 +74,19 @@ add_libc_test(
     libc.src.strings.strcasecmp
 )
 
+add_libc_test(
+  strcasecmp_l_test
+  SUITE
+    libc-strings-tests
+  SRCS
+    strcasecmp_l_test.cpp
+  DEPENDS
+    libc.include.locale
+    libc.src.locale.freelocale
+    libc.src.locale.newlocale
+    libc.src.strings.strcasecmp_l
+)
+
 add_libc_test(
   strncasecmp_test
   SUITE
@@ -84,5 +97,18 @@ add_libc_test(
     libc.src.strings.strncasecmp
 )
 
+add_libc_test(
+  strncasecmp_l_test
+  SUITE
+    libc-strings-tests
+  SRCS
+    strncasecmp_l_test.cpp
+  DEPENDS
+    libc.include.locale
+    libc.src.locale.freelocale
+    libc.src.locale.newlocale
+    libc.src.strings.strncasecmp_l
+)
+
 add_libc_multi_impl_test(bcmp libc-strings-tests SRCS bcmp_test.cpp)
 add_libc_multi_impl_test(bzero libc-strings-tests SRCS bzero_test.cpp)
diff --git a/libc/test/src/strings/strcasecmp_l_test.cpp b/libc/test/src/strings/strcasecmp_l_test.cpp
new file mode 100644
index 0000000000000..8eca6b51d0cee
--- /dev/null
+++ b/libc/test/src/strings/strcasecmp_l_test.cpp
@@ -0,0 +1,21 @@
+//===-- Unittests for strcasecmp_l ----------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "include/llvm-libc-macros/locale-macros.h"
+#include "src/locale/freelocale.h"
+#include "src/locale/newlocale.h"
+#include "src/strings/strcasecmp_l.h"
+#include "test/UnitTest/Test.h"
+
+TEST(LlvmLibcStrCaseCmpLTest, Case) {
+  locale_t locale = LIBC_NAMESPACE::newlocale(LC_ALL, "C", nullptr);
+  ASSERT_EQ(LIBC_NAMESPACE::strcasecmp_l("hello", "HELLO", locale), 0);
+  ASSERT_LT(LIBC_NAMESPACE::strcasecmp_l("hello1", "hello2", locale), 0);
+  ASSERT_GT(LIBC_NAMESPACE::strcasecmp_l("hello2", "hello1", locale), 0);
+  LIBC_NAMESPACE::freelocale(locale);
+}
diff --git a/libc/test/src/strings/strncasecmp_l_test.cpp b/libc/test/src/strings/strncasecmp_l_test.cpp
new file mode 100644
index 0000000000000..51b9ea4c16cd0
--- /dev/null
+++ b/libc/test/src/strings/strncasecmp_l_test.cpp
@@ -0,0 +1,22 @@
+//===-- Unittests for strncasecmp_l ---------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "include/llvm-libc-macros/locale-macros.h"
+#include "src/locale/freelocale.h"
+#include "src/locale/newlocale.h"
+#include "src/strings/strncasecmp_l.h"
+#include "test/UnitTest/Test.h"
+
+TEST(LlvmLibcStrNCaseCmpLTest, Case) {
+  locale_t locale = LIBC_NAMESPACE::newlocale(LC_ALL, "C", nullptr);
+  ASSERT_EQ(LIBC_NAMESPACE::strncasecmp_l("hello", "HELLO", 3, locale), 0);
+  ASSERT_EQ(LIBC_NAMESPACE::strncasecmp_l("abcXX", "ABCYY", 3, locale), 0);
+  ASSERT_LT(LIBC_NAMESPACE::strncasecmp_l("hello1", "hello2", 6, locale), 0);
+  ASSERT_GT(LIBC_NAMESPACE::strncasecmp_l("hello2", "hello1", 6, locale), 0);
+  LIBC_NAMESPACE::freelocale(locale);
+}

@@ -6,15 +6,16 @@
//
//===----------------------------------------------------------------------===//

#include "include/llvm-libc-macros/locale-macros.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind creating an issue or a PR to add proxy header for locale-macros.h? Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will do it tonight. The locale_t now has a proxy header, but the related macros do not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

c8ef added a commit that referenced this pull request Mar 11, 2025
Address review comments in #130407.

This patch is already covered by existing locale test cases.
c8ef added 2 commits March 11, 2025 22:24
@c8ef
Copy link
Contributor Author

c8ef commented Mar 11, 2025

Thank you for your reviews! I have a question about target dependency. With the introduction of proxy headers for locale-related types and macros, should we specify dependencies more precisely for these types and macros, or is it still necessary to depend on libc.include.locale in locale related implementations?

@c8ef c8ef requested a review from lntue March 11, 2025 15:34
@lntue
Copy link
Contributor

lntue commented Mar 11, 2025

Thank you for your reviews! I have a question about target dependency. With the introduction of proxy headers for locale-related types and macros, should we specify dependencies more precisely for these types and macros, or is it still necessary to depend on libc.include.locale in locale related implementations?

After changing to only include the proxy headers, the dependency should be updated to include the proxy header targets, and libc.include.* can be removed, since the dependency from proxy header targets will bring in the correct ones transitively.

@lntue
Copy link
Contributor

lntue commented Mar 11, 2025

@michaelrj-google : Do we need to update the implementation status doc or is it updated automatically?

@michaelrj-google
Copy link
Contributor

I think you need to run docgen to update the docs but I'm not sure how to do that. It's in libc/utils/docgen/docgen.py

@c8ef
Copy link
Contributor Author

c8ef commented Mar 11, 2025

I think you need to run docgen to update the docs but I'm not sure how to do that. It's in libc/utils/docgen/docgen.py

My previous patch appears to have been synced to the website without me having to call docgen. I am not sure if it was done by a routine job or by someone else : )

@lntue
Copy link
Contributor

lntue commented Mar 11, 2025

I think you need to run docgen to update the docs but I'm not sure how to do that. It's in libc/utils/docgen/docgen.py

My previous patch appears to have been synced to the website without me having to call docgen. I am not sure if it was done by a routine job or by someone else : )

Please keep an eye on that and add it in the future PR if needed.

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

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

LGTM with one nit

Co-authored-by: Michael Jones <[email protected]>
@c8ef c8ef merged commit ab22f65 into llvm:main Mar 12, 2025
14 of 15 checks passed
@c8ef c8ef deleted the strcase branch March 12, 2025 02:49
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.

None yet

5 participants