sammccall added a comment. Herald added a project: clang-tools-extra. Whoops, lost this patch...
In D97803#2602787 <https://reviews.llvm.org/D97803#2602787>, @kbobyrev wrote: > So, if my understanding is correct, this will make the whole bundle > non-deprecated if at least one overload is not deprecated? This is probably > an improvement over the existing behaviour. Yes, exactly. > However, do we maybe want to split the bundles into deprecated and > non-deprecated groups? I don't think splitting bundles over this is worthwhile. - it's a distraction. The point of bundling is to hide some differences within an overload set, to let you see all the methods more clearly. - the "deprecated" signal means "you don't want this". This is well-defined, and false, for a mixed bundle. ================ Comment at: clang-tools-extra/clangd/CodeComplete.cpp:410 + if (Completion.Deprecated) { + Completion.Deprecated = + (C.SemaResult && ---------------- 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 :-) 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
