kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.
I see, thank you very much for the explanation!
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:410
+ if (Completion.Deprecated) {
+ Completion.Deprecated =
+ (C.SemaResult &&
----------------
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.
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
