remove this-> when calling member functions
Details
- Reviewers
• Quuxplusone Mordante ldionne - Group Reviewers
Restricted Project - Commits
- rG52f37c24c3f8: [libc++][NFC] remove this-> when calling member functions in <string>
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I'm happy when the CI is happy, but the CI won't be happy ;-) D116334 should be a parent revision.
libcxx/include/string | ||
---|---|---|
1733 | 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. |
Nope, LGTM! (@Mordante already accepted, and I don't see anything weird here, so accepting as libc++)
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.