From d5fe4695c872b34f7ddd282d5ae99c79dc81514d Mon Sep 17 00:00:00 2001 From: StefanStojanovic Date: Thu, 20 Jun 2024 12:29:34 +0200 Subject: [PATCH] Addressing review comments 2 --- icu4c/source/common/unicode/platform.h | 10 ---------- icu4c/source/tools/genccode/genccode.c | 18 ++++++++---------- icu4c/source/tools/toolutil/pkg_genc.cpp | 14 ++++++++++---- icu4c/source/tools/toolutil/pkg_genc.h | 3 +++ 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/icu4c/source/common/unicode/platform.h b/icu4c/source/common/unicode/platform.h index d0d87202f36a..59176005f334 100644 --- 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 index f79c2b51cd3d..0f243952a7d4 100644 --- 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 index 69bd5d30254c..d51a52c8fff1 100644 --- a/icu4c/source/tools/toolutil/pkg_genc.cpp +++ b/icu4c/source/tools/toolutil/pkg_genc.cpp @@ -16,6 +16,9 @@ # define NOMCX #include #include +# if defined(__clang__) +# include +# 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,7 +866,7 @@ 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) { @@ -866,9 +874,7 @@ getArchitecture( } 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 index 2f0ac811dcf7..76474ec7df6f 100644 --- 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,