Page MenuHomePhabricator

dgoldman (David Goldman)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 26 2018, 1:58 PM (129 w, 10 h)

Recent Activity

Tue, Apr 6

dgoldman added inline comments to D99975: [clangd][ObjC] Improve support for class properties.
Tue, Apr 6, 10:38 AM · Restricted Project
dgoldman updated the diff for D99975: [clangd][ObjC] Improve support for class properties.
  • Add FIXMEs
Tue, Apr 6, 10:37 AM · Restricted Project
dgoldman requested review of D99975: [clangd][ObjC] Improve support for class properties.
Tue, Apr 6, 10:34 AM · Restricted Project

Sun, Mar 21

dgoldman added inline comments to D98984: [clangd] Improve handling of Objective-C protocols in types.
Sun, Mar 21, 7:34 AM · Restricted Project

Fri, Mar 19

dgoldman added a reviewer for D98984: [clangd] Improve handling of Objective-C protocols in types: sammccall.
Fri, Mar 19, 1:56 PM · Restricted Project
dgoldman requested review of D98984: [clangd] Improve handling of Objective-C protocols in types.
Fri, Mar 19, 1:25 PM · Restricted Project

Mar 3 2021

dgoldman accepted D97617: [clangd] ObjC fixes for semantic highlighting and xref highlights.
Mar 3 2021, 7:15 AM · Restricted Project

Mar 1 2021

dgoldman added a comment to D97617: [clangd] ObjC fixes for semantic highlighting and xref highlights.

(not really sure why this suddenly seemed important to me, but if you don't have semantic highlighting on, you're missing out!)

Mar 1 2021, 10:31 AM · Restricted Project
dgoldman committed rG5a2141e3a08c: [clangd] Improve document symbols support for Objective-C categories and methods (authored by dgoldman).
[clangd] Improve document symbols support for Objective-C categories and methods
Mar 1 2021, 9:38 AM
dgoldman closed D96612: [clangd] Improve printing of Objective-C categories and methods.
Mar 1 2021, 9:38 AM · Restricted Project
dgoldman updated the diff for D96612: [clangd] Improve printing of Objective-C categories and methods.

Add comment and rebase

Mar 1 2021, 9:29 AM · Restricted Project

Feb 26 2021

dgoldman updated the summary of D96612: [clangd] Improve printing of Objective-C categories and methods.
Feb 26 2021, 9:11 AM · Restricted Project
dgoldman updated the diff for D96612: [clangd] Improve printing of Objective-C categories and methods.

Limit changes to document symbols

Feb 26 2021, 9:10 AM · Restricted Project

Feb 16 2021

dgoldman added inline comments to D96612: [clangd] Improve printing of Objective-C categories and methods.
Feb 16 2021, 3:10 PM · Restricted Project
dgoldman added inline comments to D96612: [clangd] Improve printing of Objective-C categories and methods.
Feb 16 2021, 3:06 PM · Restricted Project

Feb 12 2021

dgoldman updated the diff for D96612: [clangd] Improve printing of Objective-C categories and methods.
  • Add class method test case + swap to auto
Feb 12 2021, 12:20 PM · Restricted Project
dgoldman requested review of D96612: [clangd] Improve printing of Objective-C categories and methods.
Feb 12 2021, 8:59 AM · Restricted Project
dgoldman committed rG07c5a800dc17: Improve hover scopes for Objective-C code (authored by dgoldman).
Improve hover scopes for Objective-C code
Feb 12 2021, 7:28 AM
dgoldman closed D68590: [clangd] Improve hover scopes for Objective-C code.
Feb 12 2021, 7:28 AM · Restricted Project

Feb 11 2021

dgoldman updated the summary of D68590: [clangd] Improve hover scopes for Objective-C code.
Feb 11 2021, 2:27 PM · Restricted Project
dgoldman updated the diff for D68590: [clangd] Improve hover scopes for Objective-C code.
  • Support protocols
