[PATCH] D56916: Fix crash due to ObjCPropertyDecl

dgoldman added a comment. In D56916#1367559 , @ilya-biryukov wrote: > It looks like `Container::_magic` is a platform-dependent completion, I don't > have it on Linux, but >

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov added a comment. It looks like `Container::_magic` is a platform-dependent completion, I don't have it on Linux, but http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/42665 fails because it's not in the list. Submitted rL351943

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov added a comment. Done. I had to remove `Container::_magic` from the list of expeced symbols to make the test pass, let me know if it should actually be there. Again, thanks for the fix! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56916/new/

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

This revision was automatically updated to reflect the committed changes. Closed by commit rL351941: [clangd] Fix crash due to ObjCPropertyDecl (authored by ibiryukov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

dgoldman added a comment. In D56916#1366456 , @ilya-biryukov wrote: > > Unfortunately I can't get ninja check-clangd to build: > > Looks like `clang-tools-extra` uses an old revision. Rebasing after rL350916 > should

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

dgoldman updated this revision to Diff 182942. dgoldman added a comment. Herald added a subscriber: jfb. - FIXME and dyn_cast - use auto - Add test Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56916/new/ https://reviews.llvm.org/D56916 Files:

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov added a comment. > Unfortunately I can't get ninja check-clangd to build: Looks like `clang-tools-extra` uses an old revision. Rebasing after rL350916 should do the trick. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

dgoldman updated this revision to Diff 182921. dgoldman marked an inline comment as done. dgoldman added a comment. - use auto Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56916/new/ https://reviews.llvm.org/D56916 Files:

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

dgoldman added a comment. In D56916#1365357 , @ilya-biryukov wrote: > Good point, @aaron.ballman! @dgoldman, could you please add a test case? Unfortunately I can't get ninja check-clangd to build:

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov added a comment. Good point, @aaron.ballman! @dgoldman, could you please add a test case? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56916/new/ https://reviews.llvm.org/D56916 ___

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

aaron.ballman added a comment. Test case? Or does one already cover this crash? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56916/new/ https://reviews.llvm.org/D56916 ___ cfe-commits mailing list

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov added a comment. @dgoldman, do you have commit access or should I land this for you? Comment at: clangd/index/SymbolCollector.cpp:375 + // not a NamedDecl. + const NamedDecl *OriginalDecl = dyn_cast(ASTNode.OrigD); + if (!OriginalDecl) NIT:

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

dgoldman updated this revision to Diff 182562. dgoldman marked an inline comment as done. dgoldman added a comment. - FIXME and dyn_cast Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56916/new/ https://reviews.llvm.org/D56916 Files:

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks for fixing this. Quick LGTM to fix a crash, albeit a fews NITs. Comment at: clangd/index/SymbolCollector.cpp:372 + // ObjCPropertyDecl may have an

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

dgoldman created this revision. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, ioeric, ilya-biryukov. With ObjCPropertyDecl, ASTNode.OrigD can be a ObjCPropertyImplDecl which is not a NamedDecl, leading to a crash since the code incorrectly assumes ASTNode.OrigD will always