r174263 - libclang: introduce cxstring::{createRef, createDup} for StringRefs

Evgeniy Stepanov eugeni.stepanov at gmail.com
Tue Feb 19 03:58:33 PST 2013


Hi,

the code at CXString.cpp:85 does not look good at all. It reads 1 byte
past the end of the string, which may belong to a different
allocation, and, after some time, that '\0' may not be there anymore!

I see this with MemorySanitizer thanks to the annotations in BumpPtrAllocator.


On Sat, Feb 2, 2013 at 6:19 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> Author: gribozavr
> Date: Fri Feb  1 20:19:29 2013
> New Revision: 174263
>
> URL: http://llvm.org/viewvc/llvm-project?rev=174263&view=rev
> Log:
> libclang: introduce cxstring::{createRef,createDup} for StringRefs
>
> Also migrate all clients from the old API.
>
> Modified:
>     cfe/trunk/tools/libclang/ARCMigrate.cpp
>     cfe/trunk/tools/libclang/CIndex.cpp
>     cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp
>     cfe/trunk/tools/libclang/CIndexDiagnostic.cpp
>     cfe/trunk/tools/libclang/CIndexUSRs.cpp
>     cfe/trunk/tools/libclang/CXComment.cpp
>     cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp
>     cfe/trunk/tools/libclang/CXStoredDiagnostic.cpp
>     cfe/trunk/tools/libclang/CXString.cpp
>     cfe/trunk/tools/libclang/CXString.h
>     cfe/trunk/tools/libclang/CXType.cpp
>
> Modified: cfe/trunk/tools/libclang/ARCMigrate.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/ARCMigrate.cpp?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/ARCMigrate.cpp (original)
> +++ cfe/trunk/tools/libclang/ARCMigrate.cpp Fri Feb  1 20:19:29 2013
> @@ -122,13 +122,11 @@ unsigned clang_remap_getNumFiles(CXRemap
>  void clang_remap_getFilenames(CXRemapping map, unsigned index,
>                                CXString *original, CXString *transformed) {
>    if (original)
> -    *original = cxstring::createCXString(
> -                                    static_cast<Remap *>(map)->Vec[index].first,
> -                                        /*DupString =*/ true);
> +    *original = cxstring::createDup(
> +                    static_cast<Remap *>(map)->Vec[index].first);
>    if (transformed)
> -    *transformed = cxstring::createCXString(
> -                                   static_cast<Remap *>(map)->Vec[index].second,
> -                                  /*DupString =*/ true);
> +    *transformed = cxstring::createDup(
> +                    static_cast<Remap *>(map)->Vec[index].second);
>  }
>
>  void clang_remap_dispose(CXRemapping map) {
>
> Modified: cfe/trunk/tools/libclang/CIndex.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CIndex.cpp (original)
> +++ cfe/trunk/tools/libclang/CIndex.cpp Fri Feb  1 20:19:29 2013
> @@ -2918,7 +2918,7 @@ CXString clang_getTranslationUnitSpellin
>      return cxstring::createEmpty();
>
>    ASTUnit *CXXUnit = cxtu::getASTUnit(CTUnit);
> -  return createCXString(CXXUnit->getOriginalSourceFileName(), true);
> +  return cxstring::createDup(CXXUnit->getOriginalSourceFileName());
>  }
>
>  CXCursor clang_getTranslationUnitCursor(CXTranslationUnit TU) {
> @@ -3114,17 +3114,17 @@ static CXString getDeclSpelling(const De
>      if (const ObjCPropertyImplDecl *PropImpl =
>              dyn_cast<ObjCPropertyImplDecl>(D))
>        if (ObjCPropertyDecl *Property = PropImpl->getPropertyDecl())
> -        return createCXString(Property->getIdentifier()->getName());
> +        return cxstring::createDup(Property->getIdentifier()->getName());
>
>      if (const ImportDecl *ImportD = dyn_cast<ImportDecl>(D))
>        if (Module *Mod = ImportD->getImportedModule())
> -        return createCXString(Mod->getFullModuleName());
> +        return cxstring::createDup(Mod->getFullModuleName());
>
>      return cxstring::createEmpty();
>    }
>
>    if (const ObjCMethodDecl *OMD = dyn_cast<ObjCMethodDecl>(ND))
> -    return createCXString(OMD->getSelector().getAsString());
> +    return cxstring::createDup(OMD->getSelector().getAsString());
>
>    if (const ObjCCategoryImplDecl *CIMP = dyn_cast<ObjCCategoryImplDecl>(ND))
>      // No, this isn't the same as the code below. getIdentifier() is non-virtual
> @@ -3139,7 +3139,7 @@ static CXString getDeclSpelling(const De
>    llvm::raw_svector_ostream os(S);
>    ND->printName(os);
>
> -  return createCXString(os.str());
> +  return cxstring::createDup(os.str());
>  }
>
>  CXString clang_getCursorSpelling(CXCursor C) {
> @@ -3163,34 +3163,34 @@ CXString clang_getCursorSpelling(CXCurso
>      }
>      case CXCursor_CXXBaseSpecifier: {
>        const CXXBaseSpecifier *B = getCursorCXXBaseSpecifier(C);
> -      return createCXString(B->getType().getAsString());
> +      return cxstring::createDup(B->getType().getAsString());
>      }
>      case CXCursor_TypeRef: {
>        const TypeDecl *Type = getCursorTypeRef(C).first;
>        assert(Type && "Missing type decl");
>
> -      return createCXString(getCursorContext(C).getTypeDeclType(Type).
> +      return cxstring::createDup(getCursorContext(C).getTypeDeclType(Type).
>                                getAsString());
>      }
>      case CXCursor_TemplateRef: {
>        const TemplateDecl *Template = getCursorTemplateRef(C).first;
>        assert(Template && "Missing template decl");
>
> -      return createCXString(Template->getNameAsString());
> +      return cxstring::createDup(Template->getNameAsString());
>      }
>
>      case CXCursor_NamespaceRef: {
>        const NamedDecl *NS = getCursorNamespaceRef(C).first;
>        assert(NS && "Missing namespace decl");
>
> -      return createCXString(NS->getNameAsString());
> +      return cxstring::createDup(NS->getNameAsString());
>      }
>
>      case CXCursor_MemberRef: {
>        const FieldDecl *Field = getCursorMemberRef(C).first;
>        assert(Field && "Missing member decl");
>
> -      return createCXString(Field->getNameAsString());
> +      return cxstring::createDup(Field->getNameAsString());
>      }
>
>      case CXCursor_LabelRef: {
> @@ -3204,23 +3204,23 @@ CXString clang_getCursorSpelling(CXCurso
>        OverloadedDeclRefStorage Storage = getCursorOverloadedDeclRef(C).first;
>        if (const Decl *D = Storage.dyn_cast<const Decl *>()) {
>          if (const NamedDecl *ND = dyn_cast<NamedDecl>(D))
> -          return createCXString(ND->getNameAsString());
> +          return cxstring::createDup(ND->getNameAsString());
>          return cxstring::createEmpty();
>        }
>        if (const OverloadExpr *E = Storage.dyn_cast<const OverloadExpr *>())
> -        return createCXString(E->getName().getAsString());
> +        return cxstring::createDup(E->getName().getAsString());
>        OverloadedTemplateStorage *Ovl
>          = Storage.get<OverloadedTemplateStorage*>();
>        if (Ovl->size() == 0)
>          return cxstring::createEmpty();
> -      return createCXString((*Ovl->begin())->getNameAsString());
> +      return cxstring::createDup((*Ovl->begin())->getNameAsString());
>      }
>
>      case CXCursor_VariableRef: {
>        const VarDecl *Var = getCursorVariableRef(C).first;
>        assert(Var && "Missing variable decl");
>
> -      return createCXString(Var->getNameAsString());
> +      return cxstring::createDup(Var->getNameAsString());
>      }
>
>      default:
> @@ -3252,19 +3252,19 @@ CXString clang_getCursorSpelling(CXCurso
>                                                             ->getNameStart());
>
>    if (C.kind == CXCursor_InclusionDirective)
> -    return createCXString(getCursorInclusionDirective(C)->getFileName());
> +    return cxstring::createDup(getCursorInclusionDirective(C)->getFileName());
>
>    if (clang_isDeclaration(C.kind))
>      return getDeclSpelling(getCursorDecl(C));
>
>    if (C.kind == CXCursor_AnnotateAttr) {
>      const AnnotateAttr *AA = cast<AnnotateAttr>(cxcursor::getCursorAttr(C));
> -    return createCXString(AA->getAnnotation());
> +    return cxstring::createDup(AA->getAnnotation());
>    }
>
>    if (C.kind == CXCursor_AsmLabelAttr) {
>      const AsmLabelAttr *AA = cast<AsmLabelAttr>(cxcursor::getCursorAttr(C));
> -    return createCXString(AA->getLabel());
> +    return cxstring::createDup(AA->getLabel());
>    }
>
>    return cxstring::createEmpty();
> @@ -3383,7 +3383,7 @@ CXString clang_getCursorDisplayName(CXCu
>        OS << "...";
>      }
>      OS << ")";
> -    return createCXString(OS.str());
> +    return cxstring::createDup(OS.str());
>    }
>
>    if (const ClassTemplateDecl *ClassTemplate = dyn_cast<ClassTemplateDecl>(D)) {
> @@ -3414,14 +3414,14 @@ CXString clang_getCursorDisplayName(CXCu
>      }
>
>      OS << ">";
> -    return createCXString(OS.str());
> +    return cxstring::createDup(OS.str());
>    }
>
>    if (const ClassTemplateSpecializationDecl *ClassSpec
>                                = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
>      // If the type was explicitly written, use that.
>      if (TypeSourceInfo *TSInfo = ClassSpec->getTypeAsWritten())
> -      return createCXString(TSInfo->getType().getAsString(Policy));
> +      return cxstring::createDup(TSInfo->getType().getAsString(Policy));
>
>      SmallString<64> Str;
>      llvm::raw_svector_ostream OS(Str);
> @@ -3430,7 +3430,7 @@ CXString clang_getCursorDisplayName(CXCu
>                                        ClassSpec->getTemplateArgs().data(),
>                                        ClassSpec->getTemplateArgs().size(),
>                                                                  Policy);
> -    return createCXString(OS.str());
> +    return cxstring::createDup(OS.str());
>    }
>
>    return clang_getCursorSpelling(C);
> @@ -4780,7 +4780,7 @@ CXString clang_getTokenSpelling(CXTransl
>    case CXToken_Literal: {
>      // We have stashed the starting pointer in the ptr_data field. Use it.
>      const char *Text = static_cast<const char *>(CXTok.ptr_data);
> -    return createCXString(StringRef(Text, CXTok.int_data[2]));
> +    return cxstring::createDup(StringRef(Text, CXTok.int_data[2]));
>    }
>
>    case CXToken_Punctuation:
> @@ -4803,7 +4803,7 @@ CXString clang_getTokenSpelling(CXTransl
>    if (Invalid)
>      return cxstring::createEmpty();
>
> -  return createCXString(Buffer.substr(LocInfo.second, CXTok.int_data[2]));
> +  return cxstring::createDup(Buffer.substr(LocInfo.second, CXTok.int_data[2]));
>  }
>
>  CXSourceLocation clang_getTokenLocation(CXTranslationUnit TU, CXToken CXTok) {
> @@ -5754,7 +5754,7 @@ int clang_getCursorPlatformAvailability(
>        if (always_deprecated)
>          *always_deprecated = 1;
>        if (deprecated_message)
> -        *deprecated_message = cxstring::createCXString(Deprecated->getMessage());
> +        *deprecated_message = cxstring::createDup(Deprecated->getMessage());
>        continue;
>      }
>
> @@ -5762,8 +5762,7 @@ int clang_getCursorPlatformAvailability(
>        if (always_unavailable)
>          *always_unavailable = 1;
>        if (unavailable_message) {
> -        *unavailable_message
> -          = cxstring::createCXString(Unavailable->getMessage());
> +        *unavailable_message = cxstring::createDup(Unavailable->getMessage());
>        }
>        continue;
>      }
> @@ -5771,12 +5770,12 @@ int clang_getCursorPlatformAvailability(
>      if (AvailabilityAttr *Avail = dyn_cast<AvailabilityAttr>(*A)) {
>        if (N < availability_size) {
>          availability[N].Platform
> -          = cxstring::createCXString(Avail->getPlatform()->getName());
> +          = cxstring::createDup(Avail->getPlatform()->getName());
>          availability[N].Introduced = convertVersion(Avail->getIntroduced());
>          availability[N].Deprecated = convertVersion(Avail->getDeprecated());
>          availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
>          availability[N].Unavailable = Avail->getUnavailable();
> -        availability[N].Message = cxstring::createCXString(Avail->getMessage());
> +        availability[N].Message = cxstring::createDup(Avail->getMessage());
>        }
>        ++N;
>      }
> @@ -5885,7 +5884,7 @@ CXString clang_Cursor_getRawCommentText(
>
>    // Don't duplicate the string because RawText points directly into source
>    // code.
> -  return createCXString(RawText, false);
> +  return cxstring::createRef(RawText);
>  }
>
>  CXString clang_Cursor_getBriefCommentText(CXCursor C) {
> @@ -5901,7 +5900,7 @@ CXString clang_Cursor_getBriefCommentTex
>
>      // Don't duplicate the string because RawComment ensures that this memory
>      // will not go away.
> -    return createCXString(BriefText, false);
> +    return cxstring::createRef(BriefText);
>    }
>
>    return cxstring::createNull();
> @@ -5939,14 +5938,14 @@ CXString clang_Module_getName(CXModule C
>    if (!CXMod)
>      return cxstring::createEmpty();
>    Module *Mod = static_cast<Module*>(CXMod);
> -  return createCXString(Mod->Name);
> +  return cxstring::createDup(Mod->Name);
>  }
>
>  CXString clang_Module_getFullName(CXModule CXMod) {
>    if (!CXMod)
>      return cxstring::createEmpty();
>    Module *Mod = static_cast<Module*>(CXMod);
> -  return createCXString(Mod->getFullModuleName());
> +  return cxstring::createDup(Mod->getFullModuleName());
>  }
>
>  unsigned clang_Module_getNumTopLevelHeaders(CXModule CXMod) {
> @@ -6348,7 +6347,7 @@ MacroDefinition *cxindex::checkForMacroI
>  extern "C" {
>
>  CXString clang_getClangVersion() {
> -  return createCXString(getClangFullVersion());
> +  return cxstring::createDup(getClangFullVersion());
>  }
>
>  } // end: extern "C"
>
> Modified: cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp (original)
> +++ cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp Fri Feb  1 20:19:29 2013
> @@ -224,7 +224,7 @@ clang_getCompletionParent(CXCompletionSt
>    if (!CCStr)
>      return cxstring::createNull();
>
> -  return createCXString(CCStr->getParentContextName(), /*DupString=*/false);
> +  return cxstring::createRef(CCStr->getParentContextName());
>  }
>
>  CXString
> @@ -923,7 +923,7 @@ CXString clang_codeCompleteGetObjCSelect
>    if (!Results)
>      return cxstring::createEmpty();
>
> -  return createCXString(Results->Selector);
> +  return cxstring::createDup(Results->Selector);
>  }
>
>  } // end extern "C"
>
> Modified: cfe/trunk/tools/libclang/CIndexDiagnostic.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndexDiagnostic.cpp?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CIndexDiagnostic.cpp (original)
> +++ cfe/trunk/tools/libclang/CIndexDiagnostic.cpp Fri Feb  1 20:19:29 2013
> @@ -62,7 +62,7 @@ public:
>    }
>
>    CXString getSpelling() const {
> -    return createCXString(StringRef(Message), false);
> +    return cxstring::createRef(Message.c_str());
>    }
>
>    CXString getDiagnosticOption(CXString *Disable) const {
> @@ -354,7 +354,7 @@ CXString clang_formatDiagnostic(CXDiagno
>        Out << "]";
>    }
>
> -  return createCXString(Out.str(), true);
> +  return cxstring::createDup(Out.str());
>  }
>
>  unsigned clang_defaultDiagnosticDisplayOptions() {
> @@ -398,7 +398,7 @@ unsigned clang_getDiagnosticCategory(CXD
>
>  CXString clang_getDiagnosticCategoryName(unsigned Category) {
>    // Kept for backwards compatibility.
> -  return createCXString(DiagnosticIDs::getCategoryNameFromID(Category));
> +  return cxstring::createRef(DiagnosticIDs::getCategoryNameFromID(Category));
>  }
>
>  CXString clang_getDiagnosticCategoryText(CXDiagnostic Diag) {
>
> Modified: cfe/trunk/tools/libclang/CIndexUSRs.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndexUSRs.cpp?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CIndexUSRs.cpp (original)
> +++ cfe/trunk/tools/libclang/CIndexUSRs.cpp Fri Feb  1 20:19:29 2013
> @@ -871,7 +871,7 @@ CXString clang_constructUSR_ObjCIvar(con
>    USRGenerator UG;
>    UG << extractUSRSuffix(clang_getCString(classUSR));
>    UG->GenObjCIvar(name);
> -  return createCXString(UG.str(), true);
> +  return cxstring::createDup(UG.str());
>  }
>
>  CXString clang_constructUSR_ObjCMethod(const char *name,
> @@ -880,26 +880,26 @@ CXString clang_constructUSR_ObjCMethod(c
>    USRGenerator UG;
>    UG << extractUSRSuffix(clang_getCString(classUSR));
>    UG->GenObjCMethod(name, isInstanceMethod);
> -  return createCXString(UG.str(), true);
> +  return cxstring::createDup(UG.str());
>  }
>
>  CXString clang_constructUSR_ObjCClass(const char *name) {
>    USRGenerator UG;
>    UG->GenObjCClass(name);
> -  return createCXString(UG.str(), true);
> +  return cxstring::createDup(UG.str());
>  }
>
>  CXString clang_constructUSR_ObjCProtocol(const char *name) {
>    USRGenerator UG;
>    UG->GenObjCProtocol(name);
> -  return createCXString(UG.str(), true);
> +  return cxstring::createDup(UG.str());
>  }
>
>  CXString clang_constructUSR_ObjCCategory(const char *class_name,
>                                           const char *category_name) {
>    USRGenerator UG;
>    UG->GenObjCCategory(class_name, category_name);
> -  return createCXString(UG.str(), true);
> +  return cxstring::createDup(UG.str());
>  }
>
>  CXString clang_constructUSR_ObjCProperty(const char *property,
> @@ -907,7 +907,7 @@ CXString clang_constructUSR_ObjCProperty
>    USRGenerator UG;
>    UG << extractUSRSuffix(clang_getCString(classUSR));
>    UG->GenObjCProperty(property);
> -  return createCXString(UG.str(), true);
> +  return cxstring::createDup(UG.str());
>  }
>
>  } // end extern "C"
>
> Modified: cfe/trunk/tools/libclang/CXComment.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXComment.cpp?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CXComment.cpp (original)
> +++ cfe/trunk/tools/libclang/CXComment.cpp Fri Feb  1 20:19:29 2013
> @@ -126,7 +126,7 @@ CXString clang_TextComment_getText(CXCom
>    if (!TC)
>      return cxstring::createNull();
>
> -  return createCXString(TC->getText(), /*DupString=*/ false);
> +  return cxstring::createRef(TC->getText());
>  }
>
>  CXString clang_InlineCommandComment_getCommandName(CXComment CXC) {
> @@ -135,7 +135,7 @@ CXString clang_InlineCommandComment_getC
>      return cxstring::createNull();
>
>    const CommandTraits &Traits = getCommandTraits(CXC);
> -  return createCXString(ICC->getCommandName(Traits), /*DupString=*/ false);
> +  return cxstring::createRef(ICC->getCommandName(Traits));
>  }
>
>  enum CXCommentInlineCommandRenderKind
> @@ -174,7 +174,7 @@ CXString clang_InlineCommandComment_getA
>    if (!ICC || ArgIdx >= ICC->getNumArgs())
>      return cxstring::createNull();
>
> -  return createCXString(ICC->getArgText(ArgIdx), /*DupString=*/ false);
> +  return cxstring::createRef(ICC->getArgText(ArgIdx));
>  }
>
>  CXString clang_HTMLTagComment_getTagName(CXComment CXC) {
> @@ -182,7 +182,7 @@ CXString clang_HTMLTagComment_getTagName
>    if (!HTC)
>      return cxstring::createNull();
>
> -  return createCXString(HTC->getTagName(), /*DupString=*/ false);
> +  return cxstring::createRef(HTC->getTagName());
>  }
>
>  unsigned clang_HTMLStartTagComment_isSelfClosing(CXComment CXC) {
> @@ -206,7 +206,7 @@ CXString clang_HTMLStartTag_getAttrName(
>    if (!HST || AttrIdx >= HST->getNumAttrs())
>      return cxstring::createNull();
>
> -  return createCXString(HST->getAttr(AttrIdx).Name, /*DupString=*/ false);
> +  return cxstring::createRef(HST->getAttr(AttrIdx).Name);
>  }
>
>  CXString clang_HTMLStartTag_getAttrValue(CXComment CXC, unsigned AttrIdx) {
> @@ -214,7 +214,7 @@ CXString clang_HTMLStartTag_getAttrValue
>    if (!HST || AttrIdx >= HST->getNumAttrs())
>      return cxstring::createNull();
>
> -  return createCXString(HST->getAttr(AttrIdx).Value, /*DupString=*/ false);
> +  return cxstring::createRef(HST->getAttr(AttrIdx).Value);
>  }
>
>  CXString clang_BlockCommandComment_getCommandName(CXComment CXC) {
> @@ -223,7 +223,7 @@ CXString clang_BlockCommandComment_getCo
>      return cxstring::createNull();
>
>    const CommandTraits &Traits = getCommandTraits(CXC);
> -  return createCXString(BCC->getCommandName(Traits), /*DupString=*/ false);
> +  return cxstring::createRef(BCC->getCommandName(Traits));
>  }
>
>  unsigned clang_BlockCommandComment_getNumArgs(CXComment CXC) {
> @@ -240,7 +240,7 @@ CXString clang_BlockCommandComment_getAr
>    if (!BCC || ArgIdx >= BCC->getNumArgs())
>      return cxstring::createNull();
>
> -  return createCXString(BCC->getArgText(ArgIdx), /*DupString=*/ false);
> +  return cxstring::createRef(BCC->getArgText(ArgIdx));
>  }
>
>  CXComment clang_BlockCommandComment_getParagraph(CXComment CXC) {
> @@ -256,7 +256,7 @@ CXString clang_ParamCommandComment_getPa
>    if (!PCC || !PCC->hasParamName())
>      return cxstring::createNull();
>
> -  return createCXString(PCC->getParamNameAsWritten(), /*DupString=*/ false);
> +  return cxstring::createRef(PCC->getParamNameAsWritten());
>  }
>
>  unsigned clang_ParamCommandComment_isParamIndexValid(CXComment CXC) {
> @@ -307,7 +307,7 @@ CXString clang_TParamCommandComment_getP
>    if (!TPCC || !TPCC->hasParamName())
>      return cxstring::createNull();
>
> -  return createCXString(TPCC->getParamNameAsWritten(), /*DupString=*/ false);
> +  return cxstring::createRef(TPCC->getParamNameAsWritten());
>  }
>
>  unsigned clang_TParamCommandComment_isParamPositionValid(CXComment CXC) {
> @@ -340,7 +340,7 @@ CXString clang_VerbatimBlockLineComment_
>    if (!VBL)
>      return cxstring::createNull();
>
> -  return createCXString(VBL->getText(), /*DupString=*/ false);
> +  return cxstring::createRef(VBL->getText());
>  }
>
>  CXString clang_VerbatimLineComment_getText(CXComment CXC) {
> @@ -348,7 +348,7 @@ CXString clang_VerbatimLineComment_getTe
>    if (!VLC)
>      return cxstring::createNull();
>
> -  return createCXString(VLC->getText(), /*DupString=*/ false);
> +  return cxstring::createRef(VLC->getText());
>  }
>
>  } // end extern "C"
> @@ -843,7 +843,7 @@ CXString clang_HTMLTagComment_getAsStrin
>    SmallString<128> HTML;
>    CommentASTToHTMLConverter Converter(0, HTML, getCommandTraits(CXC));
>    Converter.visit(HTC);
> -  return createCXString(HTML.str(), /* DupString = */ true);
> +  return cxstring::createDup(HTML.str());
>  }
>
>  CXString clang_FullComment_getAsHTML(CXComment CXC) {
> @@ -854,7 +854,7 @@ CXString clang_FullComment_getAsHTML(CXC
>    SmallString<1024> HTML;
>    CommentASTToHTMLConverter Converter(FC, HTML, getCommandTraits(CXC));
>    Converter.visit(FC);
> -  return createCXString(HTML.str(), /* DupString = */ true);
> +  return cxstring::createDup(HTML.str());
>  }
>
>  } // end extern "C"
> @@ -1435,7 +1435,7 @@ CXString clang_FullComment_getAsXML(CXCo
>                                       *TU->FormatContext,
>                                       TU->FormatInMemoryUniqueId++);
>    Converter.visit(FC);
> -  return createCXString(XML.str(), /* DupString = */ true);
> +  return cxstring::createDup(XML.str());
>  }
>
>  } // end extern "C"
>
> Modified: cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp (original)
> +++ cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp Fri Feb  1 20:19:29 2013
> @@ -97,7 +97,7 @@ CXSourceLocation CXLoadedDiagnostic::get
>  }
>
>  CXString CXLoadedDiagnostic::getSpelling() const {
> -  return cxstring::createCXString(Spelling, false);
> +  return cxstring::createRef(Spelling);
>  }
>
>  CXString CXLoadedDiagnostic::getDiagnosticOption(CXString *Disable) const {
> @@ -106,8 +106,8 @@ CXString CXLoadedDiagnostic::getDiagnost
>
>    // FIXME: possibly refactor with logic in CXStoredDiagnostic.
>    if (Disable)
> -    *Disable = createCXString((Twine("-Wno-") + DiagOption).str());
> -  return createCXString((Twine("-W") + DiagOption).str());
> +    *Disable = cxstring::createDup((Twine("-Wno-") + DiagOption).str());
> +  return cxstring::createDup((Twine("-W") + DiagOption).str());
>  }
>
>  unsigned CXLoadedDiagnostic::getCategory() const {
> @@ -115,7 +115,7 @@ unsigned CXLoadedDiagnostic::getCategory
>  }
>
>  CXString CXLoadedDiagnostic::getCategoryText() const {
> -  return cxstring::createCXString(CategoryText);
> +  return cxstring::createDup(CategoryText);
>  }
>
>  unsigned CXLoadedDiagnostic::getNumRanges() const {
> @@ -195,7 +195,7 @@ class DiagLoader {
>      if (error)
>        *error = code;
>      if (errorString)
> -      *errorString = createCXString(err);
> +      *errorString = cxstring::createDup(err);
>    }
>
>    void reportInvalidFile(llvm::StringRef err) {
> @@ -627,7 +627,7 @@ LoadResult DiagLoader::readDiagnosticBlo
>          if (readString(TopDiags, RetStr, "FIXIT", Record, Blob,
>                         /* allowEmptyString */ true))
>            return Failure;
> -        D->FixIts.push_back(std::make_pair(SR, createCXString(RetStr, false)));
> +        D->FixIts.push_back(std::make_pair(SR, cxstring::createRef(RetStr)));
>          continue;
>        }
>
>
> Modified: cfe/trunk/tools/libclang/CXStoredDiagnostic.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXStoredDiagnostic.cpp?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CXStoredDiagnostic.cpp (original)
> +++ cfe/trunk/tools/libclang/CXStoredDiagnostic.cpp Fri Feb  1 20:19:29 2013
> @@ -49,7 +49,7 @@ CXSourceLocation CXStoredDiagnostic::get
>  }
>
>  CXString CXStoredDiagnostic::getSpelling() const {
> -  return createCXString(Diag.getMessage(), false);
> +  return cxstring::createRef(Diag.getMessage());
>  }
>
>  CXString CXStoredDiagnostic::getDiagnosticOption(CXString *Disable) const {
> @@ -57,8 +57,8 @@ CXString CXStoredDiagnostic::getDiagnost
>    StringRef Option = DiagnosticIDs::getWarningOptionForDiag(ID);
>    if (!Option.empty()) {
>      if (Disable)
> -      *Disable = createCXString((Twine("-Wno-") + Option).str());
> -    return createCXString((Twine("-W") + Option).str());
> +      *Disable = cxstring::createDup((Twine("-Wno-") + Option).str());
> +    return cxstring::createDup((Twine("-W") + Option).str());
>    }
>
>    if (ID == diag::fatal_too_many_errors) {
> @@ -76,7 +76,7 @@ unsigned CXStoredDiagnostic::getCategory
>
>  CXString CXStoredDiagnostic::getCategoryText() const {
>    unsigned catID = DiagnosticIDs::getCategoryNumberForDiag(Diag.getID());
> -  return createCXString(DiagnosticIDs::getCategoryNameFromID(catID));
> +  return cxstring::createRef(DiagnosticIDs::getCategoryNameFromID(catID));
>  }
>
>  unsigned CXStoredDiagnostic::getNumRanges() const {
> @@ -109,6 +109,6 @@ CXString CXStoredDiagnostic::getFixIt(un
>      *ReplacementRange = translateSourceRange(Diag.getLocation().getManager(),
>                                               LangOpts, Hint.RemoveRange);
>    }
> -  return createCXString(Hint.CodeToInsert);
> +  return cxstring::createDup(Hint.CodeToInsert);
>  }
>
>
> Modified: cfe/trunk/tools/libclang/CXString.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXString.cpp?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CXString.cpp (original)
> +++ cfe/trunk/tools/libclang/CXString.cpp Fri Feb  1 20:19:29 2013
> @@ -77,18 +77,25 @@ CXString cxstring::createDup(const char
>    return Str;
>  }
>
> -CXString cxstring::createCXString(StringRef String, bool DupString) {
> +CXString cxstring::createRef(StringRef String) {
> +  // If the string is not nul-terminated, we have to make a copy.
> +  // This is doing a one past end read, and should be removed!
> +  if (!String.empty() && String.data()[String.size()] != 0)
> +    return cxstring::createDup(String);
> +
>    CXString Result;
> -  if (DupString || (!String.empty() && String.data()[String.size()] != 0)) {
> -    char *Spelling = static_cast<char *>(malloc(String.size() + 1));
> -    memmove(Spelling, String.data(), String.size());
> -    Spelling[String.size()] = 0;
> -    Result.data = Spelling;
> -    Result.private_flags = (unsigned) CXS_Malloc;
> -  } else {
> -    Result.data = String.data();
> -    Result.private_flags = (unsigned) CXS_Unmanaged;
> -  }
> +  Result.data = String.data();
> +  Result.private_flags = (unsigned) CXS_Unmanaged;
> +  return Result;
> +}
> +
> +CXString cxstring::createDup(StringRef String) {
> +  CXString Result;
> +  char *Spelling = static_cast<char *>(malloc(String.size() + 1));
> +  memmove(Spelling, String.data(), String.size());
> +  Spelling[String.size()] = 0;
> +  Result.data = Spelling;
> +  Result.private_flags = (unsigned) CXS_Malloc;
>    return Result;
>  }
>
>
> Modified: cfe/trunk/tools/libclang/CXString.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXString.h?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CXString.h (original)
> +++ cfe/trunk/tools/libclang/CXString.h Fri Feb  1 20:19:29 2013
> @@ -18,7 +18,9 @@
>  #include "clang/Basic/LLVM.h"
>  #include "llvm/ADT/SmallString.h"
>  #include "llvm/ADT/StringRef.h"
> +#include "llvm/Support/Compiler.h"
>  #include <vector>
> +#include <string>
>
>  namespace clang {
>  namespace cxstring {
> @@ -45,8 +47,23 @@ CXString createRef(const char *String);
>  /// \p String can be changed or freed by the caller.
>  CXString createDup(const char *String);
>
> -/// \brief Create a CXString object from a StringRef.
> -CXString createCXString(StringRef String, bool DupString = true);
> +/// \brief 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);
> +
> +/// \brief Create a CXString object from a StringRef.  New CXString will
> +/// contain a copy of \p String.
> +///
> +/// \p String can be changed or freed by the caller.
> +CXString createDup(StringRef String);
> +
> +// Usually std::string is intended to be used as backing storage for CXString.
> +// In this case, call \c createRef(String.c_str()).
> +//
> +// If you need to make a copy, call \c createDup(StringRef(String)).
> +CXString createRef(std::string String) LLVM_DELETED_FUNCTION;
>
>  /// \brief Create a CXString object that is backed by a string buffer.
>  CXString createCXString(CXStringBuf *buf);
>
> Modified: cfe/trunk/tools/libclang/CXType.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXType.cpp?rev=174263&r1=174262&r2=174263&view=diff
> ==============================================================================
> --- cfe/trunk/tools/libclang/CXType.cpp (original)
> +++ cfe/trunk/tools/libclang/CXType.cpp Fri Feb  1 20:19:29 2013
> @@ -660,7 +660,7 @@ CXString clang_getDeclObjCTypeEncoding(C
>      Ctx.getObjCEncodingForType(Ty, encoding);
>    }
>
> -  return cxstring::createCXString(encoding);
> +  return cxstring::createDup(encoding);
>  }
>
>  } // end: extern "C"
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list