[cfe-dev] Refactoring internal CXString APIs

Dmitri Gribenko gribozavr at gmail.com
Sun Jan 27 13:59:59 PST 2013


Hello Argyrios and cfe-dev,

In order to fix the bug discussed in "createCXString reads one past
end byte", we need change CXString internal APIs: when creating a
CXString from a StringRef, we also need a TU, to find the StringPool.

I see this as a good opportunity to refactor internal CXString APIs.
What I want to do is to change current createCXString to be more
intuitive.

(1) Since these functions are in cxstring namespace, I want to drop
CXString from the name, *and* remove "using namespace cxstring"
everywhere.  All calls would become cxstring::createWhatever().

(2) Introduce CXString createEmpty();  There are more than 70 places
in libclang where we create empty CXStrings and we 4 different ways to
do exactly the same thing:

createCXString("")
createCXString("", false)
createCXString((const char *) 0)
createCXString((const char *) NULL)

(3) libclang code is full of comments explaining the meaning of the
second parameter to createCXString:

createCXString(Cmd->CommandLine[Arg].c_str(), /*DupString=*/false)

If the API needs that, we should consider changing the API :)  And we
easily can:

CXString createRef(const char *String);
CXString createDup(const char *String);

CXString createRef(StringRef String);
CXString createDup(StringRef String);

CXString createRef(CXStringBuf *buf);

(4) After fixing that bug, create...(StringRef) will accept an
additional parameter -- TU.

(5) We can also change createDup(const char *String) to accept a TU,
so that we will use our string pool in all cases.

What do you think?

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/



More information about the cfe-dev mailing list