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

[Chris Lattner]

On Jan 11, 2013, at 5:53 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:

> /**
> * 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.

Yep, the C API should stay the way it is.  It should just get the context from the module, then call the method on the context.

-Chris

> 
> ...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
>>