[libcxx-commits] [PATCH] D116324: [libc++][NFC] remove this-> when calling member functions in <string>

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/string:1718
 #ifndef _LIBCPP_NO_EXCEPTIONS
         __basic_string_common<true>::__throw_length_error();
 #else
----------------
My suggestion here will merge-conflict with D116334, but: let's take this opportunity to be consistent. If we're removing `this->`s, we should also remove `Base::`s. Contra @Mordante below, I think CI //will// be happy with this patch, because while `__basic_string_common<true>` is a base class of a class template, it is not a //dependent// base class.

Btw, as a general rule I would much rather see `this->` than `Base::`, when the thing is indeed a dependent base class. Saves brain cells //and// generally looks less arcane and scary.

However, in light of D116334 and my comment that I think you may be asked to keep the calls to the external library function for code-size reasons: maybe you should think about rewriting all these calls as `__basic_string_common<true>::__throw_length_error();` so that then they'll keep working after `__basic_string_common<true>` becomes not-a-base-class-anymore.

Unless I'm mistaken, I think it will ultimately make sense to abandon D116324 and roll its surviving changes (if any) into D116334.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116324