[cfe-commits] r86915 - /cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

[Daniel Dunbar]

On Thu, Nov 12, 2009 at 9:14 AM, Devang Patel <dpatel at apple.com> wrote:
>
> On Nov 12, 2009, at 9:04 AM, Daniel Dunbar wrote:
>
> First, what's the motivation?
>
> To avoid if(Str) check in StringRef constructor, I switched DebugInfo APIs
> to not use StringRef.

That part I guessed, however, I'm not sure why this is the right
approach. Why does the DebugInfo interface need to deal with null
strings so much? It seems like that responsibility could be pushed to
clients, does clang even ever do this? Can you give me a concrete
example of a specific problem?

 - Daniel

>
> Second, this is not a safe way (from an API point of view) to pass a
> 'C string': Decl->getName().data(). StringRef never guarantees null
> termination.
>
>
>
> Third, it looks like this patch is introducing a number of unnecessary
> std::string constructions (although it also looks like its removing a
> some).
>
> It is  a win overall. And we can reduce unnecessary std::string
> constructions.
>
> Finally, note that getName() *IS NOT* an alternate form of
> getNameAsString -- getNameAsString handles constructing a name in the
> cases when the internal representation is more complicated than a
> single string. I'm not sure whether the cases that changed are all
> safe (i.e., they cannot have specially named decls), but I suspect
> they might not be.
>
> After re-reading the comments, I found that there is getNameAsCString()
> which is what we need here. I'll update.
> -
> Devang
>
>