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

[NAKAMURA Takumi]

/**
 * Obtain a Type from a module by its registered name.
 */
LLVMTypeRef LLVMGetTypeByName(LLVMModuleRef M, const char *Name);

Would we leave C API untouched? IMO, I'd like Module::getTypeByName() obsoleted.

...Takumi

2013/1/11 Chris Lattner <clattner at apple.com>:
>
> 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
>