[PATCH] D94919: [clangd] Fix a crash when indexing invalid ObjC method declaration

[David Goldman via Phabricator via cfe-commits]

dgoldman added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:1807-1811
+    @interface Foo
+    - (void)fun:(bool)foo
+      bar:(bool)bar,
+      baz:(bool)baz;
+    @end
----------------
It's hard to tell what Clang makes of this, I think instead we should test out the root cause of this crash, IIUC, are the legacy C style parameters:

```
@interface Foo
- (void)func:(bool)foo, bool bar;
@end
```


================
Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:1816-1819
+  // We mostly care about not crashing, but verify that we didn't insert garbage
+  // about X too.
+  EXPECT_THAT(TU.headerSymbols(), Not(Contains(QName("X"))));
+}
----------------
We should verify the method name is exactly as expected but comment we mostly care about not crashing.


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3519
                                               PEnd = Method->param_end();
          P != PEnd; (void)++P, ++Idx) {
       if (Idx > 0) {
----------------
I think it might be simpler just to check the index:

```
unsigned NumSelectorArgs = Sel.getNumArgs();

P != PEnd && Idx < NumSelectorArgs
```

and note that we need to check this due to legacy use of C-style parameters in Objective-C method declarations which in practice only occurs due to typos



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94919/new/

https://reviews.llvm.org/D94919