[llvm-commits] Move Module::getTypeByName to LLVMContext::getTypeByName

[Chris Lattner]

On Jan 10, 2013, at 4:03 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:

> LGTM. Chris, I don't understand why getTypeByName() belongs to Module?

It was put there as a convenience.  Moving it to LLVMContext makes perfect sense.

-Chris

> 
> Mael, please update and resend your patch along trunk, thank you.
> LLVMContext.h has been moved to include/llvm/IR.
> 
> 
>> --- a/include/llvm/LLVMContext.h
>> +++ b/include/llvm/LLVMContext.h
>> @@ -86,6 +87,10 @@ public:
>>   void emitError(const Instruction *I, const Twine &ErrorStr);
>>   void emitError(const Twine &ErrorStr);
>> 
>> +  /// getTypeByName - Return the type with the specified name, or null if there
>> +  /// is none by that name.
>> +  StructType * getTypeByName(StringRef Name) const;
> 
> We prefer;
>> +  StructType *getTypeByName(StringRef Name) const;
> 
>> +
>> private:
>>   // DO NOT IMPLEMENT
>>   LLVMContext(LLVMContext&);
> 
>> --- a/lib/VMCore/Type.cpp
>> +++ b/lib/VMCore/Type.cpp
>> @@ -631,11 +631,7 @@ bool StructType::isLayoutIdentical(StructType *Other) const {
>> /// getTypeByName - Return the type with the specified name, or null if there
>> /// is none by that name.
>> StructType *Module::getTypeByName(StringRef Name) const {
>> -  StringMap<StructType*>::iterator I =
>> -    getContext().pImpl->NamedStructTypes.find(Name);
>> -  if (I != getContext().pImpl->NamedStructTypes.end())
>> -    return I->second;
>> -  return 0;
>> +    return getContext().getTypeByName(Name);
> 
> We prefer here;
>> +  return getContext().getTypeByName(Name);
> 
>> }
> 
> 2013/1/11 Maël Nison <nison.mael at gmail.com>:
>> Could it be possible to have a feedback, even negative, please ?
>> 
>> 
>> On 5 August 2012 14:44, Maël Nison <nison.mael at gmail.com> wrote:
>>> 
>>> As a workaround, creating a temporary stack-alloced Module to use its
>>> getTypeByName() method works.
>>> 
>>> But that's a workaround, it requires useless additional instructions. : /
>>> 
>>> 
>>> On 2 August 2012 18:40, Maël Nison <nison.mael at gmail.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> This very short patch moves Module's getTypeByName into LLVMContext,
>>>> because :
>>>> 
>>>> - Every types are linked to a Context, so it makes sense to get types
>>>> from the context and not from a module
>>>> - The Module::getTypeByName function doesn't actually use anything owned
>>>> by the Module class
>>>> - It is not possible to specialize the TypeBuilder to return a named
>>>> structure (we can only returns literal structures). By moving getTypeByName
>>>> into the LLVMContext class, we are able to do it (TypeBuilder's
>>>> specializations are called with a LLVMContext reference as parameter).
>>>> 
>>>> In order to keep backward compatibility, Module::getTypeByName now
>>>> forwards to the new method.
>>>> 
>>>> It's my first patch so .. am I doing it wrong ?
>>>> 
>>>> --
>>>> Maël Nison
>>>> Epitech 2014, Paris - Astek
>>>> 
>>> 
>>> 
>>> 
>>> --
>>> Maël Nison
>>> Epitech 2014, Paris - Astek
>>> 
>> 
>> 
>> 
>> --
>> Maël Nison
>> JS Github hipster, Assistant C++ chez Epitech
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits