dblaikie added inline comments.
================
Comment at: llvm/lib/AsmParser/LLParser.cpp:4858
/// ::= !DITemplateValueParameter(tag: DW_TAG_template_value_parameter,
-/// name: "V", type: !1, value: i32 7)
+/// name: "V", type: !1, defaultValue: "false",
+/// value: i32 7)
----------------
dblaikie wrote:
> defaultValue should probably be rendered/encoded as a boolean, rather than a
> string?
>
> (& "defaultValue" might be a misleading name (if the DITemplateValueParameter
> was a boolean non type template parameter, then "defaultValue: true" could
> look like the default value is true, not that the "value: "parameter is the
> default value... if that makes sense) - perhaps "defaulted" would read more
> easily ("defaulted: true" or "defaulted: false"))
>
> Perhaps just "default" though that still feels a bit ambiguous between "is
> this the default value itself, or is it specifying that the "value:" field is
> the default value?".
I see that the actual DWARFv5 feature is "DW_AT_default_value" - so I guess
sticking with that is good, even though it's not a great name (& indeed
elsewhere in DWARF this same attribute name is used with a value that
represents the default value, not a boolean about whether the default value is
elsewhere... :/)
Perhaps we should push the name further up then - into the API, (IsDefaultValue
member, maybe make the accessor "isDefaultValue" rather than "getDefaultValue"
to avoid some of the ambiguity/confusion, etc).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73462/new/
https://reviews.llvm.org/D73462
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
