[clang] clang_EvalResult_getAsCXString impl (PR #134551)
xTachyon wrote: @AaronBallman :smile: https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
https://github.com/xTachyon updated https://github.com/llvm/llvm-project/pull/134551 >From 88a7517918ff8f6a5521527e9e1a141af09035c5 Mon Sep 17 00:00:00 2001 From: Andrei Damian Date: Sun, 6 Apr 2025 19:55:59 +0300 Subject: [PATCH] clang_EvalResult_getAsCXString impl --- clang/include/clang-c/CXString.h | 3 ++ clang/include/clang-c/Index.h | 15 ++-- clang/tools/libclang/CIndex.cpp | 46 ++- clang/tools/libclang/CXString.cpp | 22 --- clang/tools/libclang/libclang.map | 6 +++ clang/unittests/libclang/LibclangTest.cpp | 41 6 files changed, 96 insertions(+), 37 deletions(-) diff --git a/clang/include/clang-c/CXString.h b/clang/include/clang-c/CXString.h index 63dce4d140ce2..a0ad830230338 100644 --- a/clang/include/clang-c/CXString.h +++ b/clang/include/clang-c/CXString.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_C_CXSTRING_H #define LLVM_CLANG_C_CXSTRING_H +#include #include "clang-c/ExternC.h" #include "clang-c/Platform.h" @@ -36,6 +37,7 @@ LLVM_CLANG_C_EXTERN_C_BEGIN */ typedef struct { const void *data; + size_t length; unsigned private_flags; } CXString; @@ -52,6 +54,7 @@ typedef struct { * to `std::string::c_str()`. */ CINDEX_LINKAGE const char *clang_getCString(CXString string); +CINDEX_LINKAGE const char *clang_getCString2(CXString string, size_t *length); /** * Free the given string. diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h index 38e2417dcd181..4ef41d56e43ae 100644 --- a/clang/include/clang-c/Index.h +++ b/clang/include/clang-c/Index.h @@ -5918,12 +5918,19 @@ clang_EvalResult_getAsUnsigned(CXEvalResult E); CINDEX_LINKAGE double clang_EvalResult_getAsDouble(CXEvalResult E); /** - * Returns the evaluation result as a constant string if the - * kind is other than Int or float. User must not free this pointer, - * instead call clang_EvalResult_dispose on the CXEvalResult returned - * by clang_Cursor_Evaluate. + * This function behaves the same as clang_EvalResult_getAsCXString, with 2 + * exceptions: + * - the string literal will be truncated if a nul byte is found in the string. + * For this reason clang_EvalResult_getAsCXString is recommended. + * - User must not free this pointer, instead call clang_EvalResult_dispose on + * the CXEvalResult returned by clang_Cursor_Evaluate. */ CINDEX_LINKAGE const char *clang_EvalResult_getAsStr(CXEvalResult E); +/** + * Returns the evaluation result as a constant string if the + * kind is other than Int or float. This might include zero bytes. + */ +CINDEX_LINKAGE CXString clang_EvalResult_getAsCXString(CXEvalResult E); /** * Disposes the created Eval memory. diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index c8db6c92bb4d4..672e791ad3455 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -4589,13 +4589,13 @@ struct ExprEvalResult { unsigned long long unsignedVal; long long intVal; double floatVal; -char *stringVal; +CXString stringVal; } EvalData; bool IsUnsignedInt; ~ExprEvalResult() { if (EvalType != CXEval_UnExposed && EvalType != CXEval_Float && EvalType != CXEval_Int) { - delete[] EvalData.stringVal; + clang_disposeString(EvalData.stringVal); } } }; @@ -4651,7 +4651,17 @@ const char *clang_EvalResult_getAsStr(CXEvalResult E) { if (!E) { return nullptr; } - return ((ExprEvalResult *)E)->EvalData.stringVal; + return clang_getCString(((ExprEvalResult *)E)->EvalData.stringVal); +} + +CXString clang_EvalResult_getAsCXString(CXEvalResult E) { + if (!E) { +return cxstring::createNull(); + } + size_t length; + auto data = + clang_getCString2(((ExprEvalResult *)E)->EvalData.stringVal, &length); + return cxstring::createDup(StringRef(data, length)); } static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { @@ -4716,11 +4726,7 @@ static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { result->EvalType = CXEval_StrLiteral; } - std::string strRef(StrE->getString().str()); - result->EvalData.stringVal = new char[strRef.size() + 1]; - strncpy((char *)result->EvalData.stringVal, strRef.c_str(), - strRef.size()); - result->EvalData.stringVal[strRef.size()] = '\0'; + result->EvalData.stringVal = cxstring::createDup(StrE->getString()); return result.release(); } } else if (expr->getStmtClass() == Stmt::ObjCStringLiteralClass || @@ -4737,10 +4743,7 @@ static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { result->EvalType = CXEval_StrLiteral; } -std::string strRef(StrE->getString().str()); -result->EvalData.stringVal = new char[strRef.size() + 1]; -strncpy((char *)result->EvalData.stringVal, strRef.c_str(), strRef.size()); -result->EvalData.stringVal[strRef.size()] = '\0'; +result->EvalData.strin
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
xTachyon wrote: `CXString::private_flags` only has 2 bits used. Could we use the rest for the size? Would it be enough? There's also the possibility to not touch `CXString` at all, and add a function that returns ptr+size only for `clang_EvalResult_getAs...`. What do you think? https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
@@ -52,6 +54,7 @@ typedef struct { * to `std::string::c_str()`. */ CINDEX_LINKAGE const char *clang_getCString(CXString string); +CINDEX_LINKAGE const char *clang_getCString2(CXString string, size_t *length); xTachyon wrote: I tried to find a macro that wraps _Nonnull to use here, but I couldn't find it. https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
https://github.com/xTachyon created https://github.com/llvm/llvm-project/pull/134551 Tries to implement #69749. >From Discord: ` In terms of how to solve it... I'm hoping we can extend CXString to be length-aware and then add an interface that returns a CXString object instead. Perhaps clang_EvalResult_getAsCXString() with a comment explaining that getAsStr() is only valid for null-terminated strings? ` There's some questions I have: 1. Is `size_t` appropriate here? I see some functions using `unsigned`, and some using `size_t`. 2. On this new flow to get a string literal, there's at least 2 allocations of the string from the 2 `cxstring::createDup`, one when evaluating the result, and one when returning it. Is this ok/can it be done better? 3. Is returning a "null" `CXString` on error a good idea? There's probably more things that I did wrong, so I'm hoping for a review. >From 0d23f05ff443ef88e52f0cd4e7c50f221642d209 Mon Sep 17 00:00:00 2001 From: Andrei DAMIAN Date: Sun, 6 Apr 2025 19:55:59 +0300 Subject: [PATCH] clang_EvalResult_getAsCXString impl --- clang/include/clang-c/CXString.h | 3 ++ clang/include/clang-c/Index.h | 15 ++-- clang/tools/libclang/CIndex.cpp | 46 ++- clang/tools/libclang/CXString.cpp | 22 --- clang/tools/libclang/libclang.map | 6 +++ clang/unittests/libclang/LibclangTest.cpp | 41 6 files changed, 96 insertions(+), 37 deletions(-) diff --git a/clang/include/clang-c/CXString.h b/clang/include/clang-c/CXString.h index 63dce4d140ce2..a0ad830230338 100644 --- a/clang/include/clang-c/CXString.h +++ b/clang/include/clang-c/CXString.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_C_CXSTRING_H #define LLVM_CLANG_C_CXSTRING_H +#include #include "clang-c/ExternC.h" #include "clang-c/Platform.h" @@ -36,6 +37,7 @@ LLVM_CLANG_C_EXTERN_C_BEGIN */ typedef struct { const void *data; + size_t length; unsigned private_flags; } CXString; @@ -52,6 +54,7 @@ typedef struct { * to `std::string::c_str()`. */ CINDEX_LINKAGE const char *clang_getCString(CXString string); +CINDEX_LINKAGE const char *clang_getCString2(CXString string, size_t *length); /** * Free the given string. diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h index 38e2417dcd181..4ef41d56e43ae 100644 --- a/clang/include/clang-c/Index.h +++ b/clang/include/clang-c/Index.h @@ -5918,12 +5918,19 @@ clang_EvalResult_getAsUnsigned(CXEvalResult E); CINDEX_LINKAGE double clang_EvalResult_getAsDouble(CXEvalResult E); /** - * Returns the evaluation result as a constant string if the - * kind is other than Int or float. User must not free this pointer, - * instead call clang_EvalResult_dispose on the CXEvalResult returned - * by clang_Cursor_Evaluate. + * This function behaves the same as clang_EvalResult_getAsCXString, with 2 + * exceptions: + * - the string literal will be truncated if a nul byte is found in the string. + * For this reason clang_EvalResult_getAsCXString is recommended. + * - User must not free this pointer, instead call clang_EvalResult_dispose on + * the CXEvalResult returned by clang_Cursor_Evaluate. */ CINDEX_LINKAGE const char *clang_EvalResult_getAsStr(CXEvalResult E); +/** + * Returns the evaluation result as a constant string if the + * kind is other than Int or float. This might include zero bytes. + */ +CINDEX_LINKAGE CXString clang_EvalResult_getAsCXString(CXEvalResult E); /** * Disposes the created Eval memory. diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index c8db6c92bb4d4..672e791ad3455 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -4589,13 +4589,13 @@ struct ExprEvalResult { unsigned long long unsignedVal; long long intVal; double floatVal; -char *stringVal; +CXString stringVal; } EvalData; bool IsUnsignedInt; ~ExprEvalResult() { if (EvalType != CXEval_UnExposed && EvalType != CXEval_Float && EvalType != CXEval_Int) { - delete[] EvalData.stringVal; + clang_disposeString(EvalData.stringVal); } } }; @@ -4651,7 +4651,17 @@ const char *clang_EvalResult_getAsStr(CXEvalResult E) { if (!E) { return nullptr; } - return ((ExprEvalResult *)E)->EvalData.stringVal; + return clang_getCString(((ExprEvalResult *)E)->EvalData.stringVal); +} + +CXString clang_EvalResult_getAsCXString(CXEvalResult E) { + if (!E) { +return cxstring::createNull(); + } + size_t length; + auto data = + clang_getCString2(((ExprEvalResult *)E)->EvalData.stringVal, &length); + return cxstring::createDup(StringRef(data, length)); } static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { @@ -4716,11 +4726,7 @@ static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { result->EvalType = CXEval_StrLiteral; } - std::string strRef(StrE->getString().str()); - result->
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
https://github.com/xTachyon updated https://github.com/llvm/llvm-project/pull/134551 >From 88a7517918ff8f6a5521527e9e1a141af09035c5 Mon Sep 17 00:00:00 2001 From: Andrei Damian Date: Sun, 6 Apr 2025 19:55:59 +0300 Subject: [PATCH 1/3] clang_EvalResult_getAsCXString impl --- clang/include/clang-c/CXString.h | 3 ++ clang/include/clang-c/Index.h | 15 ++-- clang/tools/libclang/CIndex.cpp | 46 ++- clang/tools/libclang/CXString.cpp | 22 --- clang/tools/libclang/libclang.map | 6 +++ clang/unittests/libclang/LibclangTest.cpp | 41 6 files changed, 96 insertions(+), 37 deletions(-) diff --git a/clang/include/clang-c/CXString.h b/clang/include/clang-c/CXString.h index 63dce4d140ce2..a0ad830230338 100644 --- a/clang/include/clang-c/CXString.h +++ b/clang/include/clang-c/CXString.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_C_CXSTRING_H #define LLVM_CLANG_C_CXSTRING_H +#include #include "clang-c/ExternC.h" #include "clang-c/Platform.h" @@ -36,6 +37,7 @@ LLVM_CLANG_C_EXTERN_C_BEGIN */ typedef struct { const void *data; + size_t length; unsigned private_flags; } CXString; @@ -52,6 +54,7 @@ typedef struct { * to `std::string::c_str()`. */ CINDEX_LINKAGE const char *clang_getCString(CXString string); +CINDEX_LINKAGE const char *clang_getCString2(CXString string, size_t *length); /** * Free the given string. diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h index 38e2417dcd181..4ef41d56e43ae 100644 --- a/clang/include/clang-c/Index.h +++ b/clang/include/clang-c/Index.h @@ -5918,12 +5918,19 @@ clang_EvalResult_getAsUnsigned(CXEvalResult E); CINDEX_LINKAGE double clang_EvalResult_getAsDouble(CXEvalResult E); /** - * Returns the evaluation result as a constant string if the - * kind is other than Int or float. User must not free this pointer, - * instead call clang_EvalResult_dispose on the CXEvalResult returned - * by clang_Cursor_Evaluate. + * This function behaves the same as clang_EvalResult_getAsCXString, with 2 + * exceptions: + * - the string literal will be truncated if a nul byte is found in the string. + * For this reason clang_EvalResult_getAsCXString is recommended. + * - User must not free this pointer, instead call clang_EvalResult_dispose on + * the CXEvalResult returned by clang_Cursor_Evaluate. */ CINDEX_LINKAGE const char *clang_EvalResult_getAsStr(CXEvalResult E); +/** + * Returns the evaluation result as a constant string if the + * kind is other than Int or float. This might include zero bytes. + */ +CINDEX_LINKAGE CXString clang_EvalResult_getAsCXString(CXEvalResult E); /** * Disposes the created Eval memory. diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index c8db6c92bb4d4..672e791ad3455 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -4589,13 +4589,13 @@ struct ExprEvalResult { unsigned long long unsignedVal; long long intVal; double floatVal; -char *stringVal; +CXString stringVal; } EvalData; bool IsUnsignedInt; ~ExprEvalResult() { if (EvalType != CXEval_UnExposed && EvalType != CXEval_Float && EvalType != CXEval_Int) { - delete[] EvalData.stringVal; + clang_disposeString(EvalData.stringVal); } } }; @@ -4651,7 +4651,17 @@ const char *clang_EvalResult_getAsStr(CXEvalResult E) { if (!E) { return nullptr; } - return ((ExprEvalResult *)E)->EvalData.stringVal; + return clang_getCString(((ExprEvalResult *)E)->EvalData.stringVal); +} + +CXString clang_EvalResult_getAsCXString(CXEvalResult E) { + if (!E) { +return cxstring::createNull(); + } + size_t length; + auto data = + clang_getCString2(((ExprEvalResult *)E)->EvalData.stringVal, &length); + return cxstring::createDup(StringRef(data, length)); } static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { @@ -4716,11 +4726,7 @@ static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { result->EvalType = CXEval_StrLiteral; } - std::string strRef(StrE->getString().str()); - result->EvalData.stringVal = new char[strRef.size() + 1]; - strncpy((char *)result->EvalData.stringVal, strRef.c_str(), - strRef.size()); - result->EvalData.stringVal[strRef.size()] = '\0'; + result->EvalData.stringVal = cxstring::createDup(StrE->getString()); return result.release(); } } else if (expr->getStmtClass() == Stmt::ObjCStringLiteralClass || @@ -4737,10 +4743,7 @@ static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { result->EvalType = CXEval_StrLiteral; } -std::string strRef(StrE->getString().str()); -result->EvalData.stringVal = new char[strRef.size() + 1]; -strncpy((char *)result->EvalData.stringVal, strRef.c_str(), strRef.size()); -result->EvalData.stringVal[strRef.size()] = '\0'; +result->EvalData.s
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
https://github.com/xTachyon updated https://github.com/llvm/llvm-project/pull/134551 >From 88a7517918ff8f6a5521527e9e1a141af09035c5 Mon Sep 17 00:00:00 2001 From: Andrei Damian Date: Sun, 6 Apr 2025 19:55:59 +0300 Subject: [PATCH 1/2] clang_EvalResult_getAsCXString impl --- clang/include/clang-c/CXString.h | 3 ++ clang/include/clang-c/Index.h | 15 ++-- clang/tools/libclang/CIndex.cpp | 46 ++- clang/tools/libclang/CXString.cpp | 22 --- clang/tools/libclang/libclang.map | 6 +++ clang/unittests/libclang/LibclangTest.cpp | 41 6 files changed, 96 insertions(+), 37 deletions(-) diff --git a/clang/include/clang-c/CXString.h b/clang/include/clang-c/CXString.h index 63dce4d140ce2..a0ad830230338 100644 --- a/clang/include/clang-c/CXString.h +++ b/clang/include/clang-c/CXString.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_C_CXSTRING_H #define LLVM_CLANG_C_CXSTRING_H +#include #include "clang-c/ExternC.h" #include "clang-c/Platform.h" @@ -36,6 +37,7 @@ LLVM_CLANG_C_EXTERN_C_BEGIN */ typedef struct { const void *data; + size_t length; unsigned private_flags; } CXString; @@ -52,6 +54,7 @@ typedef struct { * to `std::string::c_str()`. */ CINDEX_LINKAGE const char *clang_getCString(CXString string); +CINDEX_LINKAGE const char *clang_getCString2(CXString string, size_t *length); /** * Free the given string. diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h index 38e2417dcd181..4ef41d56e43ae 100644 --- a/clang/include/clang-c/Index.h +++ b/clang/include/clang-c/Index.h @@ -5918,12 +5918,19 @@ clang_EvalResult_getAsUnsigned(CXEvalResult E); CINDEX_LINKAGE double clang_EvalResult_getAsDouble(CXEvalResult E); /** - * Returns the evaluation result as a constant string if the - * kind is other than Int or float. User must not free this pointer, - * instead call clang_EvalResult_dispose on the CXEvalResult returned - * by clang_Cursor_Evaluate. + * This function behaves the same as clang_EvalResult_getAsCXString, with 2 + * exceptions: + * - the string literal will be truncated if a nul byte is found in the string. + * For this reason clang_EvalResult_getAsCXString is recommended. + * - User must not free this pointer, instead call clang_EvalResult_dispose on + * the CXEvalResult returned by clang_Cursor_Evaluate. */ CINDEX_LINKAGE const char *clang_EvalResult_getAsStr(CXEvalResult E); +/** + * Returns the evaluation result as a constant string if the + * kind is other than Int or float. This might include zero bytes. + */ +CINDEX_LINKAGE CXString clang_EvalResult_getAsCXString(CXEvalResult E); /** * Disposes the created Eval memory. diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index c8db6c92bb4d4..672e791ad3455 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -4589,13 +4589,13 @@ struct ExprEvalResult { unsigned long long unsignedVal; long long intVal; double floatVal; -char *stringVal; +CXString stringVal; } EvalData; bool IsUnsignedInt; ~ExprEvalResult() { if (EvalType != CXEval_UnExposed && EvalType != CXEval_Float && EvalType != CXEval_Int) { - delete[] EvalData.stringVal; + clang_disposeString(EvalData.stringVal); } } }; @@ -4651,7 +4651,17 @@ const char *clang_EvalResult_getAsStr(CXEvalResult E) { if (!E) { return nullptr; } - return ((ExprEvalResult *)E)->EvalData.stringVal; + return clang_getCString(((ExprEvalResult *)E)->EvalData.stringVal); +} + +CXString clang_EvalResult_getAsCXString(CXEvalResult E) { + if (!E) { +return cxstring::createNull(); + } + size_t length; + auto data = + clang_getCString2(((ExprEvalResult *)E)->EvalData.stringVal, &length); + return cxstring::createDup(StringRef(data, length)); } static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { @@ -4716,11 +4726,7 @@ static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { result->EvalType = CXEval_StrLiteral; } - std::string strRef(StrE->getString().str()); - result->EvalData.stringVal = new char[strRef.size() + 1]; - strncpy((char *)result->EvalData.stringVal, strRef.c_str(), - strRef.size()); - result->EvalData.stringVal[strRef.size()] = '\0'; + result->EvalData.stringVal = cxstring::createDup(StrE->getString()); return result.release(); } } else if (expr->getStmtClass() == Stmt::ObjCStringLiteralClass || @@ -4737,10 +4743,7 @@ static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { result->EvalType = CXEval_StrLiteral; } -std::string strRef(StrE->getString().str()); -result->EvalData.stringVal = new char[strRef.size() + 1]; -strncpy((char *)result->EvalData.stringVal, strRef.c_str(), strRef.size()); -result->EvalData.stringVal[strRef.size()] = '\0'; +result->EvalData.s
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
xTachyon wrote: > What if we add unsigned clang_getCStringLength(CXString); which gets the > length but not the contents? Do we need to package pointer and size together? Where do we store the length, if we can't add a new field to `CXString`, and we can't use `private_flags` to store it? https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
xTachyon wrote: > It means all such string accesses would require two indirections to get to > the string data I don't think that's true. Getting the pointer would be `ptr+sizeof(size_t)`. > I also wonder if we want to be slightly less memory efficient and add a > version field in case we need to make changes again in the future Where would this go? The `Data` structure would be private API, not exposed, so it shouldn't matter when we change it. I don't understand very well the last messages of you and [cor3ntin](https://github.com/cor3ntin), sorry. https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
xTachyon wrote: That's pretty much what I was thinking of in terms of implementation. > and we'd need the versioning information if we had Clang N tracking length + > contents and Clang N + 1 was tracking length + contents + encoding because > the newer Clang would be casting to a pointer of the wrong type if it was > given a CXString from the older Clang. > > But now that I think about it, this still runs into ABI issues like the > original solution. Newer Clang would produce a private_flags value that older > Clang couldn't handle gracefully. Does libclang support having different versions of it at the same time loaded in a process? Otherwise I don't see how the outside world would be able to tell or care what `data` and `private_flags` mean. In my suggestion, `StringWithLength` would only have a definition inside `CXString.cpp`; I don't want to make it part of the public API. I believe this would mean that this can be changed at any point for any reason. https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
https://github.com/xTachyon updated https://github.com/llvm/llvm-project/pull/134551 >From 88a7517918ff8f6a5521527e9e1a141af09035c5 Mon Sep 17 00:00:00 2001 From: Andrei Damian Date: Sun, 6 Apr 2025 19:55:59 +0300 Subject: [PATCH 1/3] clang_EvalResult_getAsCXString impl --- clang/include/clang-c/CXString.h | 3 ++ clang/include/clang-c/Index.h | 15 ++-- clang/tools/libclang/CIndex.cpp | 46 ++- clang/tools/libclang/CXString.cpp | 22 --- clang/tools/libclang/libclang.map | 6 +++ clang/unittests/libclang/LibclangTest.cpp | 41 6 files changed, 96 insertions(+), 37 deletions(-) diff --git a/clang/include/clang-c/CXString.h b/clang/include/clang-c/CXString.h index 63dce4d140ce2..a0ad830230338 100644 --- a/clang/include/clang-c/CXString.h +++ b/clang/include/clang-c/CXString.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_C_CXSTRING_H #define LLVM_CLANG_C_CXSTRING_H +#include #include "clang-c/ExternC.h" #include "clang-c/Platform.h" @@ -36,6 +37,7 @@ LLVM_CLANG_C_EXTERN_C_BEGIN */ typedef struct { const void *data; + size_t length; unsigned private_flags; } CXString; @@ -52,6 +54,7 @@ typedef struct { * to `std::string::c_str()`. */ CINDEX_LINKAGE const char *clang_getCString(CXString string); +CINDEX_LINKAGE const char *clang_getCString2(CXString string, size_t *length); /** * Free the given string. diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h index 38e2417dcd181..4ef41d56e43ae 100644 --- a/clang/include/clang-c/Index.h +++ b/clang/include/clang-c/Index.h @@ -5918,12 +5918,19 @@ clang_EvalResult_getAsUnsigned(CXEvalResult E); CINDEX_LINKAGE double clang_EvalResult_getAsDouble(CXEvalResult E); /** - * Returns the evaluation result as a constant string if the - * kind is other than Int or float. User must not free this pointer, - * instead call clang_EvalResult_dispose on the CXEvalResult returned - * by clang_Cursor_Evaluate. + * This function behaves the same as clang_EvalResult_getAsCXString, with 2 + * exceptions: + * - the string literal will be truncated if a nul byte is found in the string. + * For this reason clang_EvalResult_getAsCXString is recommended. + * - User must not free this pointer, instead call clang_EvalResult_dispose on + * the CXEvalResult returned by clang_Cursor_Evaluate. */ CINDEX_LINKAGE const char *clang_EvalResult_getAsStr(CXEvalResult E); +/** + * Returns the evaluation result as a constant string if the + * kind is other than Int or float. This might include zero bytes. + */ +CINDEX_LINKAGE CXString clang_EvalResult_getAsCXString(CXEvalResult E); /** * Disposes the created Eval memory. diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index c8db6c92bb4d4..672e791ad3455 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -4589,13 +4589,13 @@ struct ExprEvalResult { unsigned long long unsignedVal; long long intVal; double floatVal; -char *stringVal; +CXString stringVal; } EvalData; bool IsUnsignedInt; ~ExprEvalResult() { if (EvalType != CXEval_UnExposed && EvalType != CXEval_Float && EvalType != CXEval_Int) { - delete[] EvalData.stringVal; + clang_disposeString(EvalData.stringVal); } } }; @@ -4651,7 +4651,17 @@ const char *clang_EvalResult_getAsStr(CXEvalResult E) { if (!E) { return nullptr; } - return ((ExprEvalResult *)E)->EvalData.stringVal; + return clang_getCString(((ExprEvalResult *)E)->EvalData.stringVal); +} + +CXString clang_EvalResult_getAsCXString(CXEvalResult E) { + if (!E) { +return cxstring::createNull(); + } + size_t length; + auto data = + clang_getCString2(((ExprEvalResult *)E)->EvalData.stringVal, &length); + return cxstring::createDup(StringRef(data, length)); } static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { @@ -4716,11 +4726,7 @@ static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { result->EvalType = CXEval_StrLiteral; } - std::string strRef(StrE->getString().str()); - result->EvalData.stringVal = new char[strRef.size() + 1]; - strncpy((char *)result->EvalData.stringVal, strRef.c_str(), - strRef.size()); - result->EvalData.stringVal[strRef.size()] = '\0'; + result->EvalData.stringVal = cxstring::createDup(StrE->getString()); return result.release(); } } else if (expr->getStmtClass() == Stmt::ObjCStringLiteralClass || @@ -4737,10 +4743,7 @@ static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { result->EvalType = CXEval_StrLiteral; } -std::string strRef(StrE->getString().str()); -result->EvalData.stringVal = new char[strRef.size() + 1]; -strncpy((char *)result->EvalData.stringVal, strRef.c_str(), strRef.size()); -result->EvalData.stringVal[strRef.size()] = '\0'; +result->EvalData.s
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
xTachyon wrote: We could keep `CXString` as it was before, but `const void *data` could actually point to a dynamically allocated struct that has roughly this definition: ```c struct Data { size_t length; char data[1]; }; ``` This wouldn't change the current API, and it would still be only one allocation. What do you think? https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
@@ -25,14 +25,19 @@ enum CXStringFlag { /// CXString contains a 'const char *' that it doesn't own. CXS_Unmanaged, - /// CXString contains a 'const char *' that it allocated with malloc(). - CXS_Malloc, + /// CXString contains a 'CStringImpl' that it allocated with malloc(). + CXS_MallocWithSize, /// CXString contains a CXStringBuf that needs to be returned to the /// CXStringPool. CXS_StringBuf }; +struct CStringImpl { + size_t length; + char buffer[sizeof(length)]; xTachyon wrote: I'll look at that class when I get the time, but just allocating the data without making a struct seems straight forward enough. I also need to see why tests fail while I'm at it. https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
@@ -25,14 +25,19 @@ enum CXStringFlag { /// CXString contains a 'const char *' that it doesn't own. CXS_Unmanaged, - /// CXString contains a 'const char *' that it allocated with malloc(). - CXS_Malloc, + /// CXString contains a 'CStringImpl' that it allocated with malloc(). + CXS_MallocWithSize, /// CXString contains a CXStringBuf that needs to be returned to the /// CXStringPool. CXS_StringBuf }; +struct CStringImpl { + size_t length; + char buffer[sizeof(length)]; xTachyon wrote: I forgot about oob here. I was trying to avoid compiler extensions (flexible array member), as I understand it's not standard C++. I can change it to flexible array member without problem. https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
https://github.com/xTachyon edited https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
https://github.com/xTachyon updated https://github.com/llvm/llvm-project/pull/134551 >From 6d8d8955c1e3c706e3195e440f9435e5ae197f9f Mon Sep 17 00:00:00 2001 From: Andrei Damian Date: Sun, 6 Apr 2025 19:55:59 +0300 Subject: [PATCH] clang_EvalResult_getAsCXString impl --- clang/include/clang-c/CXString.h | 16 ++- clang/include/clang-c/Index.h | 17 +-- clang/tools/libclang/CIndex.cpp | 44 +++-- clang/tools/libclang/CXString.cpp | 57 +-- clang/tools/libclang/libclang.map | 2 + clang/unittests/libclang/LibclangTest.cpp | 40 6 files changed, 129 insertions(+), 47 deletions(-) diff --git a/clang/include/clang-c/CXString.h b/clang/include/clang-c/CXString.h index 63dce4d140ce2..14d2eebbc8733 100644 --- a/clang/include/clang-c/CXString.h +++ b/clang/include/clang-c/CXString.h @@ -16,6 +16,7 @@ #include "clang-c/ExternC.h" #include "clang-c/Platform.h" +#include LLVM_CLANG_C_EXTERN_C_BEGIN @@ -44,6 +45,11 @@ typedef struct { unsigned Count; } CXStringSet; +typedef struct { + const char *string; + size_t length; +} CStringInfo; + /** * Retrieve the character data associated with the given string. * @@ -53,6 +59,15 @@ typedef struct { */ CINDEX_LINKAGE const char *clang_getCString(CXString string); +/** + * Retrieve the character data associated with the given string and its length. + * + * The returned lenght might be bigger than strlen(.string) if the string + * contains nul bytes. This function has the same requirements and guarantees as + * clang_getCString. + */ +CINDEX_LINKAGE CStringInfo clang_getCStringInfo(CXString string); + /** * Free the given string. */ @@ -70,4 +85,3 @@ CINDEX_LINKAGE void clang_disposeStringSet(CXStringSet *set); LLVM_CLANG_C_EXTERN_C_END #endif - diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h index c35311c886413..f86a1c25f6c92 100644 --- a/clang/include/clang-c/Index.h +++ b/clang/include/clang-c/Index.h @@ -6058,12 +6058,21 @@ clang_EvalResult_getAsUnsigned(CXEvalResult E); CINDEX_LINKAGE double clang_EvalResult_getAsDouble(CXEvalResult E); /** - * Returns the evaluation result as a constant string if the - * kind is other than Int or float. User must not free this pointer, - * instead call clang_EvalResult_dispose on the CXEvalResult returned - * by clang_Cursor_Evaluate. + * This function behaves the same as clang_EvalResult_getAsCXString, with 2 + * exceptions: + * - the string literal will be truncated if a nul byte is found in the string. + * For this reason clang_EvalResult_getAsCXString is recommended. + * - the caller must not free this pointer; instead call + * clang_EvalResult_dispose on the CXEvalResult returned by + * clang_Cursor_Evaluate. */ CINDEX_LINKAGE const char *clang_EvalResult_getAsStr(CXEvalResult E); +/** + * Returns the evaluation result as a CXString if the + * kind is other than Int or float. This might include zero bytes. + * The caller is responsible for freeing the CXString using clang_disposeString. + */ +CINDEX_LINKAGE CXString clang_EvalResult_getAsCXString(CXEvalResult E); /** * Disposes the created Eval memory. diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index 9089984fa4a54..2d27a90b80544 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -4595,13 +4595,13 @@ struct ExprEvalResult { unsigned long long unsignedVal; long long intVal; double floatVal; -char *stringVal; +CXString stringVal; } EvalData; bool IsUnsignedInt; ~ExprEvalResult() { if (EvalType != CXEval_UnExposed && EvalType != CXEval_Float && EvalType != CXEval_Int) { - delete[] EvalData.stringVal; + clang_disposeString(EvalData.stringVal); } } }; @@ -4657,7 +4657,15 @@ const char *clang_EvalResult_getAsStr(CXEvalResult E) { if (!E) { return nullptr; } - return ((ExprEvalResult *)E)->EvalData.stringVal; + return clang_getCString(((ExprEvalResult *)E)->EvalData.stringVal); +} + +CXString clang_EvalResult_getAsCXString(CXEvalResult E) { + if (!E) { +return cxstring::createNull(); + } + auto data = clang_getCStringInfo(((ExprEvalResult *)E)->EvalData.stringVal); + return cxstring::createDup(StringRef(data.string, data.length)); } static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { @@ -4721,11 +4729,7 @@ static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) { result->EvalType = CXEval_StrLiteral; } - std::string strRef(StrE->getString().str()); - result->EvalData.stringVal = new char[strRef.size() + 1]; - strncpy((char *)result->EvalData.stringVal, strRef.c_str(), - strRef.size()); - result->EvalData.stringVal[strRef.size()] = '\0'; + result->EvalData.stringVal = cxstring::createDup(StrE->getString()); return result.release(); } } else if (expr->ge
[clang] clang_EvalResult_getAsCXString impl (PR #134551)
@@ -44,6 +45,11 @@ typedef struct { unsigned Count; } CXStringSet; +typedef struct { + const char *string; + size_t length; +} CStringInfo; xTachyon wrote: Should we love space for possible expansion? https://github.com/llvm/llvm-project/pull/134551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits