Skip to content

[libclang] Replace createRef with createDup #126683

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.

Sign up for GitHub

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

[libclang] Replace createRef with createDup #126683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9a801de
[𝘀𝗽𝗿] changes to main this commit is based on
vitalybuka Feb 11, 2025
a8bc4e1
[𝘀𝗽𝗿] initial version
vitalybuka Feb 11, 2025
c5e9141
[𝘀𝗽𝗿] changes introduced through rebase
vitalybuka Feb 13, 2025
f4d53b4
relnote
vitalybuka Feb 13, 2025
70dec50
[𝘀𝗽𝗿] changes introduced through rebase
vitalybuka Feb 13, 2025
69c7eb0
quotes
vitalybuka Feb 13, 2025
b809fd1
[𝘀𝗽𝗿] changes introduced through rebase
vitalybuka Feb 13, 2025
e36e6d7
the
vitalybuka Feb 13, 2025
fc84c46
[𝘀𝗽𝗿] changes introduced through rebase
vitalybuka Feb 13, 2025
41d8f8b
rebase
vitalybuka Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions clang/tools/libclang/CIndex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5066,7 +5066,7 @@ CXString clang_getFileName(CXFile SFile) {
return cxstring::createNull();

FileEntryRef FEnt = *cxfile::getFileEntryRef(SFile);
return cxstring::createRef(FEnt.getName());
return cxstring::createDup(FEnt.getName());
}

