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

Tobias Grosser tobias at grosser.es
Mon May 6 00:39:36 PDT 2013


On 05/05/2013 01:07 AM, Dan Liew 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 found only a single primitive test. Was that your definition of 'some' 
or did I miss some tests?

> 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.

I looked again through the patch. It looks good and works as expected. I 
also tested the empty option category handling and its behavior looks good.

> 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.

This seems reasonable, but I defer to Andrew's expertise regarding this 
point as well as what can be tested reasonably.

Cheers,
Tobias




More information about the llvm-commits mailing list