[PATCH] Update dwarf::ApplePropertyAttributes enum to meaningful values.

[Frédéric Riss]

> On Oct 7, 2014, at 2:30 PM, Eric Christopher <echristo at gmail.com> wrote:
> 
> On Tue, Oct 7, 2014 at 2:00 PM, Frédéric Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
>> 
>> On Oct 7, 2014, at 1:26 PM, Eric Christopher <echristo at gmail.com> wrote:
>> 
>> On Tue, Oct 7, 2014 at 12:57 PM, Frederic Riss <friss at apple.com> wrote:
>> 
>> ================
>> Comment at: include/llvm/Support/Dwarf.h:784
>> @@ -783,1 +783,3 @@
>> +/// Contstants for the DW_APPLE_PROPERTY_attributes attribute.
>> +/// Keep this list in sync with clang's DeclSpec.h
>> ObjCPropertyAttributeKind.
>> enum ApplePropertyAttributes {
>> ----------------
>> dblaikie wrote:
>> 
>> Crazy idea: Could we do this the other way around, and remove
>> ObjCPropertyAttributeKind in favor of using this enum directly?
>> 
>> Probably not, but figured I'd mention it.
>> 
>> I thought of that but I think there would be (rightful) resistance to having
>> Sema code depend on the Dwarf header. In a followup patch for the dwarfdump
>> functionality, I'll try to add coverage for most if not all of these, that
>> will at least prevent us from breaking the current values unknowingly.
>> 
>> 
>> So it's not so much resistance to having Sema code depend on the dwarf
>> header... if we'd write the same code both ways. This code looks a bit
>> weird that way in that having the header wouldn't necessarily change
>> things.
>> 
>> 
>> Not sure I’m getting what you’re implying. I could have included Dwarf.h in
>> DeclSpec.h and define the enum values of the ObjCPropertyAttributeKind in
>> terms of the Dwarf.h values, or even more radically remove
>> ObjCPropertyAttributeKind and rewrite all the code using these enums to use
>> DW_APPLE_PROPERTY_* values instead.
>> 
> 
> Right, the problem is that it is a pretty bad hack either way. Or at
> least seems so to me. Can run the code by Richard though if you'd like
> to try.

No, I think the current patch with the upcoming test coverage is better. I was just mentioning to Dave that I had the same thoughts while writing the patch, but it would look too weird IMHO. 

Fred

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141007/b84c5a8c/attachment.html>