[llvm-commits] [llvm] r86914 - in /llvm/trunk: include/llvm/Analysis/DebugInfo.h include/llvm/Metadata.h lib/Analysis/DebugInfo.cpp lib/VMCore/Metadata.cpp
Jeffrey Yasskin
jyasskin at google.com
Fri Nov 20 10:22:51 PST 2009
On Fri, Nov 20, 2009 at 9:20 AM, Devang Patel <dpatel at apple.com> wrote:
> Hi!
>
> On Nov 20, 2009, at 8:32 AM, Jeffrey Yasskin wrote:
>
>> Hi Devang. This is inconvenient when I want to pass a string in from a
>> std::string, PyString, or from Function::getName(). I don't think it's
>> worth it just to let users pass 0 instead of "".
>
> The debugger and other tools consuming DWARF info are not expecting to see
> "".
>
> If StringRef are used then
> 1) The DwarfWriter needs to check for ""
DwarfDebug::CreateGlobalVariableDIE currently does:
const char *LinkageName = GV.getLinkageName();
if (LinkageName) { ...
If getLinkageName returned a StringRef, DwarfDebug could use "if
(!LinkageName.empty())" instead. getLinkageName calls getStringField,
which actually jumps through hoops to _avoid_ returning a StringRef,
and to canonicalize "" to NULL ... which means that even today if the
CreateFoo functions took StringRef, users could pass "", and
DwarfDebug's current code would work.
> 2) The clients needs to check for optional arguments.
I don't understand: callers of CreateGlobalVariable, for example,
already have to pass something for LinkageName. It's not defaulted.
And if it were defaulted, defaulting to "" would be as easy as
defaulting to NULL.
If you're talking about clients like gdb, it's clearly a win to write
less debug information when a field isn't present, but "" should count
as not present.
> If const char * are used then only clients needs to get pointer to the
> string.
>
> My preferred approach was to let StringRef handle null for optional
> arguments, but it has performance implications.
I think my position on StringRef is that StringRef() should equal
either StringRef(NULL) or StringRef(""). If we like people using NULL
for "no data", then StringRef(NULL) should work. If not—if we don't
think there's a use for two "no data" representations, which is my
position—then StringRef() should ==StringRef(""), which allows
DwarfDebug to only handle one case again, even if it wants to pull the
data out of the StringRef.
... but that's not particularly important for debug info.
> I am not tied to one position, but prefer a approach that makes either
> DwarfWriter OR clients responsible to do the right thing.
>
> -
> Devang
>>
>> On Wed, Nov 11, 2009 at 4:50 PM, Devang Patel <dpatel at apple.com> wrote:
>>>
>>> Author: dpatel
>>> Date: Wed Nov 11 18:50:58 2009
>>> New Revision: 86914
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=86914&view=rev
>>> Log:
>>> Do not use StringRef in DebugInfo interface.
>>> This allows StringRef to skip controversial if(str) check in constructor.
>>> Buildbots, wait for corresponding clang and llvm-gcc FE check-ins!
>>>
>>> Modified:
>>> llvm/trunk/include/llvm/Analysis/DebugInfo.h
>>> llvm/trunk/include/llvm/Metadata.h
>>> llvm/trunk/lib/Analysis/DebugInfo.cpp
>>> llvm/trunk/lib/VMCore/Metadata.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/Analysis/DebugInfo.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/DebugInfo.h?rev=86914&r1=86913&r2=86914&view=diff
>>>
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Analysis/DebugInfo.h (original)
>>> +++ llvm/trunk/include/llvm/Analysis/DebugInfo.h Wed Nov 11 18:50:58 2009
>>> @@ -496,26 +496,26 @@
>>> /// CreateCompileUnit - Create a new descriptor for the specified
>>> compile
>>> /// unit.
>>> DICompileUnit CreateCompileUnit(unsigned LangID,
>>> - StringRef Filenae,
>>> - StringRef Directory,
>>> - StringRef Producer,
>>> + const char * Filename,
>>> + const char * Directory,
>>> + const char * Producer,
>>> bool isMain = false,
>>> bool isOptimized = false,
>>> const char *Flags = "",
>>> unsigned RunTimeVer = 0);
>>>
>>> /// CreateEnumerator - Create a single enumerator value.
>>> - DIEnumerator CreateEnumerator(StringRef Name, uint64_t Val);
>>> + DIEnumerator CreateEnumerator(const char * Name, uint64_t Val);
>>>
>>> /// CreateBasicType - Create a basic type like int, float, etc.
>>> - DIBasicType CreateBasicType(DIDescriptor Context, StringRef Name,
>>> + DIBasicType CreateBasicType(DIDescriptor Context, const char * Name,
>>> DICompileUnit CompileUnit, unsigned
>>> LineNumber,
>>> uint64_t SizeInBits, uint64_t AlignInBits,
>>> uint64_t OffsetInBits, unsigned Flags,
>>> unsigned Encoding);
>>>
>>> /// CreateBasicType - Create a basic type like int, float, etc.
>>> - DIBasicType CreateBasicTypeEx(DIDescriptor Context, StringRef Name,
>>> + DIBasicType CreateBasicTypeEx(DIDescriptor Context, const char *
>>> Name,
>>> DICompileUnit CompileUnit, unsigned
>>> LineNumber,
>>> Constant *SizeInBits, Constant
>>> *AlignInBits,
>>> Constant *OffsetInBits, unsigned Flags,
>>> @@ -524,7 +524,7 @@
>>> /// CreateDerivedType - Create a derived type like const qualified
>>> type,
>>> /// pointer, typedef, etc.
>>> DIDerivedType CreateDerivedType(unsigned Tag, DIDescriptor Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNumber,
>>> uint64_t SizeInBits, uint64_t
>>> AlignInBits,
>>> @@ -534,7 +534,7 @@
>>> /// CreateDerivedType - Create a derived type like const qualified
>>> type,
>>> /// pointer, typedef, etc.
>>> DIDerivedType CreateDerivedTypeEx(unsigned Tag, DIDescriptor Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNumber,
>>> Constant *SizeInBits, Constant
>>> *AlignInBits,
>>> @@ -543,7 +543,7 @@
>>>
>>> /// CreateCompositeType - Create a composite type like array, struct,
>>> etc.
>>> DICompositeType CreateCompositeType(unsigned Tag, DIDescriptor
>>> Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNumber,
>>> uint64_t SizeInBits,
>>> @@ -555,7 +555,7 @@
>>>
>>> /// CreateCompositeType - Create a composite type like array, struct,
>>> etc.
>>> DICompositeType CreateCompositeTypeEx(unsigned Tag, DIDescriptor
>>> Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNumber,
>>> Constant *SizeInBits,
>>> @@ -567,25 +567,25 @@
>>>
>>> /// CreateSubprogram - Create a new descriptor for the specified
>>> subprogram.
>>> /// See comments in DISubprogram for descriptions of these fields.
>>> - DISubprogram CreateSubprogram(DIDescriptor Context, StringRef Name,
>>> - StringRef DisplayName,
>>> - StringRef LinkageName,
>>> + DISubprogram CreateSubprogram(DIDescriptor Context, const char *
>>> Name,
>>> + const char * DisplayName,
>>> + const char * LinkageName,
>>> DICompileUnit CompileUnit, unsigned
>>> LineNo,
>>> DIType Type, bool isLocalToUnit,
>>> bool isDefinition);
>>>
>>> /// CreateGlobalVariable - Create a new descriptor for the specified
>>> global.
>>> DIGlobalVariable
>>> - CreateGlobalVariable(DIDescriptor Context, StringRef Name,
>>> - StringRef DisplayName,
>>> - StringRef LinkageName,
>>> + CreateGlobalVariable(DIDescriptor Context, const char * Name,
>>> + const char * DisplayName,
>>> + const char * LinkageName,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNo, DIType Type, bool isLocalToUnit,
>>> bool isDefinition, llvm::GlobalVariable *GV);
>>>
>>> /// CreateVariable - Create a new descriptor for the specified
>>> variable.
>>> DIVariable CreateVariable(unsigned Tag, DIDescriptor Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit, unsigned LineNo,
>>> DIType Type);
>>>
>>>
>>> Modified: llvm/trunk/include/llvm/Metadata.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Metadata.h?rev=86914&r1=86913&r2=86914&view=diff
>>>
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Metadata.h (original)
>>> +++ llvm/trunk/include/llvm/Metadata.h Wed Nov 11 18:50:58 2009
>>> @@ -60,6 +60,7 @@
>>>
>>> public:
>>> static MDString *get(LLVMContext &Context, StringRef Str);
>>> + static MDString *get(LLVMContext &Context, const char *Str);
>>>
>>> StringRef getString() const { return Str; }
>>>
>>>
>>> Modified: llvm/trunk/lib/Analysis/DebugInfo.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/DebugInfo.cpp?rev=86914&r1=86913&r2=86914&view=diff
>>>
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Analysis/DebugInfo.cpp (original)
>>> +++ llvm/trunk/lib/Analysis/DebugInfo.cpp Wed Nov 11 18:50:58 2009
>>> @@ -653,9 +653,9 @@
>>> /// CreateCompileUnit - Create a new descriptor for the specified
>>> compile
>>> /// unit. Note that this does not unique compile units within the
>>> module.
>>> DICompileUnit DIFactory::CreateCompileUnit(unsigned LangID,
>>> - StringRef Filename,
>>> - StringRef Directory,
>>> - StringRef Producer,
>>> + const char * Filename,
>>> + const char * Directory,
>>> + const char * Producer,
>>> bool isMain,
>>> bool isOptimized,
>>> const char *Flags,
>>> @@ -677,7 +677,7 @@
>>> }
>>>
>>> /// CreateEnumerator - Create a single enumerator value.
>>> -DIEnumerator DIFactory::CreateEnumerator(StringRef Name, uint64_t Val){
>>> +DIEnumerator DIFactory::CreateEnumerator(const char * Name, uint64_t
>>> Val){
>>> Value *Elts[] = {
>>> GetTagConstant(dwarf::DW_TAG_enumerator),
>>> MDString::get(VMContext, Name),
>>> @@ -689,7 +689,7 @@
>>>
>>> /// CreateBasicType - Create a basic type like int, float, etc.
>>> DIBasicType DIFactory::CreateBasicType(DIDescriptor Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNumber,
>>> uint64_t SizeInBits,
>>> @@ -714,7 +714,7 @@
>>>
>>> /// CreateBasicType - Create a basic type like int, float, etc.
>>> DIBasicType DIFactory::CreateBasicTypeEx(DIDescriptor Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNumber,
>>> Constant *SizeInBits,
>>> @@ -741,7 +741,7 @@
>>> /// pointer, typedef, etc.
>>> DIDerivedType DIFactory::CreateDerivedType(unsigned Tag,
>>> DIDescriptor Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNumber,
>>> uint64_t SizeInBits,
>>> @@ -769,7 +769,7 @@
>>> /// pointer, typedef, etc.
>>> DIDerivedType DIFactory::CreateDerivedTypeEx(unsigned Tag,
>>> DIDescriptor Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNumber,
>>> Constant *SizeInBits,
>>> @@ -796,7 +796,7 @@
>>> /// CreateCompositeType - Create a composite type like array, struct,
>>> etc.
>>> DICompositeType DIFactory::CreateCompositeType(unsigned Tag,
>>> DIDescriptor Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNumber,
>>> uint64_t SizeInBits,
>>> @@ -828,7 +828,7 @@
>>> /// CreateCompositeType - Create a composite type like array, struct,
>>> etc.
>>> DICompositeType DIFactory::CreateCompositeTypeEx(unsigned Tag,
>>> DIDescriptor Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit
>>> CompileUnit,
>>> unsigned LineNumber,
>>> Constant *SizeInBits,
>>> @@ -861,9 +861,9 @@
>>> /// See comments in DISubprogram for descriptions of these fields. This
>>> /// method does not unique the generated descriptors.
>>> DISubprogram DIFactory::CreateSubprogram(DIDescriptor Context,
>>> - StringRef Name,
>>> - StringRef DisplayName,
>>> - StringRef LinkageName,
>>> + const char * Name,
>>> + const char * DisplayName,
>>> + const char * LinkageName,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNo, DIType Type,
>>> bool isLocalToUnit,
>>> @@ -887,9 +887,9 @@
>>>
>>> /// CreateGlobalVariable - Create a new descriptor for the specified
>>> global.
>>> DIGlobalVariable
>>> -DIFactory::CreateGlobalVariable(DIDescriptor Context, StringRef Name,
>>> - StringRef DisplayName,
>>> - StringRef LinkageName,
>>> +DIFactory::CreateGlobalVariable(DIDescriptor Context, const char * Name,
>>> + const char * DisplayName,
>>> + const char * LinkageName,
>>> DICompileUnit CompileUnit,
>>> unsigned LineNo, DIType Type,bool
>>> isLocalToUnit,
>>> bool isDefinition, llvm::GlobalVariable
>>> *Val) {
>>> @@ -921,7 +921,7 @@
>>>
>>> /// CreateVariable - Create a new descriptor for the specified variable.
>>> DIVariable DIFactory::CreateVariable(unsigned Tag, DIDescriptor Context,
>>> - StringRef Name,
>>> + const char * Name,
>>> DICompileUnit CompileUnit, unsigned
>>> LineNo,
>>> DIType Type) {
>>> Value *Elts[] = {
>>>
>>> Modified: llvm/trunk/lib/VMCore/Metadata.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Metadata.cpp?rev=86914&r1=86913&r2=86914&view=diff
>>>
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/VMCore/Metadata.cpp (original)
>>> +++ llvm/trunk/lib/VMCore/Metadata.cpp Wed Nov 11 18:50:58 2009
>>> @@ -39,6 +39,17 @@
>>> new MDString(Context, Entry.getKey());
>>> }
>>>
>>> +MDString *MDString::get(LLVMContext &Context, const char *Str) {
>>> + LLVMContextImpl *pImpl = Context.pImpl;
>>> + StringMapEntry<MDString *> &Entry =
>>> + pImpl->MDStringCache.GetOrCreateValue(Str ? StringRef(Str) :
>>> StringRef());
>>> + MDString *&S = Entry.getValue();
>>> + if (S) return S;
>>> +
>>> + return S =
>>> + new MDString(Context, Entry.getKey());
>>> +}
>>> +
>>>
>>> //===----------------------------------------------------------------------===//
>>> // MDNode implementation.
>>> //
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>
>
More information about the llvm-commits
mailing list