[llvm-commits] [llvm] r165923 - in /llvm/trunk: include/llvm/Attributes.h lib/VMCore/Attributes.cpp

[David Blaikie]

On Mon, Oct 15, 2012 at 9:24 AM, Bill Wendling <isanbard at gmail.com> wrote:
> On Oct 15, 2012, at 8:13 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Sun, Oct 14, 2012 at 11:53 PM, Bill Wendling <isanbard at gmail.com> wrote:
>>> Author: void
>>> Date: Mon Oct 15 01:53:28 2012
>>> New Revision: 165923
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=165923&view=rev
>>> Log:
>>> Use a ::get method to create the attribute from Attributes::AttrVals instead of a constructor.
>>>
>>> Modified:
>>>    llvm/trunk/include/llvm/Attributes.h
>>>    llvm/trunk/lib/VMCore/Attributes.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/Attributes.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Attributes.h?rev=165923&r1=165922&r2=165923&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Attributes.h (original)
>>> +++ llvm/trunk/include/llvm/Attributes.h Mon Oct 15 01:53:28 2012
>>> @@ -89,17 +89,26 @@
>>>   };
>>> private:
>>>   AttributesImpl *Attrs;
>>> -
>>> -  explicit Attributes(AttributesImpl *A);
>>> +  Attributes(AttributesImpl *A);
>>
>> Any particular reason this became non-explicit along the way? (I
>> assume it just got dropped by accident)
>>
> There's no reason for it to be explicit, since that's a real datatype instead of something convertible like an enum to integral value. :)

I'm not entirely sure what you mean by that, sorry.

This could still occur implicitly in forms like:

void func(const Attribute&);
AttributesImpl *X = ...;
func(X);

Which might not matter, but I'm not sure you actually want/need that
conversion for this type. Granted it's hardly the most confusing
conversion as the types have a clear relationship to one another.

>> I don't think we have a convention that says "explicit by default
>> unless it needs to be otherwise" but it's probably generally a good
>> idea.
>>
>
> -bw
>