[cfe-dev] Allowing '_' as version tuple separator?

[Aaron Ballman]

On Thu, Oct 2, 2014 at 1:09 PM, jahanian <fjahanian at apple.com> wrote:
>
> On Oct 2, 2014, at 10:07 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>
>> On Thu, Oct 2, 2014 at 1:01 PM, jahanian <fjahanian at apple.com> wrote:
>>>
>>> On Oct 1, 2014, at 9:57 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>>
>>>>>
>>>>>
>>>>> Parser::ParseVersionTuple(…) is where 'major[.minor[.subminor]]’ is parsed.
>>>>> I am suggesting to modify it to recognize ‘_’ as separator. So, I propose to
>>>>> change
>>>>> Parser::ParseVersionTuple(…) to also accept ‘major[_minor[_subminor]]’
>>>>> syntax as well.
>>>>> As an example, I brought up an example of versioning in availability
>>>>> attribute and showed
>>>>> how this change will allow 10_1_0 as alternative version. Hope it is clear.
>>>>
>>>> Thank you for the clarification! So long as VersionTuple remembers
>>>> which form is used (period or underscore) so that it can be printed in
>>>> diagnostics and pretty printed by attributes properly (with
>>>> appropriate test coverage, of course), I am okay with this.
>>>
>>> ObjC pretty printing has not been supported in a while and does not print “availability” attribute
>>> at all.
>>
>> Except this affects more than just ObjC, and the availability
>> attributes are printed out in my trivial tests.
>>
>>> Diagnostic still uses ‘.’. Use of ‘_’ in availability attribute declaration is a notational convenience and
>>> remembering it in AST imo does not justify cost vs. benefit.
>>
>> I disagree, which is why I specifically brought it up when you were
>> asking about introducing this change.
>
> Please file a bugzilla for these you care about. I, or some one else, will implement them.

The comments are in this thread already, when you were asking for
pre-commit feedback on this change. To recap: VersionTuple needs to be
modified to remember which syntax was used, and provide tests (like
-ast-print in Misc) for ensuring the diagnostics and the pretty
printing are not broken (which they currently are). I don't believe
this requires a Bugzilla ticket.

~Aaron