sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:410
+ if (Completion.Deprecated) {
+ Completion.Deprecated =
+ (C.SemaResult &&
----------------
kbobyrev wrote:
> sammccall wrote:
> > kbobyrev wrote:
> > > The comment you added says "cleared" which means this should probably be
> > > `|=` rather than `=`, right?
> > >
> > > Also, I this looks longer but probably more readable at least for me.
> > >
> > > Also, the sources might be `SemaResult`, `IndexResult` or
> > > `IdentifierResult`, right? :( Otherwise could've been
> > > `Completion.Deprecated |= C.SemaResult ? C.SemaResult->Availability ==
> > > CXAvailability_Deprecated : C.IndexResult->Flags & Symbol::Deprecated;`
> > > The comment you added says "cleared" which means this should probably be
> > > |= rather than =, right?
> >
> > No, `Deprecated` *starts out true* and gets set to false (cleared) if we
> > see any non-deprecated entry. (computes AND)
> >
> > Your version assumes it starts out false and sets it if we see any
> > deprecated entry. (computes OR).
> >
> > I agree the OR version reads better - it's wrong though :-)
> Ahh, okay, makes sense, thank you!
>
> Nit: I think the version I suggested (with fixes `|=` vs `=`) is somewhat
> easier to read and doesn't take much more space.
Done, though with `&=` instead of `=` otherwise the logic is subtly different
(the second clobbers the first).
And because of &= we need an explicit cast to bool.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97803/new/
https://reviews.llvm.org/D97803
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
