Page MenuHomePhabricator

dgoldman (David Goldman)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 26 2018, 1:58 PM (187 w, 7 h)

Recent Activity

Fri, May 20

dgoldman added inline comments to D124486: [clangd] ExtractVariable support for C and Objective-C.
Fri, May 20, 12:39 PM · Restricted Project, Restricted Project
dgoldman added inline comments to D124486: [clangd] ExtractVariable support for C and Objective-C.
Fri, May 20, 9:33 AM · Restricted Project, Restricted Project
dgoldman added inline comments to D124486: [clangd] ExtractVariable support for C and Objective-C.
Fri, May 20, 9:03 AM · Restricted Project, Restricted Project
dgoldman committed rG322e2a3b40fa: [clangd][ObjC] Filter ObjC method completions on the remaining selector (authored by dgoldman).
[clangd][ObjC] Filter ObjC method completions on the remaining selector
Fri, May 20, 8:50 AM · Restricted Project, Restricted Project, Restricted Project
dgoldman closed D124637: [clangd][ObjC] Filter ObjC method completions on the remaining selector.
Fri, May 20, 8:50 AM · Restricted Project, Restricted Project, Restricted Project
dgoldman updated the diff for D124637: [clangd][ObjC] Filter ObjC method completions on the remaining selector.

Go through chunks in getName and add missed FilterText place

Fri, May 20, 7:54 AM · Restricted Project, Restricted Project, Restricted Project

Wed, May 18

dgoldman added a reviewer for D124637: [clangd][ObjC] Filter ObjC method completions on the remaining selector: kadircet.
Wed, May 18, 10:05 AM · Restricted Project, Restricted Project, Restricted Project

Thu, May 12

dgoldman committed rGe91a73de24d6: [Lit] Add pushd and popd builtins (authored by dgoldman).
[Lit] Add pushd and popd builtins
Thu, May 12, 4:25 PM · Restricted Project, Restricted Project
dgoldman closed D125502: [Lit] Add pushd and popd builtins.
Thu, May 12, 4:25 PM · Restricted Project, Restricted Project
dgoldman updated the diff for D125502: [Lit] Add pushd and popd builtins.

Update commit message

Thu, May 12, 4:20 PM · Restricted Project, Restricted Project
dgoldman requested review of D125502: [Lit] Add pushd and popd builtins.
Thu, May 12, 1:34 PM · Restricted Project, Restricted Project

Fri, May 6

dgoldman added a comment to D124486: [clangd] ExtractVariable support for C and Objective-C.

Friendly ping

Fri, May 6, 7:30 AM · Restricted Project, Restricted Project
dgoldman added a comment to D124637: [clangd][ObjC] Filter ObjC method completions on the remaining selector.

Friendly ping

Fri, May 6, 7:30 AM · Restricted Project, Restricted Project, Restricted Project

Wed, May 4

dgoldman added a comment to D124715: Add ThreadPriority::Low, and use QoS class Utility on Mac.

(How) does this interact with battery vs mains power on laptops?
It seems like there's a common scenario where:

  • the user is on a relatively slow laptop, running off battery
  • the codebase is large, and indexing is unlikely to finish within an editing session

In this case, it seems like only using efficiency cores is what you'd want, and that people are likely to be upset if clangd 15 keeps their performance cores running at all times.

I can only speak for myself here: I'm a laptop user myself, and I work on battery the majority of the day; but I still wouldn't trade indexing speed for saving battery. The clangd index is so essential for my work that I always want it to be available as quickly as possible.

Reading the docs it seems like background is the intended QoS for this type of work ("such as indexing"..."minutes or hours").

How long it takes is only one aspect of it. Other clues from the documentation:

Utility: "typically have a progress bar that is visible to the user. Focuses on providing a balance between responsiveness, performance, and energy efficiency."
Background: "isn't visible to the user. Focuses on energy efficiency."

The way I read it, Background is intended for tasks that don't make a significant difference to the functionality of the software. An example might be face detection in the Photos app; as long as it is not finished, it only affects a small part of the functionality, the rest of the app is perfectly usable. That's not the case for clangd, the software is pretty much not usable without an index.

Wed, May 4, 11:01 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Tue, May 3

dgoldman accepted D124715: Add ThreadPriority::Low, and use QoS class Utility on Mac.

Fix documentation of ThreadPriority::Low (was: Background), and move it up from
set_thread_priority to ThreadPriority.