Feb 11 2021, 2:25 PM · Restricted Project
dgoldman updated the diff for D68590: [clangd] Improve hover scopes for Objective-C code.
  • Move over to AST.cpp
Feb 11 2021, 2:14 PM · Restricted Project
dgoldman added inline comments to D68590: [clangd] Improve hover scopes for Objective-C code.
Feb 11 2021, 1:53 PM · Restricted Project
dgoldman updated the diff for D68590: [clangd] Improve hover scopes for Objective-C code.
  • Address review comments
Feb 11 2021, 1:53 PM · Restricted Project

Feb 10 2021

dgoldman added a comment to D68590: [clangd] Improve hover scopes for Objective-C code.

Friendly ping, I think this is still worth merging in even without the QName changes

Feb 10 2021, 8:36 AM · Restricted Project

Jan 22 2021

dgoldman accepted D94919: [clangd] Fix a crash when indexing invalid ObjC method declaration.

The crash happens while indexing the file, not during code completion. The code completion tests generally specify a cursor position, but in this case it is not relevant and could confuse the reader. This seemed like a logical place for indexing bug, in the spirit of using minimal repro case (i.e. you don't need to trigger code completion at all)

Jan 22 2021, 10:21 AM · Restricted Project
dgoldman requested changes to D94919: [clangd] Fix a crash when indexing invalid ObjC method declaration.

Looks good but I think for posterity we should add a similar test to clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp for the exact intended code complete behavior (or instead of SymbolCollectorTests.cpp - is there a particular reason you added a test there)?

Jan 22 2021, 9:24 AM · Restricted Project

Jan 21 2021

dgoldman added inline comments to D94919: [clangd] Fix a crash when indexing invalid ObjC method declaration.
Jan 21 2021, 12:48 PM · Restricted Project
dgoldman added a reviewer for D94919: [clangd] Fix a crash when indexing invalid ObjC method declaration: dgoldman.
Jan 21 2021, 12:04 PM · Restricted Project

Dec 7 2020

dgoldman added a comment to D68590: [clangd] Improve hover scopes for Objective-C code.

I messed around with qualified name changes - lots of things internally rely upon the qualified names so that will be best for a separate change. I could potentially handle DocumentSymbol fixes in here though - LMK if you think it makes sense to move some of this logic in AST.cpp (clangd). objcContainerLocalScope seems like it would be useful to generalize support for objc container decls by ensuring that categories becoming fully qualified with their class name.

Dec 7 2020, 12:21 PM · Restricted Project
dgoldman updated the diff for D68590: [clangd] Improve hover scopes for Objective-C code.

Add protocol test

Dec 7 2020, 12:16 PM · Restricted Project
dgoldman updated the diff for D68590: [clangd] Improve hover scopes for Objective-C code.

Swap to isa

Dec 7 2020, 12:08 PM · Restricted Project

Dec 4 2020

dgoldman added a comment to D68590: [clangd] Improve hover scopes for Objective-C code.

I think there's still some more work to be done after this which might move some of this around:

Dec 4 2020, 12:27 PM · Restricted Project
dgoldman retitled D68590: [clangd] Improve hover scopes for Objective-C code from [clangd] Improve hover support for Objective-C to [clangd] Improve hover scopes for Objective-C code.
Dec 4 2020, 12:20 PM · Restricted Project
dgoldman added reviewers for D68590: [clangd] Improve hover scopes for Objective-C code: benlangmuir, gribozavr2.
Dec 4 2020, 12:18 PM · Restricted Project
dgoldman updated the diff for D68590: [clangd] Improve hover scopes for Objective-C code.
  • Rebase + refactor, sorry for the long delay
Dec 4 2020, 12:15 PM · Restricted Project

Oct 20 2020

dgoldman committed rGd5c022d84699: [clangd][ObjC] Support nullability annotations (authored by dgoldman).
[clangd][ObjC] Support nullability annotations
Oct 20 2020, 2:37 PM
dgoldman closed D89579: [clangd][ObjC] Support nullability annotations.
Oct 20 2020, 2:37 PM · Restricted Project
dgoldman updated the diff for D89579: [clangd][ObjC] Support nullability annotations.

Revert to previous AttributedTypeLoc behavior

Oct 20 2020, 7:38 AM · Restricted Project
dgoldman added a comment to D89579: [clangd][ObjC] Support nullability annotations.

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 support, and it has a generalization of the fix here. (It returns false for any node with an attribute attached).
But it's worth landing this first as it has good tests for the objc cases, and that patch has its own prerequisites and risks of regressions.
(Not a timely coincidence, rather I got curious about the AST around Attrs after seeing this patch)

Oct 20 2020, 7:10 AM · Restricted Project

Oct 19 2020

dgoldman added inline comments to D89579: [clangd][ObjC] Support nullability annotations.
Oct 19 2020, 9:01 AM · Restricted Project
dgoldman added a comment to D89579: [clangd][ObjC] Support nullability annotations.

With that change, somehow this extract test is now failing, not sure why or if it's intended behavior (seems like it isn't)

