[libc] Add scaffolding for ctype and implementation of isalpha

[abrachet]

libc/config/linux/aarch64/entrypoints.txt

8

Nit: Maybe these should go above errno.h entrypoints for alphabetical order

libc/config/linux/api.td

408 ↗

(On Diff #280646)

Presumably not necessary? You don't have empty Macros so presumably it is fine to leave this out for now.

libc/src/ctype/isalpha.cpp

17

Just use unsigned

libc/test/src/ctype/isalpha_test.cpp

15

You don't need the cast.

27–35

I don't understand the comment, there is no implementation where character would pass but not character++. You're testing the compiler here, not isalpha

[cgyurgyik]

libc/test/src/ctype/isalpha_test.cpp

27–35

This was written after reading the wikipedia page: https://en.wikipedia.org/wiki/C_character_classification#Implementation

I am happy to remove it if you think this is a compiler test instead.

[cgyurgyik]

Removed CType from libc/config/linux/api.td, as well as other comment responses.

[sivachandra]

libc/include/ctype.h.def

2

I think we have a ctype.h file already present. It was added when I was setting up examples. But, ctype.h.def as you have added is the right approach. Can you remove ctype.h in a separate patch and add appropriate targets for ctype.h.def in this patch? Also, you do need to add an entry to linux/api.td isn't it?

libc/src/ctype/isalpha.h

15

Can you move this TODO to the .cpp file?

libc/test/src/ctype/isalpha_test.cpp

12

Wouldn't it be simple if we have just one test like this:

TEST(IsAlpha, DefaultLocale) {
  for (int i = 0; i < 255; ++i) {
    if (('a' <= i && i <= 'z') || ('A' <= i && i <= 'Z')
      ASSERT_TRUE(...);
    else
      ASSERT_FALSE(...);
  }
}

[cgyurgyik]

Update unit tests to a single test that goes through all characters 0-255.
Re-add linux/api.td.
Address other comments

[cgyurgyik]

libc/include/ctype.h.def

2

I will remove ctype.h immediately following the submission of this revision.

libc/test/src/ctype/isalpha_test.cpp

12

Wouldn't it be simple if we have just one test like this:

TEST(IsAlpha, DefaultLocale) {
  for (int i = 0; i < 255; ++i) {
    if (('a' <= i && i <= 'z') || ('A' <= i && i <= 'Z')
      ASSERT_TRUE(...);
    else
      ASSERT_FALSE(...);
  }
}

Yep that works as well. It definitely breaks the idea of unit testing different behaviors, but isalpha is probably simple enough that this won't matter a whole lot. I mostly wasn't sure if 255 calls to is* would get hefty after a few more of these are implemented.

libc/config/linux/aarch64/entrypoints.txt

 set(TARGET_LIBC_ENTRYPOINTS
+    # ctype.h entrypoints
+    libc.src.ctype.isalpha
+    
     # errno.h entrypoints
     libc.src.errno.__errno_location
 
     # string.h entrypoints
     libc.src.string.bzero
     libc.src.string.memchr
     libc.src.string.memcpy
     libc.src.string.memset
     libc.src.string.memrchr
     libc.src.string.strcat
     libc.src.string.strchr
     libc.src.string.strcpy

libc/config/linux/x86_64/entrypoints.txt

 set(TARGET_LIBC_ENTRYPOINTS
     # assert.h entrypoints
     libc.src.assert.__assert_fail
     
+    # ctype.h entrypoints
+    libc.src.ctype.isalpha
+
     # errno.h entrypoints
     libc.src.errno.__errno_location
 
     # signal.h entrypoints
     libc.src.signal.raise
     libc.src.signal.sigaction
     libc.src.signal.sigdelset
     libc.src.signal.sigaddset

libc/include/ctype.h.def

+//===-- C standard library header ctype.h --------------------------------===//
+//
+// 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_CTYPE_H
+#define LLVM_LIBC_CTYPE_H
+
+#include <__llvm-libc-common.h>
+
+%%public_api()
+
+#endif // LLVM_LIBC_CTYPE_H

libc/spec/stdc.td

HeaderSpec Assert = HeaderSpec<
       [
           Macro<"static_assert">,
           Macro<"assert">,
       ],
       [], // Types
       [], // Enumerations
       []
   >;
   
+  HeaderSpec CType = HeaderSpec<
+      "ctype.h",
+      [], // Macros
+      [], // Types
+      [], // Enumerations
+      [
+          FunctionSpec<
+              "isalpha",
+              RetValSpec<IntType>,
+              [ArgSpec<IntType>]
+          >,
+      ]
+  >;
+     
   HeaderSpec String = HeaderSpec<
       "string.h",
       [
           Macro<"NULL">,
       ],
       [
           SizeTType,
       ],
HeaderSpec Threads = HeaderSpec<
                   ArgSpec<IntPtr>,
               ]
           >
       ]
   >;
 
   let Headers = [
     Assert,
+    CType,
     Errno,
     Math,
     String,
     StdIO,
     StdLib,
     Signal,
     Threads,
   ];
 }

libc/src/CMakeLists.txt

 add_subdirectory(assert)
+add_subdirectory(ctype)
 add_subdirectory(errno)
 add_subdirectory(math)
 add_subdirectory(signal)
 add_subdirectory(stdio)
 add_subdirectory(stdlib)
 add_subdirectory(string)
 # TODO: Add this target conditional to the target OS.
 add_subdirectory(sys)
 add_subdirectory(threads)
 add_subdirectory(unistd)
 
 add_subdirectory(__support)

libc/src/ctype/CMakeLists.txt

+add_entrypoint_object(
+  isalpha
+  SRCS
+    isalpha.cpp
+  HDRS
+    isalpha.h
+)

libc/src/ctype/isalpha.h

+//===-- Implementation header for isalpha -------------------------*-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_CTYPE_ISALPHA_H
+#define LLVM_LIBC_SRC_CTYPE_ISALPHA_H
+
+namespace __llvm_libc {
+
+// TODO: Currently restricted to default locale.
+// These should be extended using locale information.
+int isalpha(int c);
+
+} // namespace __llvm_libc
+
+#endif //  LLVM_LIBC_SRC_CTYPE_ISALPHA_H

libc/src/ctype/isalpha.cpp

+//===-- Implementation of isalpha------------------------------------------===//
+//
+// 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/ctype/isalpha.h"
+
+#include "src/__support/common.h"
+
+namespace __llvm_libc {
+
+int LLVM_LIBC_ENTRYPOINT(isalpha)(int c) {
+  const unsigned ch = c;
+  return (ch | 32) - 'a' < 26;
+}
+
+} // namespace __llvm_libc

libc/test/src/CMakeLists.txt

 add_subdirectory(assert)
+add_subdirectory(ctype)
 add_subdirectory(errno)
 add_subdirectory(math)
 add_subdirectory(signal)
 add_subdirectory(stdio)
 add_subdirectory(stdlib)
 add_subdirectory(string)
 add_subdirectory(sys)
 add_subdirectory(threads)
if(COMPILER_RESOURCE_DIR AND LLVM_LIBC_ENABLE_LINTING)
   )
   add_dependencies(libc-integration-test libc-integration-test-tidy)
 endif()
 
 target_link_libraries(libc-integration-test
   PRIVATE
   ${library_files}
 )
 No newline at end of file

libc/test/src/ctype/CMakeLists.txt

+add_libc_testsuite(libc_ctype_unittests)
+
+add_libc_unittest(
+  isalpha
+  SUITE
+    libc_ctype_unittests
+  SRCS
+    isalpha_test.cpp
+  DEPENDS
+    libc.src.ctype.isalpha
+)

libc/test/src/ctype/isalpha_test.cpp

+//===-- Unittests for isalpha----------------------------------------------===//
+//
+// 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/ctype/isalpha.h"
+#include "utils/UnitTest/Test.h"
+
+// Helper function that makes a call to isalpha a bit cleaner
+// for use with testing utilities, since it explicitly requires
+// a boolean value for EXPECT_TRUE and EXPECT_FALSE.
+bool call_isalpha(int c) { return __llvm_libc::isalpha(c); }
+
+TEST(IsAlpha, AllLowercaseLettersOfTheAlphabetShouldReturnTrue) {
+  for (int ch = 'a'; ch <= 'z'; ++ch)
+    EXPECT_TRUE(call_isalpha(ch));
+}
+
+TEST(IsAlpha, AllUppercaseLettersOfTheAlphabetShouldReturnTrue) {
+  for (int ch = 'A'; ch <= 'Z'; ++ch)
+    EXPECT_TRUE(call_isalpha(ch));
+}
+
+TEST(IsAlpha, PostFixOperatorOnBoundaryCharacterShouldStillWork) {
+  int character = 'Z';
+  // This should return true, even though it has side effects.
+  // This may fail in the case of conditional logical operators
+  // being used in the 'isalpha' implementation.
+  EXPECT_TRUE(call_isalpha(character++));
+  // Verify the next character is indeed not true.
+  EXPECT_FALSE(call_isalpha(character));
+}
+
+TEST(IsAlpha, BoundaryCharactersOfAlphabetReturnFalse) {
+  // Verify there is no off-by-one error.
+  EXPECT_FALSE(call_isalpha('a' - 1));
+  EXPECT_FALSE(call_isalpha('z' + 1));
+  EXPECT_FALSE(call_isalpha('A' - 1));
+  EXPECT_FALSE(call_isalpha('Z' + 1));
+}
+
+TEST(IsAlpha, NumbersShouldReturnFalse) {
+  for (int ch = '0'; ch <= '9'; ++ch)
+    EXPECT_FALSE(call_isalpha(ch));
+}
+
+TEST(IsAlpha, ZeroShouldReturnFalse) { EXPECT_FALSE(call_isalpha(0)); }
+
+TEST(IsAlpha, NonAlphabetSymbolsShouldReturnFalse) {
+  EXPECT_FALSE(call_isalpha('?'));
+  EXPECT_FALSE(call_isalpha('~'));
+  EXPECT_FALSE(call_isalpha('+'));
+  EXPECT_FALSE(call_isalpha('\n'));
+}