llvm dev - Apr 2019 - [CommandLine] Unable to implement a custom parser -- all marked final

Hi David:

I'd actually like to use a custom parser in a tool I'm developing,
parsing
YAML files.  The partial fix for my purposes was:

--- a/llvm/include/llvm/Support/CommandLine.h
+++ b/llvm/include/llvm/Support/CommandLine.h
@@ -1044,7 +1044,7 @@ extern template class basic_parser<float>;
 //--------------------------------------------------
 // parser<std::string>
 //
-template <> class parser<std::string> final : public
basic_parser<std::string> {
+template <> class parser<std::string> : public
basic_parser<std::string> {
 public:
   parser(Option &O) : basic_parser(O) {}

@@ -1062,6 +1062,8 @@ public:

   // An out-of-line virtual method to provide a 'home' for this class.
   void anchor() override;
+
+  virtual ~parser() = default;
 };

If that's acceptable, I submit a patch.

thanks...
don

On Thu, Apr 18, 2019 at 1:56 PM David Blaikie <dblaikie at gmail.com>
wrote:
> Yeah, I think I did this a while back (& you're possibly not the
only
> one who's hit this - might be worth a search on llvm-dev for previous
> threads).
>
> If you check the commit history, I think my original motivation was
> cleaning up the virtual dtor that wasn't used in LLVM by making the
> base class's dtor protected, and the derived classes final.
>
> It could be made virtual instead of protected/final - but without any
> use in LLVM, I'm not sure it's worth exposing this extension point
&
> perhaps just updating the documentation is the best thing?
>
> On Thu, Apr 18, 2019 at 1:45 PM Don Hinton via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> >
> > https://llvm.org/docs/CommandLine.html#writing-a-custom-parser
> describes how to implement a custom parser, but they're all marked
`final`,
> so I can't inherit.
> >
> > Is there a reason for this?  If not, I'll submit patch with tests.
> >
> > thanks...
> > don
> >
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190418/c93ea4a5/attachment.html>

I don't feel super strongly either way - though if you're going to
make parser's dtor virtual, could you remove 'final' from the other
parsers too?

Any idea what's different about your situation that makes this feature
useful but it hasn't been otherwise useful to LLVM for years?

On Thu, Apr 18, 2019 at 2:03 PM Don Hinton <hintonda at gmail.com>
wrote:
>
> Hi David:
>
> I'd actually like to use a custom parser in a tool I'm developing,
parsing YAML files.  The partial fix for my purposes was:
>
> --- a/llvm/include/llvm/Support/CommandLine.h
> +++ b/llvm/include/llvm/Support/CommandLine.h
> @@ -1044,7 +1044,7 @@ extern template class basic_parser<float>;
>  //--------------------------------------------------
>  // parser<std::string>
>  //
> -template <> class parser<std::string> final : public
basic_parser<std::string> {
> +template <> class parser<std::string> : public
basic_parser<std::string> {
>  public:
>    parser(Option &O) : basic_parser(O) {}
>
> @@ -1062,6 +1062,8 @@ public:
>
>    // An out-of-line virtual method to provide a 'home' for this
class.
>    void anchor() override;
> +
> +  virtual ~parser() = default;
>  };
>
> If that's acceptable, I submit a patch.
>
> thanks...
> don
>
> On Thu, Apr 18, 2019 at 1:56 PM David Blaikie <dblaikie at gmail.com>
wrote:
>>
>> Yeah, I think I did this a while back (& you're possibly not
the only
>> one who's hit this - might be worth a search on llvm-dev for
previous
>> threads).
>>
>> If you check the commit history, I think my original motivation was
>> cleaning up the virtual dtor that wasn't used in LLVM by making the
>> base class's dtor protected, and the derived classes final.
>>
>> It could be made virtual instead of protected/final - but without any
>> use in LLVM, I'm not sure it's worth exposing this extension
point &
>> perhaps just updating the documentation is the best thing?
>>
>> On Thu, Apr 18, 2019 at 1:45 PM Don Hinton via llvm-dev
>> <llvm-dev at lists.llvm.org> wrote:
>> >
>> > https://llvm.org/docs/CommandLine.html#writing-a-custom-parser
describes how to implement a custom parser, but they're all marked `final`,
so I can't inherit.
>> >
>> > Is there a reason for this?  If not, I'll submit patch with
tests.
>> >
>> > thanks...
>> > don
>> >
>> >
>> > _______________________________________________
>> > LLVM Developers mailing list
>> > llvm-dev at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On Thu, Apr 18, 2019 at 2:06 PM David Blaikie <dblaikie at gmail.com>
wrote:
> I don't feel super strongly either way - though if you're going to
> make parser's dtor virtual, could you remove 'final' from the
other
> parsers too?
> Any idea what's different about your situation that makes this feature
> useful but it hasn't been otherwise useful to LLVM for years?
>
I'm one of the RTFM guys -- I read everything and try to use the cool stuff.

I could just grab the string, then process it later, but that's what the
parser is for, so I figured I'd use it.  In my case, I have a family of
passes that take a yaml config file -- it'll override individual options,
so the user can provide either the file, individual options, or both.

Anyway, if I did it myself, I'd have to add the call to each pass -- more a
less a singleton -- but if the parser does it for me, it makes my life
easier.

> On Thu, Apr 18, 2019 at 2:03 PM Don Hinton <hintonda at gmail.com>
wrote:
> >
> > Hi David:
> >
> > I'd actually like to use a custom parser in a tool I'm
developing,
> parsing YAML files.  The partial fix for my purposes was:
> >
> > --- a/llvm/include/llvm/Support/CommandLine.h
> > +++ b/llvm/include/llvm/Support/CommandLine.h
> > @@ -1044,7 +1044,7 @@ extern template class basic_parser<float>;
> >  //--------------------------------------------------
> >  // parser<std::string>
> >  //
> > -template <> class parser<std::string> final : public
> basic_parser<std::string> {
> > +template <> class parser<std::string> : public
> basic_parser<std::string> {
> >  public:
> >    parser(Option &O) : basic_parser(O) {}
> >
> > @@ -1062,6 +1062,8 @@ public:
> >
> >    // An out-of-line virtual method to provide a 'home' for
this class.
> >    void anchor() override;
> > +
> > +  virtual ~parser() = default;
> >  };
> >
> > If that's acceptable, I submit a patch.
> >
> > thanks...
> > don
> >
> > On Thu, Apr 18, 2019 at 1:56 PM David Blaikie <dblaikie at
gmail.com>
> wrote:
> >>
> >> Yeah, I think I did this a while back (& you're possibly
not the only
> >> one who's hit this - might be worth a search on llvm-dev for
previous
> >> threads).
> >>
> >> If you check the commit history, I think my original motivation
was
> >> cleaning up the virtual dtor that wasn't used in LLVM by
making the
> >> base class's dtor protected, and the derived classes final.
> >>
> >> It could be made virtual instead of protected/final - but without
any
> >> use in LLVM, I'm not sure it's worth exposing this
extension point &
> >> perhaps just updating the documentation is the best thing?
> >>
> >> On Thu, Apr 18, 2019 at 1:45 PM Don Hinton via llvm-dev
> >> <llvm-dev at lists.llvm.org> wrote:
> >> >
> >> >
https://llvm.org/docs/CommandLine.html#writing-a-custom-parser
> describes how to implement a custom parser, but they're all marked
`final`,
> so I can't inherit.
> >> >
> >> > Is there a reason for this?  If not, I'll submit patch
with tests.
> >> >
> >> > thanks...
> >> > don
> >> >
> >> >
> >> > _______________________________________________
> >> > LLVM Developers mailing list
> >> > llvm-dev at lists.llvm.org
> >> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190418/6923f485/attachment.html>