Oct 19 2020, 6:45 AM · Restricted Project
dgoldman updated the diff for D89579: [clangd][ObjC] Support nullability annotations.

Update with changes requested

Oct 19 2020, 6:41 AM · Restricted Project

Oct 16 2020

dgoldman updated the diff for D89579: [clangd][ObjC] Support nullability annotations.

Fix merge

Oct 16 2020, 2:30 PM · Restricted Project
dgoldman updated the diff for D89579: [clangd][ObjC] Support nullability annotations.

Lint fixes

Oct 16 2020, 2:26 PM · Restricted Project
dgoldman added inline comments to D89579: [clangd][ObjC] Support nullability annotations.
Oct 16 2020, 11:42 AM · Restricted Project
dgoldman added a reviewer for D89579: [clangd][ObjC] Support nullability annotations: sammccall.
Oct 16 2020, 11:41 AM · Restricted Project
dgoldman requested review of D89579: [clangd][ObjC] Support nullability annotations.
Oct 16 2020, 11:40 AM · Restricted Project

Aug 11 2020

dgoldman committed rGcb29c33984bf: [clangd][ObjC] Improve xrefs for protocols and classes (authored by dgoldman).
[clangd][ObjC] Improve xrefs for protocols and classes
Aug 11 2020, 9:37 AM
dgoldman closed D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
Aug 11 2020, 9:37 AM · Restricted Project
dgoldman added inline comments to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
Aug 11 2020, 9:35 AM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
  • Final touches
Aug 11 2020, 9:34 AM · Restricted Project

Aug 10 2020

dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Rebase + lint fix

Aug 10 2020, 10:14 AM · Restricted Project

Jul 27 2020

dgoldman added a comment to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

