[PATCH] Extra Command Line library functionality (option grouping and runtime inspection/modification)

[Andrew Trick]

On May 7, 2013, at 2:51 AM, Daniel Liew <daniel.liew at imperial.ac.uk> wrote:

> Thanks Andrew.
> 
> Looks like the patches will make it into the 3.3 release :)
> 
> If you think the added features are worth noting in the the Release
> notes for 3.3 then please apply the attached patch (provided you're
> happy with the wording).

Looks good. r181331.
-Andy

> 
> Thanks,
> Dan Liew.
> 
> On 06/05/13 23:03, Andrew Trick wrote:
>> 
>> On May 4, 2013, at 4:07 PM, Dan Liew <daniel.liew at imperial.ac.uk
>> <mailto:daniel.liew at imperial.ac.uk>> wrote:
>> 
>>> Sorry Andrew I didn't read your e-mail correctly the first time. I've
>>> added some primitive tests to "unittests/Support/CommandLineTest.cpp" in
>>> each patch.
>>> 
>>> I have also modified the first patch so that empty option categories are
>>> not printed in the output of -help, they are however printed in the
>>> output of -help-hidden.
>>> 
>>> The modified patches are attached.
>>> 
>>> Something to note is I discovered a problem with one of the tests
>>> (CommandLineTest.ParseEnvironmentToLocalVar) in "CommandLineTest.cpp".
>>> This test creates a command line option on the stack. This option is
>>> subsequently destroyed on leaving the test but the command line system
>>> still has a pointer to that option. If an attempt is made to access
>>> registered options (e.g. through ParseCommandLineOptions() or
>>> getRegisteredOptions() ) after this test the program will SEGFAULT.
>>> 
>>> I worked around this by having my tests that need to access registered
>>> options rule run before the test
>>> CommandLineTest.ParseEnvironmentToLocalVar.
>>> 
>>> This could be fixed in general by implementing destructors for the
>>> Option objects that unregisters them from the command line system but
>>> this probably adds unnecessary overhead as the options are intended to
>>> be declared statically anyway.
>> 
>> Awesome. Committed with no modifications as r181253-4.
>> 
>> Thanks
>> -Andy
>> 
>>> On 04/05/13 12:47, Dan Liew wrote:
>>>> See my responses below.
>>>> On 03/05/13 22:39, Andrew Trick wrote:
>>>>> Daniel,
>>>>> 
>>>>> I reviewed your most recent patch (MOREFIXES) and your
>>>>> getRegisteredOptions patch. They both look very nice.
>>>>> 
>>>>> I have one suggestion. For normal (non-hidden) help, we may have empty
>>>>> categories, which looks odd. Can you just suppress printing the category
>>>>> in that case?
>>>> 
>>>> Yes I can do that. Would you like empty category printing to be
>>>> suppressed for -help-hidden as well or should we show it?
>>>> 
>>>>> Also, would you mind adding a couple simple tests to
>>>>> CommandLineTest.cpp?
>>>> 
>>>> I have not used LLVM's testing infrastructure very much so I'm going to
>>>> need some guidance here.
>>>> 
>>>> I looked at the LLVM testing infrastructure documentation and it
>>>> mentioned the regression tests (obviously not what I want) and
>>>> test-suite (sounded promising as documentation said it built whole
>>>> programs). I took a look at the test-suite but it didn't look
>>>> appropriate as I could not see a way an obvious way to link to the
>>>> LLVMSupport library.
>>>> 
>>>> I also took a look at the unittests in the LLVM repository and found
>>>> some existing in "unittests/Support/CommandLineTest.cpp".
>>>> 
>>>> Is it here that you would like me to add tests?
>>>> 
>>>> If so do you have any suggestions on testing the OptionCategory class? I
>>>> can test whether or not an option has a category set but there isn't a
>>>> way (AFAIK) to test the output of --help (which is the whole point of
>>>> the feature) against some known output in the gtest framework.
>>>> 
>>>>> Otherwise, it's ready to commit. Will you need me to check it in?
>>>> Y
>>>> Yes I will need you to check it in as I do not have commit access.
>>>> 
>>>> Thanks,
>>>> Dan.
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> 
>>> <0001-ATFIXES-Support-command-line-option-categories.patch><0002-ATFIXES-Implemented-public-interface-for-modifying-registere.patch>
>> 
> 
> <0001-Add-two-points-to-release-notes-about-recent-command.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130507/5ae03a7a/attachment.html>