[PATCH] D97803: [clangd] Overload bundles are only deprecated if each overloads is.

[Sam McCall via Phabricator via cfe-commits]

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