(Is it ok that there's no documentation left for set_thread_priority? I couldn't
come up with anything that's not trivial and redundant, e.g. "Sets the priority
of the current thread.")

Tue, May 3, 7:01 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Sat, Apr 30

dgoldman added inline comments to D124715: Add ThreadPriority::Low, and use QoS class Utility on Mac.
Sat, Apr 30, 8:43 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Thu, Apr 28

dgoldman added inline comments to D124486: [clangd] ExtractVariable support for C and Objective-C.
Thu, Apr 28, 3:32 PM · Restricted Project, Restricted Project
dgoldman updated the diff for D124486: [clangd] ExtractVariable support for C and Objective-C.
  • Extract var type handling to a function
  • Other fixes for review
Thu, Apr 28, 3:23 PM · Restricted Project, Restricted Project
dgoldman requested review of D124637: [clangd][ObjC] Filter ObjC method completions on the remaining selector.
Thu, Apr 28, 1:26 PM · Restricted Project, Restricted Project, Restricted Project

Apr 27 2022

dgoldman updated the diff for D124486: [clangd] ExtractVariable support for C and Objective-C.
  • Remove extraneous computeExprType declaration
Apr 27 2022, 4:51 PM · Restricted Project, Restricted Project
dgoldman added inline comments to D124486: [clangd] ExtractVariable support for C and Objective-C.
Apr 27 2022, 4:38 PM · Restricted Project, Restricted Project
dgoldman updated the diff for D124486: [clangd] ExtractVariable support for C and Objective-C.
  • Don't use a method for computing the type
  • Compute the type using printType
  • Support MemberExprs and add a test for function pointers
Apr 27 2022, 4:37 PM · Restricted Project, Restricted Project
dgoldman added reviewers for D124486: [clangd] ExtractVariable support for C and Objective-C: sammccall, kadircet.
Apr 27 2022, 7:05 AM · Restricted Project, Restricted Project

Apr 26 2022

dgoldman updated the diff for D124486: [clangd] ExtractVariable support for C and Objective-C.

Minor formatting fix

Apr 26 2022, 5:19 PM · Restricted Project, Restricted Project
dgoldman requested review of D124486: [clangd] ExtractVariable support for C and Objective-C.
Apr 26 2022, 4:46 PM · Restricted Project, Restricted Project

Mar 31 2022

dgoldman committed rGd9739f29cdd4: Serialize PragmaAssumeNonNullLoc to support preambles (authored by dgoldman).
Serialize PragmaAssumeNonNullLoc to support preambles
Mar 31 2022, 8:08 AM · Restricted Project, Restricted Project, Restricted Project
dgoldman closed D122179: Serialize PragmaAssumeNonNullLoc to support preambles.
Mar 31 2022, 8:08 AM · Restricted Project, Restricted Project, Restricted Project

Mar 29 2022

dgoldman updated the diff for D122179: Serialize PragmaAssumeNonNullLoc to support preambles.

Rebase

Mar 29 2022, 12:21 PM · Restricted Project, Restricted Project, Restricted Project

Mar 25 2022

dgoldman updated the diff for D122179: Serialize PragmaAssumeNonNullLoc to support preambles.

Revert comment change

Mar 25 2022, 12:43 PM · Restricted Project, Restricted Project, Restricted Project

Mar 24 2022

dgoldman updated the diff for D122179: Serialize PragmaAssumeNonNullLoc to support preambles.

Another formatting fix

Mar 24 2022, 1:19 PM · Restricted Project, Restricted Project, Restricted Project
dgoldman updated the diff for D122179: Serialize PragmaAssumeNonNullLoc to support preambles.

if/else formatting

Mar 24 2022, 11:51 AM · Restricted Project, Restricted Project, Restricted Project
dgoldman updated the diff for D122179: Serialize PragmaAssumeNonNullLoc to support preambles.

Minor lint fixes

Mar 24 2022, 11:47 AM · Restricted Project, Restricted Project, Restricted Project
dgoldman added inline comments to D122179: Serialize PragmaAssumeNonNullLoc to support preambles.
Mar 24 2022, 11:00 AM · Restricted Project, Restricted Project, Restricted Project
dgoldman updated the diff for D122179: Serialize PragmaAssumeNonNullLoc to support preambles.

Properly handle generation and restoring

Mar 24 2022, 10:25 AM · Restricted Project, Restricted Project, Restricted Project

Mar 23 2022

dgoldman added inline comments to D122179: Serialize PragmaAssumeNonNullLoc to support preambles.
Mar 23 2022, 1:00 PM · Restricted Project, Restricted Project, Restricted Project
dgoldman added inline comments to D122179: Serialize PragmaAssumeNonNullLoc to support preambles.
Mar 23 2022, 8:17 AM · Restricted Project, Restricted Project, Restricted Project
dgoldman updated the diff for D122179: Serialize PragmaAssumeNonNullLoc to support preambles.

Minor fixes

Mar 23 2022, 8:08 AM · Restricted Project, Restricted Project, Restricted Project

Mar 22 2022

dgoldman added inline comments to D122179: Serialize PragmaAssumeNonNullLoc to support preambles.
Mar 22 2022, 9:08 AM · Restricted Project, Restricted Project, Restricted Project
dgoldman updated the diff for D122179: Serialize PragmaAssumeNonNullLoc to support preambles.

Fixes for review

Mar 22 2022, 9:07 AM · Restricted Project, Restricted Project, Restricted Project

Mar 21 2022

dgoldman added a comment to D122179: Serialize PragmaAssumeNonNullLoc to support preambles.

LMK if there are other folks more suited to review this, I'm not sure...

Mar 21 2022, 2:37 PM · Restricted Project, Restricted Project, Restricted Project
dgoldman requested review of D122179: Serialize PragmaAssumeNonNullLoc to support preambles.
Mar 21 2022, 1:50 PM · Restricted Project, Restricted Project, Restricted Project

Mar 17 2022

dgoldman committed rG8522a01e842e: Attempt forward fix for Linux buildbots for D116385 (authored by dgoldman).
Attempt forward fix for Linux buildbots for D116385
Mar 17 2022, 9:50 AM · Restricted Project, Restricted Project
dgoldman committed rG1af5fbd5c605: [clangd] Code action for creating an ObjC initializer (authored by dgoldman).
[clangd] Code action for creating an ObjC initializer
Mar 17 2022, 8:32 AM · Restricted Project, Restricted Project
dgoldman closed D116385: [clangd] Code action for creating an ObjC initializer.
Mar 17 2022, 8:32 AM · Restricted Project, Restricted Project
dgoldman updated the diff for D116385: [clangd] Code action for creating an ObjC initializer.

Fixes for review

Mar 17 2022, 8:23 AM · Restricted Project, Restricted Project

Mar 2 2022

Herald added a project to D116385: [clangd] Code action for creating an ObjC initializer: Restricted Project.

friendly ping

Mar 2 2022, 3:04 PM · Restricted Project, Restricted Project

Feb 18 2022

dgoldman committed rG54a962bbfee8: [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support (authored by dgoldman).
[clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support
Feb 18 2022, 12:25 PM
dgoldman committed rG805f7a4fa4ce: [clang] Add `ObjCProtocolLoc` to represent protocol references (authored by dgoldman).
[clang] Add `ObjCProtocolLoc` to represent protocol references
Feb 18 2022, 12:25 PM
dgoldman closed D119366: [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support.
Feb 18 2022, 12:25 PM · Restricted Project
dgoldman closed D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.
Feb 18 2022, 12:25 PM · Restricted Project

Feb 15 2022

dgoldman added a comment to D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.

I'm a bit concerned about the parent map vs ASTMatchFinder's view being out of sync: we'll have a ProtocolLoc node that has a parent but the parent doesn't have the child.

I'm not sure this breaks anything immediately, but I think we should also make these nodes visible to matchers, even if there's no specific matcher yet.

Hmm I can try to do it - what do I need to modify to make them visible to matchers?

I don't remember specifically, I think ASTMatchFinderImpl has a RecursiveASTVisitor that you need to extend? I'm not sure actually how you would observe these nodes cleanly without matchers (hacks like has(hasParent()) maybe?) So maybe it's best to ignore it in this patch and add basic matchers in a next one

Feb 15 2022, 12:28 PM · Restricted Project
dgoldman updated the diff for D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.

Template fixes

Feb 15 2022, 12:19 PM · Restricted Project
dgoldman added a comment to D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.

I'm a bit concerned about the parent map vs ASTMatchFinder's view being out of sync: we'll have a ProtocolLoc node that has a parent but the parent doesn't have the child.

I'm not sure this breaks anything immediately, but I think we should also make these nodes visible to matchers, even if there's no specific matcher yet.

Feb 15 2022, 11:08 AM · Restricted Project
dgoldman updated the diff for D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.

Template and variable casing fixes

Feb 15 2022, 11:08 AM · Restricted Project
dgoldman added a comment to D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.

PTAL, thanks!

Feb 15 2022, 9:54 AM · Restricted Project
dgoldman updated the diff for D119366: [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support.

Rebase on top of latest ProtocolLoc changes

Feb 15 2022, 9:47 AM · Restricted Project
dgoldman added inline comments to D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.
Feb 15 2022, 9:37 AM · Restricted Project
dgoldman updated the diff for D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.

Add tests and public accessors

Feb 15 2022, 9:36 AM · Restricted Project

Feb 10 2022

dgoldman updated the diff for D119366: [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support.

add hover test

Feb 10 2022, 1:56 PM · Restricted Project
dgoldman added inline comments to D119366: [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support.
Feb 10 2022, 1:45 PM · Restricted Project
dgoldman added a reviewer for D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references: sammccall.
Feb 10 2022, 7:00 AM · Restricted Project

Feb 9 2022

dgoldman updated the summary of D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.
Feb 9 2022, 2:26 PM · Restricted Project
dgoldman updated the diff for D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.

Minor lint fixes

Feb 9 2022, 2:21 PM · Restricted Project
dgoldman updated the diff for D119366: [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support.

Minor lint fixes

Feb 9 2022, 2:04 PM · Restricted Project
dgoldman added a reviewer for D119366: [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support: sammccall.
Feb 9 2022, 1:59 PM · Restricted Project
dgoldman requested review of D119366: [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support.
Feb 9 2022, 12:30 PM · Restricted Project
dgoldman requested review of D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references.
Feb 9 2022, 11:58 AM · Restricted Project

Feb 7 2022

dgoldman committed rGf98bf92b6241: Reland "[clangd] Properly compute framework-style include spelling" (authored by dgoldman).
Reland "[clangd] Properly compute framework-style include spelling"
Feb 7 2022, 8:21 AM

Feb 4 2022

dgoldman added a reverting change for rG4dfd11324eb0: [clangd] Properly compute framework-style include spelling: rGfb7ddd0628f4: Revert "[clangd] Properly compute framework-style include spelling".
Feb 4 2022, 3:03 PM
dgoldman committed rGfb7ddd0628f4: Revert "[clangd] Properly compute framework-style include spelling" (authored by dgoldman).
Revert "[clangd] Properly compute framework-style include spelling"
Feb 4 2022, 3:03 PM
dgoldman added a reverting change for D117056: [clangd] Properly compute framework-style include spelling: rGfb7ddd0628f4: Revert "[clangd] Properly compute framework-style include spelling".
Feb 4 2022, 3:03 PM · Restricted Project, Restricted Project
dgoldman committed rG6abb70c2d008: Attempt forward fix after 4dfd113 (authored by dgoldman).
Attempt forward fix after 4dfd113
Feb 4 2022, 2:50 PM
dgoldman committed rG4dfd11324eb0: [clangd] Properly compute framework-style include spelling (authored by dgoldman).
[clangd] Properly compute framework-style include spelling
Feb 4 2022, 1:42 PM
dgoldman closed D117056: [clangd] Properly compute framework-style include spelling.
Feb 4 2022, 1:41 PM · Restricted Project, Restricted Project
dgoldman updated the diff for D117056: [clangd] Properly compute framework-style include spelling.

Rebase

Feb 4 2022, 12:52 PM · Restricted Project, Restricted Project
dgoldman committed rG9385ece95a4a: [HeaderSearch] Track framework name in LookupFile (authored by dgoldman).
[HeaderSearch] Track framework name in LookupFile
Feb 4 2022, 10:33 AM
dgoldman closed D117830: [HeaderSearch] Track framework name in LookupFile.
Feb 4 2022, 10:33 AM · Restricted Project

Feb 3 2022

dgoldman updated the diff for D116385: [clangd] Code action for creating an ObjC initializer.

mergingEdit --> manual file checks

Feb 3 2022, 1:19 PM · Restricted Project, Restricted Project

Jan 26 2022

dgoldman updated the diff for D117056: [clangd] Properly compute framework-style include spelling.

Formatting fixes

Jan 26 2022, 11:50 AM · Restricted Project, Restricted Project
dgoldman added inline comments to D117830: [HeaderSearch] Track framework name in LookupFile.
Jan 26 2022, 11:02 AM · Restricted Project
dgoldman updated the diff for D117830: [HeaderSearch] Track framework name in LookupFile.

Update comment + run clang-format

Jan 26 2022, 11:01 AM · Restricted Project

Jan 24 2022

dgoldman added a comment to D117830: [HeaderSearch] Track framework name in LookupFile.

My understanding for the header map restriction is because headermaps are generally emitted once for a framework build and only consumed to build such framework and this struct information is only queried during this.
It sounds like for the clangd support, you're not as interested in what framework is being built, but the name of the framework when included via framework style in any context. Is that correct? It seems fine to extend its use cases. @jansvoboda11 What do you think?

Jan 24 2022, 1:38 PM · Restricted Project
dgoldman requested review of D118063: [DONOTSUBMIT] work on prototyping bracket fix its.
Jan 24 2022, 12:03 PM · Restricted Project, Restricted Project

Jan 21 2022

dgoldman added a comment to D117830: [HeaderSearch] Track framework name in LookupFile.

Cyndy and Jan, hopefully ya'll are the right reviewers here? We're using this Framework information to know to compute framework style includes in https://reviews.llvm.org/D117056 for clangd, please LMK if this is a bad idea and we should do something else.

Jan 21 2022, 1:49 PM · Restricted Project
dgoldman added reviewers for D117830: [HeaderSearch] Track framework name in LookupFile: cishida, jansvoboda11.
Jan 21 2022, 1:47 PM · Restricted Project

Jan 20 2022

dgoldman requested review of D117830: [HeaderSearch] Track framework name in LookupFile.
Jan 20 2022, 12:33 PM · Restricted Project
dgoldman updated the diff for D117056: [clangd] Properly compute framework-style include spelling.

Split up HeaderSearch change

Jan 20 2022, 12:31 PM · Restricted Project, Restricted Project
dgoldman added a comment to D117056: [clangd] Properly compute framework-style include spelling.

This looks good now, only blocker is dropping the change to LookupFile somehow.

Is it valuable to land this before the imminent 14 branch cut? I think it's OK but as an indexing change a crash has the possibility to be really disruptive, and most of the rest of us aren't regularly dogfooding this on Mac.
I'd suggest we only land it before the branch if you feel like you can test this on a reasonable variety of code in the next few weeks. WDYT?

Jan 20 2022, 9:39 AM · Restricted Project, Restricted Project
dgoldman added inline comments to D117056: [clangd] Properly compute framework-style include spelling.
Jan 20 2022, 9:38 AM · Restricted Project, Restricted Project
dgoldman updated the diff for D117056: [clangd] Properly compute framework-style include spelling.

Support private headers + private umbrellas

Jan 20 2022, 9:37 AM · Restricted Project, Restricted Project

Jan 19 2022

dgoldman added a comment to D116385: [clangd] Code action for creating an ObjC initializer.

PTAL, let me know what you think regarding the edits, had to make sure it works even if we generate multiple edits in the same file.

Jan 19 2022, 3:55 PM · Restricted Project, Restricted Project
dgoldman added inline comments to D117056: [clangd] Properly compute framework-style include spelling.
Jan 19 2022, 3:55 PM · Restricted Project, Restricted Project
dgoldman added inline comments to D117056: [clangd] Properly compute framework-style include spelling.
Jan 19 2022, 3:33 PM · Restricted Project, Restricted Project
dgoldman updated the diff for D117056: [clangd] Properly compute framework-style include spelling.

Changes for review

Jan 19 2022, 3:32 PM · Restricted Project, Restricted Project

Jan 12 2022

dgoldman updated the diff for D117056: [clangd] Properly compute framework-style include spelling.

Run clang-format

Jan 12 2022, 8:12 AM · Restricted Project, Restricted Project

Jan 11 2022

dgoldman requested review of D117056: [clangd] Properly compute framework-style include spelling.
Jan 11 2022, 2:38 PM · Restricted Project, Restricted Project

Jan 10 2022

dgoldman committed rG0cf7e61a42c7: [clang][HeaderSearch] Support framework includes in suggestPath... (authored by dgoldman).
[clang][HeaderSearch] Support framework includes in suggestPath...
Jan 10 2022, 9:28 AM
dgoldman closed D115183: [clang][HeaderSearch] Support framework includes in suggestPath....
Jan 10 2022, 9:28 AM · Restricted Project

Jan 7 2022

dgoldman updated the diff for D115183: [clang][HeaderSearch] Support framework includes in suggestPath....

Don't suggest umbrella headers

Jan 7 2022, 9:30 AM · Restricted Project

Jan 6 2022

dgoldman added inline comments to D116385: [clangd] Code action for creating an ObjC initializer.
Jan 6 2022, 12:01 PM · Restricted Project, Restricted Project