[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-20 Thread David Goldman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd5c022d84699: [clangd][ObjC] Support nullability annotations (authored by dgoldman). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89579/new/ https://review

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-20 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 299356. dgoldman added a comment. Revert to previous AttributedTypeLoc behavior Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89579/new/ https://reviews.llvm.org/D89579 Files: clang-tools-extra/clangd/Selec

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-20 Thread David Goldman via Phabricator via cfe-commits
dgoldman added a comment. In D89579#2341513 , @sammccall wrote: > Yep, let's revert to the previous state and land that, and I'll puzzle over > the examples you give (because always returning false shouldn't affect > behavior, just performance). > > I ha

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Yep, let's revert to the previous state and land that, and I'll puzzle over the examples you give (because always returning false shouldn't affect behavior, just performance). I have put together D89785 for more general attribute sup

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-19 Thread David Goldman via Phabricator via cfe-commits
dgoldman added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:611 + // robust. + return false; if (!SelChecker.mayHit(S)) { sammccall wrote: > I'm not sure we actually want to *do it* at this point, as you're seeing we > may h

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:611 + // robust. + return false; if (!SelChecker.mayHit(S)) { I'm not sure we actually want to *do it* at this point, as you're seeing we may have some unintended

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-19 Thread David Goldman via Phabricator via cfe-commits
dgoldman added a comment. With that change, somehow this extract test is now failing: - TEST 'Clangd Unit Tests :: ./ClangdTests/ExtractFunctionTest.FunctionTest' FAILED Note: Google Test filter = ExtractFunctionTest.FunctionTest [==] Running 1 test from 1 test case

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-19 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 299038. dgoldman marked 5 inline comments as done. dgoldman added a comment. Update with changes requested - Looking into extract failure Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89579/new/ https://revie

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:621-623 + if (auto AT = TL->getAs()) +S = AT.getModifiedLoc() +.getSourceRange(); // should we just return false? dgoldman wrote: > Let me know if you

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-16 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 298759. dgoldman added a comment. Fix merge Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89579/new/ https://reviews.llvm.org/D89579 Files: clang-tools-extra/clangd/Selection.cpp clang-tools-extra/clangd/

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-16 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 298757. dgoldman added a comment. Lint fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89579/new/ https://reviews.llvm.org/D89579 Files: clang-tools-extra/clangd/Selection.cpp clang-tools-extra/clangd

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-16 Thread David Goldman via Phabricator via cfe-commits
dgoldman added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:621-623 + if (auto AT = TL->getAs()) +S = AT.getModifiedLoc() +.getSourceRange(); // should we just return false? Let me know if you think it would be

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

2020-10-16 Thread David Goldman via Phabricator via cfe-commits
dgoldman created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman. Herald added a project: clang. dgoldman requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Nullability annotations are implmented using attributes; previusly clan