On Fri, Oct 19, 2012 at 2:40 PM, Daniel Dunbar <[email protected]> wrote: > On Fri, Oct 19, 2012 at 2:23 PM, Michael Spencer <[email protected]> > wrote: >> On Fri, Oct 19, 2012 at 2:01 PM, Daniel Dunbar <[email protected]> wrote: >>> On Fri, Oct 19, 2012 at 1:08 PM, Michael Spencer <[email protected]> >>> wrote: >>>> On Fri, Oct 19, 2012 at 12:48 PM, Daniel Dunbar <[email protected]> wrote: >>>>> On Fri, Oct 19, 2012 at 12:39 PM, Michael Spencer <[email protected]> >>>>> wrote: >>>>>> On Fri, Oct 19, 2012 at 10:38 AM, Daniel Dunbar <[email protected]> >>>>>> 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 >>>>>>> <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
