[cfe-commits] [PATCH] Add multiple prefixes to Option.

[Michael Spencer]

On Fri, Oct 19, 2012 at 2:40 PM, Daniel Dunbar <daniel at zuster.org> wrote:
> On Fri, Oct 19, 2012 at 2:23 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
>> On Fri, Oct 19, 2012 at 2:01 PM, Daniel Dunbar <daniel at zuster.org> wrote:
>>> On Fri, Oct 19, 2012 at 1:08 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
>>>> On Fri, Oct 19, 2012 at 12:48 PM, Daniel Dunbar <daniel at zuster.org> wrote:
>>>>> On Fri, Oct 19, 2012 at 12:39 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
>>>>>> On Fri, Oct 19, 2012 at 10:38 AM, Daniel Dunbar <daniel at zuster.org> wrote:
>>>>>>> Hi Michael,
>>>>>>>
>>>>>>> I understand what this patch does, but I don't really understand the
>>>>>>> motivation for it, yet.
>>>>>>>
>>>>>>> Why is this the "main feature" needed for sane gnu-ld and link command
>>>>>>> line support?
>>>>>>>
>>>>>>> It seems to me, that what you really need for those is a completely
>>>>>>> separate option parsing table. I can see how this patch could be
>>>>>>> useful if you wanted to accept the same options for the link.exe
>>>>>>> parsing just with a '/' at the front, but that doesn't seem right to
>>>>>>> me. It seems to me that the link.exe parsing or cl.exe parsing would
>>>>>>> end up using a completely separate parsing table.
>>>>>>>
>>>>>>> Can you explain?
>>>>>>>
>>>>>>>  - Daniel
>>>>>>
>>>>>> The option tables would be completely separate. This is needed because
>>>>>> link supports both / and - prefixes for all options. gnu-ld supports -
>>>>>> and -- for almost all options.
>>>>>
>>>>> If the option tables are separate, can't we dramatically simplify this
>>>>> patch by making this a feature of the option table then? So that an
>>>>> option table can opt in to explicitly specifying all prefixes, and
>>>>> then we don't need to clutter up the Clang/GCC option table. Option
>>>>> tables which didn't opt-in to that would be required to supply a list
>>>>> of possible prefixes.
>>>>>
>>>>>  - Daniel
>>>>
>>>> This would work if clang and gcc used only one prefix. There are 36
>>>> options that use both - and --, and 103 that use --.
>>>
>>> I think you misunderstood me, I was proposing initializing the option
>>> table with a special mode that says "all prefixes are defined as part
>>> of the option". Only the Clang/GCC table would use that.
>>>
>>>  - Daniel
>>
>> Ah, I see. I feel this would greatly complicate the code as it would
>> require either two different parsing paths in OptParser, or
>> normalizing in the tablegen emitter.
>
> Wouldn't either of those be more localized than adding more code to
> all of the option specifications?
>
>  - Daniel

Changing all the option specifications is a very mechanical change. I
just used a regex and manually did the 36 merges. It actually made the
files smaller. And most of this can be hidden simply with tablegen.

- Michael Spencer

>>
>> - Michael Spencer
>>
>>>> I can simplify this patch by defaulting to - via tblgen and only
>>>> changing those 139 options that are special.
>>>>
>>>> - Michael Spencer
>>>>
>>>>>> From the ld man page:
>>>>>>
>>>>>> "For options whose names are multiple letters, either one dash or two
>>>>>> can precede the option name; for example, -trace-symbol and
>>>>>> --trace-symbol are equivalent. Note---there is one exception to this
>>>>>> rule. Multiple letter options that start with a lower case 'o' can
>>>>>> only be preceded by two dashes. This is to reduce confusion with the
>>>>>> -o option. So for example -omagic sets the output file name to magic
>>>>>> whereas --omagic sets the NMAGIC flag on the output."
>>>>>>
>>>>>> - Michael Spencer
>>>>>>
>>>>>>>
>>>>>>> On Thu, Oct 18, 2012 at 3:53 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
>>>>>>>> Hi ddunbar,
>>>>>>>>
>>>>>>>> This patch adds support for multiple prefixes per option. This is the main feature needed for sane gnu-ld and link command line support.
>>>>>>>>
>>>>>>>> Most of the ["-"]'s can be refactored to a common class in tablegen, but I have a few more widespread tablegen changes I want to make before doing all the refactoring needed.
>>>>>>>>
>>>>>>>> http://llvm-reviews.chandlerc.com/D69
>>>>>>>>
>>>>>>>> Files:
>>>>>>>>   include/clang/Driver/Arg.h
>>>>>>>>   include/clang/Driver/CC1AsOptions.h
>>>>>>>>   include/clang/Driver/CC1AsOptions.td
>>>>>>>>   include/clang/Driver/CC1Options.td
>>>>>>>>   include/clang/Driver/OptParser.td
>>>>>>>>   include/clang/Driver/OptTable.h
>>>>>>>>   include/clang/Driver/Option.h
>>>>>>>>   include/clang/Driver/Options.h
>>>>>>>>   include/clang/Driver/Options.td
>>>>>>>>   lib/Driver/Arg.cpp
>>>>>>>>   lib/Driver/ArgList.cpp
>>>>>>>>   lib/Driver/CC1AsOptions.cpp
>>>>>>>>   lib/Driver/Driver.cpp
>>>>>>>>   lib/Driver/DriverOptions.cpp
>>>>>>>>   lib/Driver/OptTable.cpp
>>>>>>>>   lib/Driver/Option.cpp
>>>>>>>>   lib/Driver/Tools.cpp
>>>>>>>>   lib/Frontend/CompilerInvocation.cpp
>>>>>>>>   utils/TableGen/OptParserEmitter.cpp