(Sorry this has been pending a while - I think it's basically there. Only things we really need to address to land this is have a consistent view of what the canonical decl is for the no-@interface case, and avoid too much duplication of mechanisms in the tests)

Jul 27 2020, 8:33 AM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
  • More fixes for review
Jul 27 2020, 8:33 AM · Restricted Project

Jul 24 2020

dgoldman committed rG3c1fca803bc1: Fix issue in typo handling which could lead clang to hang (authored by dgoldman).
Fix issue in typo handling which could lead clang to hang
Jul 24 2020, 2:36 PM

Jul 23 2020

dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Rebase

Jul 23 2020, 6:22 AM · Restricted Project

Jul 20 2020

dgoldman committed rGdde98c82c0ad: Fix issue in typo handling which could lead clang to hang (authored by dgoldman).
Fix issue in typo handling which could lead clang to hang
Jul 20 2020, 8:42 AM
dgoldman closed D84067: Fix issue in typo handling which could lead clang to hang.
Jul 20 2020, 8:42 AM · Restricted Project

Jul 17 2020

dgoldman updated the diff for D84067: Fix issue in typo handling which could lead clang to hang.

Rebase

Jul 17 2020, 1:27 PM · Restricted Project
dgoldman updated the diff for D84067: Fix issue in typo handling which could lead clang to hang.
  • CheckAndAdvanceTypoExprCorrectionStreams comment fix
Jul 17 2020, 12:55 PM · Restricted Project
dgoldman updated the diff for D84067: Fix issue in typo handling which could lead clang to hang.
  • Minor comment fix
Jul 17 2020, 12:10 PM · Restricted Project
Herald added a project to D84067: Fix issue in typo handling which could lead clang to hang: Restricted Project.
Jul 17 2020, 12:09 PM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
  • Minor test and formatting fixes
Jul 17 2020, 6:22 AM · Restricted Project
dgoldman added a comment to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

I implemented goto-definition from Foo() --> Foo impl in Xrefs, on the symbolcollector side I don't think there's anything to do?

This can't be done purely in xrefs as the AST may not be available.

A.m: @class Foo; ^Foo x; <-- go-to-definition at ^
B.m: @interface Foo...@end @implementation Foo...@end

The index needs to know that for the USR associated with the @class (found by targetDecl), the canonical decl is the @interface in B and the definition is the @implementation in B.
So SymbolCollector needs to see it as a definition. The tests seem to show it does already, but it's not obvious why from the code. Do you know? Maybe it's the fact that they share a USR and thus a symbol ID. This is worth making explicit somewhere.

Think we're talking about different things. See ObjCClassExtensions in SymbolCollectorTests which I added, the idea is that the Cat () can link to the implementation like I added to XRefs, but I'm not sure how this should be represented. As for @class -> @interface/@implementation those should have the same USR, yes.

Oh, sorry I indeed missed the parens.
Yes, this is a go-to-def special case, SymbolCollector shouldn't need anything. (I'm assuming that a reference to the class is reported already - could add a test that the ref exists if you like).

Jul 17 2020, 6:21 AM · Restricted Project

Jul 15 2020

dgoldman added inline comments to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
Jul 15 2020, 2:34 PM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

SymbolCollector fixes + more tests

Jul 15 2020, 2:32 PM · Restricted Project
dgoldman added a comment to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

I implemented goto-definition from Foo() --> Foo impl in Xrefs, on the symbolcollector side I don't think there's anything to do?

This can't be done purely in xrefs as the AST may not be available.

A.m: @class Foo; ^Foo x; <-- go-to-definition at ^
B.m: @interface Foo...@end @implementation Foo...@end

The index needs to know that for the USR associated with the @class (found by targetDecl), the canonical decl is the @interface in B and the definition is the @implementation in B.
So SymbolCollector needs to see it as a definition. The tests seem to show it does already, but it's not obvious why from the code. Do you know? Maybe it's the fact that they share a USR and thus a symbol ID. This is worth making explicit somewhere.

Jul 15 2020, 2:00 PM · Restricted Project

Jul 14 2020

dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Minor lint fixes

Jul 14 2020, 6:51 AM · Restricted Project

Jul 13 2020

dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Rebase

Jul 13 2020, 3:58 PM · Restricted Project
dgoldman added a comment to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Looking further into the indexing support, I've questions:

  • How targetDecl should behave for these objc container types

(Hmm, targetDecl doesn't seem to canonicalize decls anywhere, which might be a bug, but it's unrelated to objc)

The main thing is that AIUI @implementation is one thing, as far as clang's concerned, while @class+@interface are another.
i.e. getCanonicalDecl() and USR generation both split them into equivalence classes {@implementation} and {@class, @interface}. Is that right?

I believe they share the same USR but besides that, yes that's correct.

Jul 13 2020, 3:56 PM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Fixes for ObjC in SymbolCollector as well as XRefs

Jul 13 2020, 3:09 PM · Restricted Project
dgoldman added inline comments to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
Jul 13 2020, 9:29 AM · Restricted Project

Jul 10 2020

