[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-09-14 Thread David Goldman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8401713b3ef1: [clangd] Ignore ObjC `id` and `instancetype` in FindTarget (authored by dgoldman). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108556/new/

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-09-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. sorry, I thought I've already LGTM'd it in previous iteration. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108556/new/

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-09-13 Thread David Goldman via Phabricator via cfe-commits
dgoldman marked an inline comment as done. dgoldman added a comment. Friendly ping, PTAL Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108556/new/ https://reviews.llvm.org/D108556 ___ cfe-commits

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-09-07 Thread David Goldman via Phabricator via cfe-commits
dgoldman marked 2 inline comments as done. dgoldman added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:188 if (const TypedefNameDecl *TND = dyn_cast(D)) { + if (shouldSkipTypedef(TND)) +return; kadircet wrote: > why do

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-09-07 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 371103. dgoldman added a comment. Update comment + remove un-needed code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108556/new/ https://reviews.llvm.org/D108556 Files:

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-09-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks, this looks great! just a question about the extra handling Comment at: clang-tools-extra/clangd/FindTarget.cpp:100 + // Even though ObjC `id` and `instancetype` are *implemented* via typedefs, we + // don't want to treat them like typedefs

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-09-03 Thread David Goldman via Phabricator via cfe-commits
dgoldman added a comment. In D108556#2981712 , @kadircet wrote: > Hmm, it sounds like you want them to be treated one way during semantic > highlighting and another during other features, which is fine but somewhat > confusing. (e.g. we want to surface

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-09-03 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 370609. dgoldman added a comment. Swap to ignore in FindTarget Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108556/new/ https://reviews.llvm.org/D108556 Files: clang-tools-extra/clangd/FindTarget.cpp

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-09-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Hmm, it sounds like you want them to be treated one way during semantic highlighting and another during other features, which is fine but somewhat confusing. (e.g. we want to surface hover/goto on these identifiers, but we're making them less visible by encouraging

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-08-24 Thread David Goldman via Phabricator via cfe-commits
dgoldman added a comment. In D108556#2962008 , @kadircet wrote: > thanks, it looks good as a contained fix. but it feels like we probably don't > want these to be treated as decls in other places too (e.g. can we really > provide any useful goto/hover

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-08-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks, it looks good as a contained fix. but it feels like we probably don't want these to be treated as decls in other places too (e.g. can we really provide any useful goto/hover on those `id` or `instancetype` decls?) maybe we should update ASTVisitors in

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-08-23 Thread David Goldman via Phabricator via cfe-commits
dgoldman created this revision. dgoldman added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman. dgoldman requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Even though they're implemented