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

[Kirill Bobyrev via Phabricator via cfe-commits]

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