[PATCH] D60955: [llvm] Remove final and add default virtual dtor to CommandLine parsers

Tue Apr 23 07:52:42 PDT 2019

hintonda marked an inline comment as done.
hintonda added a comment.

In D60955#1474817 <https://reviews.llvm.org/D60955#1474817>, @dblaikie wrote:

> In D60955#1474642 <https://reviews.llvm.org/D60955#1474642>, @craig.topper wrote:
>
> > This seems to reversing r231221 which devirtualized this.  What kind of customization are you envisioning? There aren't many methods here. Are you planning to override the parse method but still fall back to the base class version sometimes?
>
>
> Sorry, yeah, there's some missing context in this review description - it was discussed on llvm-dev here: http://lists.llvm.org/pipermail/llvm-dev/2019-April/131873.html

Sorry, I'll update the description.  As stated in the thread, I want to customize the `std::string` parser, specifically the `parse` method, to treat the string as a yaml file name, and use `yaml::Input` to populate a struct containing all my options.  I also customize the `bool` parser to tread it as a flag to generate an example yaml file to give users something to start with.

================
Comment at: llvm/include/llvm/Support/CommandLine.h:901-902
   void anchor() override;
+
+  virtual ~parser() = default;
 };
----------------
dblaikie wrote:
> Might as well make the dtor public and virtual in the basic_parser class & remove all these explicitly virtual dtor definitions in the derived classes, I guess?
Thanks -- will do.  I should have looked at the original diff first instead of just mechanically applying my spot fix to everything else.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60955/new/

https://reviews.llvm.org/D60955