[libcxx-commits] [PATCH] D126621: [libc++] Fix typo in comment at __optional_storage_base

Will Hawkins via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon May 30 09:18:50 PDT 2022


hawkinsw marked an inline comment as done.
hawkinsw added inline comments.


================
Comment at: libcxx/include/optional:396
 
-// optional<T&> is currently required ill-formed, however it may to be in the
+// optional<T&> is currently required to be ill-formed, however it may be in the
 // future. For this reason it has already been implemented to ensure we can
----------------
jloser wrote:
> hawkinsw wrote:
> > hawkinsw wrote:
> > > jloser wrote:
> > > > philnik wrote:
> > > > > This sentence still doesn't make any sense. I think there is a "not" missing somewhere.
> > > > I'd be OK with something like this instead - what do you think?
> > > > 
> > > > 
> > > > 
> > > > > // optional<T&> is currently required to be ill-formed. However, it may be allowed in the
> > > > > // future. For this reason, it has already been implemented to ensure we can make the change in an ABI-compatible manner.
> > > > 
> > > > 
> > > Yes, of course!! I completely boofed. I was so excited to be able to fit that all within 80 characters that I never thought to reread! I am *so* sorry!
> > > 
> > > Also, thank you for being as persnickety as I am -- there is definitely a dash needed between ABI and compatible!!
> > And, for the , as well (after the opening clause). You and I are obviously of one mind!
> Just one small thing: do you mind adding the comma after "However", please? Sorry that I'm such a nitpick with this. 
> 
> After that, LGTM.
Thanks for the feedback! I posted a follow up!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126621



More information about the libcxx-commits mailing list