[PATCH] D78545: [RISCV] Make CanLowerReturn protected for downstream maintenance

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

Looks like this landed while I was composing the below:

It does look like most backends declare this as private rather than public. We've adopted that same convention, but I can't see a particularly coherent dividing line between which virtual methods (which are public in TargetLowering) are public/private in the derived class.

I think it would actually be better to go ahead and make this public (like it is in parent class) rather than establishing a precedent for protected (which is probably more confusing than the current situation as it's not clear on what basis overrides are public/private/protected). If you just make it public, I don't think we need a comment to explain why, but likely want to follow-up to better organise the overrides.

I see SystemZISelLowering.h is almost consistent in keeping overrides public (for some reason `unwrapAddress` is private). Unless there's a an argument in favour of making overrides private that I'm missing, I'd be open to a patch that moved all the overrides to public.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78545