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.
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... On Fri, Aug 17, 2012 at 2:26 AM, Manuel Klimek <[email protected]> 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 <[email protected]> > wrote: > > On Thu, Aug 16, 2012 at 7:04 PM, Manuel Klimek <[email protected]> > wrote: > >> > >> On Thu, Aug 16, 2012 at 5:45 PM, Alexander Kornienko <[email protected] > > > >> 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 > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
