[patch] Make it explicit that nulls are not allowed in names

[Chris Lattner]

On Nov 14, 2013, at 5:33 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:

>>> The object files we support use null terminated strings, so there is
>>> no way to support these.
>>> 
>>> This patch adds an assert to catch bad API use and an error check in
>>> the .ll parser.
>> 
>> I think this would be better as part of the IR Verifier than in setName(), but the idea is sensible.
> 
> I can move it there, but why? My rule of the thumb has been that
> invariants that are only true at the end of pass are checked in the
> Verifier. Invariants that are always true are checked on
> constructors/setters.

I’m only worried about the runtime cost of this.  I agree that it’s a silly thing to worry about if it is wrapped in an assert/NDEBUG though, so go ahead with your patch.  Thanks Rafael!

-Chris

> 
> For example, the GlobalVariable constructor has
> 
> if (InitVal) {
>    assert(InitVal->getType() == Ty &&
>           "Initializer should be the same type as the GlobalVariable!");
> 
> If someone is passing an initializer, they should be able to get the
> type right at construction time. On the other hand, the verifier is
> the one that checks that if there is no initializer
> 
>    Assert1(GV.hasExternalLinkage() || GV.hasDLLImportLinkage() ||
>            GV.hasExternalWeakLinkage(),
>            "invalid linkage type for global .declaration", &GV);
> 
> The reason being that it is convenient in some cases to create a
> variable without an initializer and then set it, so this invariant is
> invalid at construction time.
> 
> Cheers,
> Rafael