[cfe-commits] [PATCH] Reverted clang-check to fully supported CommandLine Library use-case: global static variables.

[Alexander Kornienko]

On Fri, Aug 17, 2012 at 11:41 AM, Chandler Carruth <chandlerc at google.com>wrote:

> Er, my suggestion was to make these *static* variables, yes, even though
> they are in a header file, and then to document thoroughly that this header
> file is *not* for use within a library, but for use directly within the
> file defining main.

As Manuel already noted, there was an idea to make definition of these
variables explicit via a macro. And I didn't find a better way to implement
that. If you think that a special "include-once-per-binary" header is still
better, I can switch to this approach.

The current patch is different from that, can you describe how? In general
> the comments weren't clear enough for me to be confident in how you expect
> this file to be used...
>
Ok, I moved the usage example to the top and added some comments. Is it
better?


>
> On Fri, Aug 17, 2012 at 2:26 AM, Manuel Klimek <klimek at google.com> wrote:
>
>> +using namespace llvm;
>>
>> Remove from header.
>>
>> Appart from that, lgtm, minus Chandler's approval of the general idea.
>>
>> On Thu, Aug 16, 2012 at 7:17 PM, Alexander Kornienko <alexfh at google.com>
>> wrote:
>> > On Thu, Aug 16, 2012 at 7:04 PM, Manuel Klimek <klimek at google.com>
>> wrote:
>> >>
>> >> On Thu, Aug 16, 2012 at 5:45 PM, Alexander Kornienko <
>> alexfh at google.com>
>> >> wrote:
>> >> > Now it's a bit uglier, but doesn't use CommandLine Library in an
>> >> > unsupported
>> >> > way.
>> >> > Chandler, please take a look if it seems better to you.
>> >>
>> >> After a short discussion off-list we came to the conclusion that
>> >> tooling::CommonOptionsParser is a better name for the class, and
>> >> especially makes the responsibilities clearer...
>> >
>> > A new patch is attached.
>> >
>> >>
>> >>
>> >> > BTW, we still have a number of alien options defined in some llvm
>> >> > libraries
>> >> > we link with. Should we try to avoid that by de-globalizing cl
>> library
>> >> > or is
>> >> > someone working on its replacement/refactoring now?
>> >
>> >
>> > --
>> > Best regards,
>> > Alexander Kornienko
>> >
>>
>
>


-- 
Best regards,
Alexander Kornienko
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120817/886b0dff/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tooling-opts.diff
Type: application/octet-stream
Size: 17953 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120817/886b0dff/attachment.obj>