Addressing review comments 2

unicode-org · Jun 20, 2024 · d5fe469 · d5fe469
1 parent 6e08385
commit d5fe469
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 24 deletions.
diff --git a/icu4c/source/common/unicode/platform.h b/icu4c/source/common/unicode/platform.h
@@ -217,16 +217,6 @@
 #   define U_REAL_MSVC
 #endif
 
-/**
- * \def U_CLANG_CL
- * Defined if the compiler is Clang compatible with MSVC (Clang on Windows).
- * Otherwise undefined.
- * @internal
- */
-#if (defined(_MSC_VER) && defined(__clang__) && __clang__)
-#   define U_CLANG_CL
-#endif
-
 /**
  * \def CYGWINMSVC
  * Defined if this is Windows with Cygwin, but using MSVC rather than gcc.

diff --git a/icu4c/source/tools/genccode/genccode.c b/icu4c/source/tools/genccode/genccode.c
@@ -70,9 +70,7 @@ enum {
 #ifdef CAN_GENERATE_OBJECTS
   kOptObject,
   kOptMatchArch,
-#ifdef U_CLANG_CL
   kOptCpuArch,
-#endif
   kOptSkipDllExport,
 #endif
   kOptFilename,
@@ -89,9 +87,7 @@ static UOption options[]={
 #ifdef CAN_GENERATE_OBJECTS
 /*6*/UOPTION_DEF("object", 'o', UOPT_NO_ARG),
      UOPTION_DEF("match-arch", 'm', UOPT_REQUIRES_ARG),
-#ifdef U_CLANG_CL
      UOPTION_DEF("cpu-arch", 'c', UOPT_REQUIRES_ARG),
-#endif
      UOPTION_DEF("skip-dll-export", '\0', UOPT_NO_ARG),
 #endif
      UOPTION_DEF("filename", 'f', UOPT_REQUIRES_ARG),
@@ -137,9 +133,8 @@ main(int argc, char* argv[]) {
             "\t-o or --object      write a .obj file instead of .c\n"
             "\t-m or --match-arch file.o  match the architecture (CPU, 32/64 bits) of the specified .o\n"
             "\t                    ELF format defaults to i386. Windows defaults to the native platform.\n"
-#ifdef U_CLANG_CL
             "\t-c or --cpu-arch    Specify a CPU architecture for which to write a .obj file for ClangCL on Windows\n"
-#endif
+            "\t                    Valid values for this opton are x64, x86 and arm64.\n"
             "\t--skip-dll-export   Don't export the ICU data entry point symbol (for use when statically linking)\n");
 #endif
         fprintf(stderr,
@@ -202,14 +197,17 @@ main(int argc, char* argv[]) {
                 break;
 #ifdef CAN_GENERATE_OBJECTS
             case CALL_WRITEOBJECT:
+                if(options[kOptCpuArch].doesOccur) {
+                    if (!checkCpuArchitecture(options[kOptCpuArch].value)) {
+                        fprintf(stderr,
+                            "CPU architecture \"%s\" is unknown.\n", options[kOptCpuArch].value);
+                        return -1;
+                    }
+                }
                 writeObjectCode(filename, options[kOptDestDir].value,
                                 options[kOptEntryPoint].doesOccur ? options[kOptEntryPoint].value : NULL,
                                 options[kOptMatchArch].doesOccur ? options[kOptMatchArch].value : NULL,
-#ifdef U_CLANG_CL
                                 options[kOptCpuArch].doesOccur ? options[kOptCpuArch].value : NULL,
-#else
-                                NULL,
-#endif
                                 options[kOptFilename].doesOccur ? options[kOptFilename].value : NULL,
                                 NULL,
                                 0,

diff --git a/icu4c/source/tools/toolutil/pkg_genc.cpp b/icu4c/source/tools/toolutil/pkg_genc.cpp
@@ -16,6 +16,9 @@
 #   define NOMCX
 #include <windows.h>
 #include <time.h>
+#   if defined(__clang__)
+#       include <exception>
+#   endif
 #   ifdef __GNUC__
 #       define WINDOWS_WITH_GNUC
 #   endif
@@ -294,6 +297,11 @@ checkAssemblyHeaderName(const char* optAssembly) {
     return false;
 }
 
+U_CAPI UBool U_EXPORT2
+checkCpuArchitecture(const char* optCpuArch) {
+    return strcmp(optCpuArch, "x64") == 0 || strcmp(optCpuArch, "x86") == 0 || strcmp(optCpuArch, "arm64") == 0;
+}
+
 
 U_CAPI void U_EXPORT2
 printAssemblyHeadersToStdErr() {
@@ -858,17 +866,15 @@ getArchitecture(
         // this would potentially be problematic when cross-compiling as this code
         // would most likely be ran on host machine to generate the .obj file for
         // the target architecture.
-#       ifdef U_CLANG_CL
+#       if defined(__clang__)
             if (strcmp(optCpuArch, "x64") == 0) {
                 *pCPU = IMAGE_FILE_MACHINE_AMD64;
             } else if (strcmp(optCpuArch, "x86") == 0) {
                 *pCPU = IMAGE_FILE_MACHINE_I386;
             } else if (strcmp(optCpuArch, "arm64") == 0) {
                 *pCPU = IMAGE_FILE_MACHINE_ARM64;
             } else {
-                // This should never happen.
-                fprintf(stderr, "genccode: unable to process %s CPU architecture\n", optCpuArch);
-                exit(U_ILLEGAL_ARGUMENT_ERROR);
+                std::terminate(); // Unreachable.
             }
 #       else
             *pCPU = IMAGE_FILE_MACHINE_UNKNOWN;

diff --git a/icu4c/source/tools/toolutil/pkg_genc.h b/icu4c/source/tools/toolutil/pkg_genc.h
@@ -74,6 +74,9 @@ printAssemblyHeadersToStdErr(void);
 U_CAPI UBool U_EXPORT2
 checkAssemblyHeaderName(const char* optAssembly);
 
+U_CAPI UBool U_EXPORT2
+checkCpuArchitecture(const char* optCpuArch);
+
 U_CAPI void U_EXPORT2
 writeCCode(
     const char *filename,