[cfe-dev] Refactoring internal CXString APIs

[Argyrios Kyrtzidis]

On Jan 27, 2013, at 1:59 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> 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().

Sounds good.

> 
> (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)

Are you suggesting that we change places where we return a null string and return an empty one instead ?
I'm a bit uncomfortable with this; while I agree this situation is unfortunate and I don't like it, this is bound to break compatibility with clients where they get an empty string when they only checked for a null string.

For now I suggest also adding "createNull()" and retain current behavior.

> 
> (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?

These are good ideas!

> 
> 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>*/