[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-11-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0df197516b69: [clangd] Value initialize SymbolIDs (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-11-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolID.h:58 + bool isNull() const { return HashValue != std::array{}; } + operator bool() const { return isNull(); } + sammccall wrote: > sammccall wrote: > > nit: I think you want

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-11-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 302230. kadircet marked 4 inline comments as done. kadircet added a comment. - Mention possibility of returning null SymbolIDs in comments. - Mark bool conversion operator explicit, fix the bug in returned value. (and run tests :)) Repository: rG LLVM

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D90397#2366769 , @sammccall wrote: > In D90397#2366727 , @nridge wrote: > >> Would it have been possible to disallow default-constructing `SymbolID` >> altogether, and preserve the

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. In D90397#2366727 , @nridge wrote: > Would it have been possible to disallow default-constructing `SymbolID` > altogether, and preserve the

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Would it have been possible to disallow default-constructing `SymbolID` altogether, and preserve the ability to represent "an always-valid symbol id" (`SymbolID`) vs. "a maybe-valid symbol id" (`Optional`) as distinct types in the type system? Repository: rG LLVM

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 301678. kadircet marked an inline comment as done. kadircet added a comment. - change invalid -> null. - add an implicit bool conversion operator. - update apis returning optionals. Note that all the functional changes are in SymbolID.h, rest is api

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-29 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolID.h:57 + bool isValid() const { return HashValue != std::array{}; } + I'm not a big fan of LLVM's habit of calling the sentinel value "invalid". Consider SourceLocation - one

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. We were default initializing SymbolIDs