time_t clang_getFileTime(CXFile SFile) {
Expand Down Expand Up @@ -5154,7 +5154,7 @@ CXString clang_File_tryGetRealPathName(CXFile SFile) {
return cxstring::createNull();

FileEntryRef FEnt = *cxfile::getFileEntryRef(SFile);
return cxstring::createRef(FEnt.getFileEntry().tryGetRealPathName());
return cxstring::createDup(FEnt.getFileEntry().tryGetRealPathName());
}

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -9234,7 +9234,7 @@ CXString clang_Cursor_getRawCommentText(CXCursor C) {

// Don't duplicate the string because RawText points directly into source
// code.
return cxstring::createRef(RawText);
return cxstring::createDup(RawText);
}

CXString clang_Cursor_getBriefCommentText(CXCursor C) {
Expand All @@ -9250,7 +9250,7 @@ CXString clang_Cursor_getBriefCommentText(CXCursor C) {

// Don't duplicate the string because RawComment ensures that this memory
// will not go away.
return cxstring::createRef(BriefText);
return cxstring::createDup(BriefText);
}

return cxstring::createNull();
Expand Down Expand Up @@ -10081,7 +10081,7 @@ cxindex::Logger::~Logger() {
}

CXString clang_getBinaryOperatorKindSpelling(enum CXBinaryOperatorKind kind) {
return cxstring::createRef(
return cxstring::createDup(
BinaryOperator::getOpcodeStr(static_cast<BinaryOperatorKind>(kind - 1)));
}

Expand All @@ -10100,7 +10100,7 @@ enum CXBinaryOperatorKind clang_getCursorBinaryOperatorKind(CXCursor cursor) {
}

CXString clang_getUnaryOperatorKindSpelling(enum CXUnaryOperatorKind kind) {
return cxstring::createRef(
return cxstring::createDup(
UnaryOperator::getOpcodeStr(static_cast<UnaryOperatorKind>(kind - 1)));
}

Expand Down
4 changes: 2 additions & 2 deletions clang/tools/libclang/CIndexCodeCompletion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ clang_getCompletionParent(CXCompletionString completion_string,
CodeCompletionString *CCStr = (CodeCompletionString *)completion_string;
if (!CCStr)
return cxstring::createNull();
return cxstring::createRef(CCStr->getParentContextName());

return cxstring::createDup(CCStr->getParentContextName());
}

CXString
Expand Down
2 changes: 1 addition & 1 deletion clang/tools/libclang/CIndexDiagnostic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ unsigned clang_getDiagnosticCategory(CXDiagnostic Diag) {

CXString clang_getDiagnosticCategoryName(unsigned Category) {
// Kept for backward compatibility.
return cxstring::createRef(DiagnosticIDs::getCategoryNameFromID(Category));
return cxstring::createDup(DiagnosticIDs::getCategoryNameFromID(Category));
}

CXString clang_getDiagnosticCategoryText(CXDiagnostic Diag) {
Expand Down
24 changes: 12 additions & 12 deletions clang/tools/libclang/CXComment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ CXString clang_TextComment_getText(CXComment CXC) {
if (!TC)
return cxstring::createNull();

return cxstring::createRef(TC->getText());
return cxstring::createDup(TC->getText());
}

CXString clang_InlineCommandComment_getCommandName(CXComment CXC) {
Expand All @@ -138,7 +138,7 @@ CXString clang_InlineCommandComment_getCommandName(CXComment CXC) {
return cxstring::createNull();

const CommandTraits &Traits = getCommandTraits(CXC);
return cxstring::createRef(ICC->getCommandName(Traits));
return cxstring::createDup(ICC->getCommandName(Traits));
}

enum CXCommentInlineCommandRenderKind
Expand Down Expand Up @@ -180,15 +180,15 @@ CXString clang_InlineCommandComment_getArgText(CXComment CXC,
if (!ICC || ArgIdx >= ICC->getNumArgs())
return cxstring::createNull();

return cxstring::createRef(ICC->getArgText(ArgIdx));
return cxstring::createDup(ICC->getArgText(ArgIdx));
}

CXString clang_HTMLTagComment_getTagName(CXComment CXC) {
const HTMLTagComment *HTC = getASTNodeAs<HTMLTagComment>(CXC);
if (!HTC)
return cxstring::createNull();

return cxstring::createRef(HTC->getTagName());
return cxstring::createDup(HTC->getTagName());
}

unsigned clang_HTMLStartTagComment_isSelfClosing(CXComment CXC) {
Expand All @@ -212,15 +212,15 @@ CXString clang_HTMLStartTag_getAttrName(CXComment CXC, unsigned AttrIdx) {
if (!HST || AttrIdx >= HST->getNumAttrs())
return cxstring::createNull();

return cxstring::createRef(HST->getAttr(AttrIdx).Name);
return cxstring::createDup(HST->getAttr(AttrIdx).Name);
}

CXString clang_HTMLStartTag_getAttrValue(CXComment CXC, unsigned AttrIdx) {
const HTMLStartTagComment *HST = getASTNodeAs<HTMLStartTagComment>(CXC);
if (!HST || AttrIdx >= HST->getNumAttrs())
return cxstring::createNull();

return cxstring::createRef(HST->getAttr(AttrIdx).Value);
return cxstring::createDup(HST->getAttr(AttrIdx).Value);
}

CXString clang_BlockCommandComment_getCommandName(CXComment CXC) {
Expand All @@ -229,7 +229,7 @@ CXString clang_BlockCommandComment_getCommandName(CXComment CXC) {
return cxstring::createNull();

const CommandTraits &Traits = getCommandTraits(CXC);
return cxstring::createRef(BCC->getCommandName(Traits));
return cxstring::createDup(BCC->getCommandName(Traits));
}

unsigned clang_BlockCommandComment_getNumArgs(CXComment CXC) {
Expand All @@ -246,7 +246,7 @@ CXString clang_BlockCommandComment_getArgText(CXComment CXC,
if (!BCC || ArgIdx >= BCC->getNumArgs())
return cxstring::createNull();

return cxstring::createRef(BCC->getArgText(ArgIdx));
return cxstring::createDup(BCC->getArgText(ArgIdx));
}

CXComment clang_BlockCommandComment_getParagraph(CXComment CXC) {
Expand All @@ -262,7 +262,7 @@ CXString clang_ParamCommandComment_getParamName(CXComment CXC) {
if (!PCC || !PCC->hasParamName())
return cxstring::createNull();

return cxstring::createRef(PCC->getParamNameAsWritten());
return cxstring::createDup(PCC->getParamNameAsWritten());
}

unsigned clang_ParamCommandComment_isParamIndexValid(CXComment CXC) {
Expand Down Expand Up @@ -313,7 +313,7 @@ CXString clang_TParamCommandComment_getParamName(CXComment CXC) {
if (!TPCC || !TPCC->hasParamName())
return cxstring::createNull();

return cxstring::createRef(TPCC->getParamNameAsWritten());
return cxstring::createDup(TPCC->getParamNameAsWritten());
}

unsigned clang_TParamCommandComment_isParamPositionValid(CXComment CXC) {
Expand Down Expand Up @@ -346,15 +346,15 @@ CXString clang_VerbatimBlockLineComment_getText(CXComment CXC) {
if (!VBL)
return cxstring::createNull();

return cxstring::createRef(VBL->getText());
return cxstring::createDup(VBL->getText());
}

CXString clang_VerbatimLineComment_getText(CXComment CXC) {
const VerbatimLineComment *VLC = getASTNodeAs<VerbatimLineComment>(CXC);
if (!VLC)
return cxstring::createNull();

return cxstring::createRef(VLC->getText());
return cxstring::createDup(VLC->getText());
}

//===----------------------------------------------------------------------===//
Expand Down
4 changes: 2 additions & 2 deletions clang/tools/libclang/CXStoredDiagnostic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ CXSourceLocation CXStoredDiagnostic::getLocation() const {
}

CXString CXStoredDiagnostic::getSpelling() const {
return cxstring::createRef(Diag.getMessage());
return cxstring::createDup(Diag.getMessage());
}

CXString CXStoredDiagnostic::getDiagnosticOption(CXString *Disable) const {
Expand Down Expand Up @@ -75,7 +75,7 @@ unsigned CXStoredDiagnostic::getCategory() const {

CXString CXStoredDiagnostic::getCategoryText() const {
unsigned catID = DiagnosticIDs::getCategoryNumberForDiag(Diag.getID());
return cxstring::createRef(DiagnosticIDs::getCategoryNameFromID(catID));
return cxstring::createDup(DiagnosticIDs::getCategoryNameFromID(catID));
}

unsigned CXStoredDiagnostic::getNumRanges() const {
Expand Down
13 changes: 0 additions & 13 deletions clang/tools/libclang/CXString.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,6 @@ CXString createDup(const char *String) {
return Str;
}

CXString createRef(StringRef String) {
if (!String.data())
return createNull();

// If the string is empty, it might point to a position in another string
// while having zero length. Make sure we don't create a reference to the
// larger string.
if (String.empty())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is losing the above two checks not problematic? It isn't clear to me actually what this is even for, @AaronBallman I think knows more about libclang than I do though, so perhaps he should review this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We're not losing those checks -- createDup() has them:

llvm-project/clang/tools/libclang/CXString.cpp

Line 67 in 8337d01

CXString createDup(const char *String) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thats only for the const char* version. Isn't the version on line 93/80 the one that will be called instead?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I see what you mean, that is a difference in behavior for the null case because clang_getCString() (part of the stable API) would start returning a non-null pointer where it previously returned a null pointer. I'm not certain if any of the changed calls to createDup() will get a null StringRef though, that's a pretty odd beast.

return createEmpty();

return createDup(String);
}

CXString createDup(StringRef String) {
CXString Result;
char *Spelling = static_cast<char *>(llvm::safe_malloc(String.size() + 1));
Expand Down
6 changes: 0 additions & 6 deletions clang/tools/libclang/CXString.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ CXString createRef(const char *String);
/// \p String can be changed or freed by the caller.
CXString createDup(const char *String);

/// Create a CXString object from a StringRef. New CXString may
/// contain a pointer to the undrelying data of \p String.
///
/// \p String should not be changed by the caller afterwards.
CXString createRef(StringRef String);

/// Create a CXString object from a StringRef. New CXString will
/// contain a copy of \p String.
///
Expand Down
Loading