dgoldman committed rGea201e83e292: [AST][ObjC] Fix crash when printing invalid objc categories (authored by dgoldman).
[AST][ObjC] Fix crash when printing invalid objc categories
Jul 10 2020, 12:37 PM
dgoldman closed D83513: [AST][ObjC] Fix crash when printing invalid objc categories.
Jul 10 2020, 12:36 PM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

rebase, phabricator keeps getting confused

Jul 10 2020, 11:02 AM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Move up null check

Jul 10 2020, 10:58 AM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Find target fixes + symbol collector test

Jul 10 2020, 10:47 AM · Restricted Project
dgoldman added inline comments to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
Jul 10 2020, 10:12 AM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Swap to getPreferredDecl and improve targetDecl

Jul 10 2020, 10:09 AM · Restricted Project
dgoldman added a comment to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

I think without index changes this will still give the wrong answer for go-to-definition if the @implementation is in a different file.
Do you want to include those too or will that work ok in a separate patch?
(I'm not 100% sure of the interactions here, possible it'll seem a bit glitchy)

Jul 10 2020, 8:43 AM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Add support for categories + fix tests

Jul 10 2020, 8:27 AM · Restricted Project
dgoldman updated the diff for D83501: [clangd][ObjC] Improve xrefs for protocols and classes.

Fixes for getDefinition/getCanonicalDecl for ObjC

Jul 10 2020, 7:06 AM · Restricted Project
dgoldman added inline comments to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
Jul 10 2020, 6:43 AM · Restricted Project

Jul 9 2020

Herald added a project to D83513: [AST][ObjC] Fix crash when printing invalid objc categories: Restricted Project.
Jul 9 2020, 2:37 PM · Restricted Project
dgoldman added inline comments to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
Jul 9 2020, 12:31 PM · Restricted Project
dgoldman added inline comments to D83501: [clangd][ObjC] Improve xrefs for protocols and classes.
Jul 9 2020, 12:04 PM · Restricted Project
Herald added a project to D83501: [clangd][ObjC] Improve xrefs for protocols and classes: Restricted Project.
Jul 9 2020, 12:01 PM · Restricted Project

Jun 25 2020

dgoldman committed rGc61ef1f25c7f: [Sema][CodeComplete][ObjC] Don't split the first selector fragment (authored by dgoldman).
[Sema][CodeComplete][ObjC] Don't split the first selector fragment
Jun 25 2020, 11:21 AM
dgoldman closed D82306: [Sema][CodeComplete][ObjC] Don't split the first selector fragment.
Jun 25 2020, 11:21 AM · Restricted Project

Jun 22 2020

dgoldman created D82306: [Sema][CodeComplete][ObjC] Don't split the first selector fragment.
Jun 22 2020, 8:35 AM · Restricted Project

Jun 8 2020

dgoldman committed rG2ef65adb6f9d: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits (authored by dgoldman).
[Sema][CodeComplete][ObjC] Don't include arrow/dot fixits
Jun 8 2020, 9:57 AM
dgoldman closed D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits.
Jun 8 2020, 9:57 AM · Restricted Project
dgoldman updated the diff for D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits.
  • Fix typo
Jun 8 2020, 9:56 AM · Restricted Project
dgoldman added inline comments to D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits.
Jun 8 2020, 9:56 AM · Restricted Project
dgoldman updated the diff for D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits.
  • Minor comment fixes
Jun 8 2020, 9:56 AM · Restricted Project
dgoldman added inline comments to D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits.
Jun 8 2020, 6:30 AM · Restricted Project
dgoldman updated the diff for D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits.

Remove stale comment

Jun 8 2020, 5:59 AM · Restricted Project
dgoldman updated the diff for D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits.

Check AccessOpFixIt.hasValue()

Jun 8 2020, 5:58 AM · Restricted Project

Jun 5 2020

dgoldman updated the diff for D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits.
  • Fix broken diff base (due to lint fixes maybe?)
Jun 5 2020, 8:52 AM · Restricted Project