[PATCH] D41674: [Support] CommandLine API -- Allow creating custom parsers for fundamental types

[Christoph Kindl via Phabricator via llvm-commits]

ckristo added a comment.

In https://reviews.llvm.org/D41674#967275, @serge-sans-paille wrote:

> +1 for this patch.


Hi,

Thanks for reviewing my little patch :-)

In https://reviews.llvm.org/D41674#967275, @serge-sans-paille wrote:

> I have the feeling that the virtual qualifier on printOptionDiff could be avoided, as this method does not seem to be called through base class at any point ?


I tried to avoid making printOptionDiff virtual, but I ended up in calling basic_parser's (dummy) method implementation instead of the concrete child class' implementation. Removing virtual might be possible when revisiting the PrintOptionDiff template methods (lines 1086ff) to always pass the parser type somehow (and making a cast to the concrete parser type before calling the parser's printOptionDiff method), but to be honest I did not want to touch the templates much because I do not really get what they're doing/needed for and which version is selected in which case. Furthermore, I thought making printOptionDiff virtual does not make much of a difference because basic_parser had virtual functions already. Anyhow, if you have more insight in the PrintOptionDiff template voodoo and see a method in making printOptionDiff non-virtual, I'd be happy if you could guide me here.


https://reviews.llvm.org/D41674