[PATCH] Support: Add dwarf::getVirtuality()
Duncan P. N. Exon Smith
dexonsmith at apple.com
Fri Feb 6 16:47:41 PST 2015
> On 2015-Feb-06, at 16:28, Frédéric Riss <friss at apple.com> wrote:
>
>>
>> On Feb 6, 2015, at 4:00 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>>
>> DW_VIRTUALITY_* (the last DW_* thing that my upcoming assembly patches
>> use) is a bit different from DW_TAG, DW_LANG and DW_ATE.
>>
>> There are only three valid values:
>>
>> - 0x0 (none),
>> - 0x1 (virtual), and
>> - 0x2 (pure_virtual),
>>
>> and there isn't a `DW_VIRTUALITY_lo_user` or `DW_VIRTUALITY_hi_user`.
>>
>> The assembly code needs to know the valid numeric range, so I've added
>> a `DW_VIRTUALITY_max` that also points at `0x2`. I've also added a
>> `DW_VIRTUALITY_invalid` to `LLVMConstants` to return as an error code
>> from `getVirtuality()`.
>>
>> I was about to commit without asking, but since it's a little different
>> I thought I'd check. Does this make sense to others?
>
> This makes sense, it LGTM.
r228473 and r228474.
> This change made me wonder if you shouldn’t define _invalid constants for the the ones where you used a simple 0 as an error return. It would make the parser code more homogenous, wouldn’t it?
I prefer `0`s because of this pattern:
if (unsigned Lang = dwarf::getLanguage(LangString)) {
// ...
}
I was even thinking of proposing that `dwarf::getTag()` return 0.
("Deep inspection" via `git grep` tells me that my introduced uses
are the *only* ones; previously this was dead code! I should have
noticed that at the time.) In that case `DW_VIRTUALITY` would be
the only one with a special "invalid" tag.
However, I suppose consistency is important too. I'm happy to
change the others to `DW_*_invalid` if that's better. I'll let the
rest of you folks decide.
More information about the llvm-commits
mailing list