llvm dev - Oct 2015 - llvm:cl::parser subclasses final in 3.7?

All,

I'm upgrading some code that uses LLVM from 3.6 to 3.7. It looks like the
llvm::cl::parser subclasses are now final?

We had been doing:

struct MaxThreadsParser : public llvm::cl::parser<unsigned> {
  bool parse(llvm::cl::Option &O, llvm::StringRef ArgName, llvm::StringRef
Arg,
             unsigned &Val);
};

But that's now causing:

In file included from
/home/lak/my_svn/llvm-carte/llvm-3.7.0/tools/carte++/tools/ir2v/ThreadSupport.cpp:1:
/home/lak/my_svn/llvm-carte/llvm-3.7.0/tools/carte++/tools/ir2v/ThreadSupport.h:12:34:
error:
      base 'parser' is marked 'final'
struct MaxThreadsParser : public llvm::cl::parser<unsigned> {

What's the new way to do this now? It looks like the documentation at
http://llvm.org/releases/3.7.0/docs/CommandLine.html#custom-parsers
describes a way that doesn't work anymore.

Thanks,
Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151008/7f11841d/attachment.html>

On Thu, Oct 8, 2015 at 3:36 PM, Matthew O'Connor via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> All,
>
> I'm upgrading some code that uses LLVM from 3.6 to 3.7. It looks like
the
> llvm::cl::parser subclasses are now final?
>
> We had been doing:
>
> struct MaxThreadsParser : public llvm::cl::parser<unsigned> {
>   bool parse(llvm::cl::Option &O, llvm::StringRef ArgName,
llvm::StringRef
> Arg,
>              unsigned &Val);
> };
>
> But that's now causing:
>
> In file included from
>
/home/lak/my_svn/llvm-carte/llvm-3.7.0/tools/carte++/tools/ir2v/ThreadSupport.cpp:1:
>
/home/lak/my_svn/llvm-carte/llvm-3.7.0/tools/carte++/tools/ir2v/ThreadSupport.h:12:34:
> error:
>       base 'parser' is marked 'final'
> struct MaxThreadsParser : public llvm::cl::parser<unsigned> {
>
> What's the new way to do this now? It looks like the documentation at
> http://llvm.org/releases/3.7.0/docs/CommandLine.html#custom-parsers
> describes a way that doesn't work anymore.
>
Yeah, someone filed https://llvm.org/bugs/show_bug.cgi?id=25104 for this
recently.

I was the person who made this API change - hadn't seen the documentation
we had suggesting/encouraging people to do this, but I'm sort of inclined
to say that was a mistake - but open to discussion.

The reason for the change was to devirtualize parser<T>'s dtor - since
it
wasn't ever owned/destroyed polymorphically, so there was no need for it.
But to keep the type safe, this meant making the dtor protected in the base
class and making the derived classes final (so there couldn't be further
derived classes and then polymorphic ownership of the intermediate class
type).

Not sure how other people feel about it - whether this is a particularly
compelling extension point for us to support.

It looks like you could just write your class as a subclass of
basic_parser<unsigned> and go from there instead of deriving from
parser<T>
directly. You might have to implement one or two more functions that way.

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151008/4eeec981/attachment.html>

On Thu, Oct 8, 2015 at 5:01 PM, David Blaikie <dblaikie at gmail.com>
wrote:
>
>
> On Thu, Oct 8, 2015 at 3:36 PM, Matthew O'Connor via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> All,
>>
>> I'm upgrading some code that uses LLVM from 3.6 to 3.7. It looks
like the
>> llvm::cl::parser subclasses are now final?
>>
>> We had been doing:
>>
>> struct MaxThreadsParser : public llvm::cl::parser<unsigned> {
>>   bool parse(llvm::cl::Option &O, llvm::StringRef ArgName,
>> llvm::StringRef Arg,
>>              unsigned &Val);
>> };
>>
>> But that's now causing:
>>
>> In file included from
>>
/home/lak/my_svn/llvm-carte/llvm-3.7.0/tools/carte++/tools/ir2v/ThreadSupport.cpp:1:
>>
/home/lak/my_svn/llvm-carte/llvm-3.7.0/tools/carte++/tools/ir2v/ThreadSupport.h:12:34:
>> error:
>>       base 'parser' is marked 'final'
>> struct MaxThreadsParser : public llvm::cl::parser<unsigned> {
>>
>> What's the new way to do this now? It looks like the documentation
at
>> http://llvm.org/releases/3.7.0/docs/CommandLine.html#custom-parsers
>> describes a way that doesn't work anymore.
>>
>
> Yeah, someone filed https://llvm.org/bugs/show_bug.cgi?id=25104 for this
> recently.
>
> I was the person who made this API change - hadn't seen the
documentation
> we had suggesting/encouraging people to do this, but I'm sort of
inclined
> to say that was a mistake - but open to discussion.
>
> The reason for the change was to devirtualize parser<T>'s dtor -
since it
> wasn't ever owned/destroyed polymorphically, so there was no need for
it.
> But to keep the type safe, this meant making the dtor protected in the base
> class and making the derived classes final (so there couldn't be
further
> derived classes and then polymorphic ownership of the intermediate class
> type).
>
> Not sure how other people feel about it - whether this is a particularly
> compelling extension point for us to support.
>
> It looks like you could just write your class as a subclass of
> basic_parser<unsigned> and go from there instead of deriving from
parser<T>
> directly. You might have to implement one or two more functions that way.
>
> - David
>
It ended up being fairly simple to subclass basic_parser<unsigned>, though
it did require a non-obvious (to me), explicit template specialization of
llvm::cl::printOptionDiff. I liked the simplicity of the being able to
subclass parser<T> and define a single method, but it really doesn't
matter
that much.

Thanks for the information and for pointing me in the right direction.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151012/a91946b0/attachment.html>