[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