Page MenuHomePhabricator

sammccall (Sam McCall)Administrator
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 26 2016, 6:53 AM (119 w, 6 d)
Roles
Administrator

Recent Activity

Tue, Nov 27

sammccall added a comment to D50147: clang-format: support external styles.

ping?

Tue, Nov 27, 11:09 AM
sammccall added a comment to D54945: This commit adds a chapter about clang-tidy integrations.

I like the overview, maybe a link to clangd here might help, as there is currently a lot of effort of integrating clang-tidy into it. (@sammccall WDYT?)

Tue, Nov 27, 9:32 AM · Restricted Project
sammccall added inline comments to D54737: [clang-tidy] Add the abseil-duration-comparison check.
Tue, Nov 27, 7:58 AM · Restricted Project
sammccall added inline comments to D54737: [clang-tidy] Add the abseil-duration-comparison check.
Tue, Nov 27, 7:24 AM · Restricted Project
sammccall added inline comments to D54737: [clang-tidy] Add the abseil-duration-comparison check.
Tue, Nov 27, 7:09 AM · Restricted Project
sammccall accepted D54817: [clangd] Put direct headers into srcs section..

Only really significant thing is I think the name "build graph" is confusing, and we should specifically mention includes.
Rest is just nits to take or leave...

Tue, Nov 27, 7:03 AM
sammccall accepted D54845: [clangd] Canonicalize file path in URIForFile..

Thanks!
Mostly just places that should be updated HintPath -> TUPath. Changing the whole codebase doesn't seem important, but in places that are touched...

Tue, Nov 27, 6:22 AM
sammccall added a comment to D54817: [clangd] Put direct headers into srcs section..

To summarize offline discussion: I think we need to split this patch up, as the scope is growing. (In a really useful direction I think!)

Tue, Nov 27, 5:37 AM
sammccall accepted D54799: [clangd] textDocument/SymbolInfo method.

Tests look great, thanks!

Tue, Nov 27, 4:55 AM
sammccall added a comment to D54817: [clangd] Put direct headers into srcs section..

structure and serialization stuff looks great!
still some questions around the auto-index logic and organization of the extraction code, let's chat offline. May make sense to split.

Tue, Nov 27, 4:53 AM
sammccall committed rL347655: [clangd] Prevent thread starvation in tests on loaded systems..
[clangd] Prevent thread starvation in tests on loaded systems.
Tue, Nov 27, 4:12 AM
sammccall committed rCTE347655: [clangd] Prevent thread starvation in tests on loaded systems..
[clangd] Prevent thread starvation in tests on loaded systems.
Tue, Nov 27, 4:12 AM
sammccall closed D54938: [clangd] Prevent thread starvation in tests on loaded systems..
Tue, Nov 27, 4:12 AM
sammccall created D54938: [clangd] Prevent thread starvation in tests on loaded systems..
Tue, Nov 27, 2:00 AM
sammccall added inline comments to D54796: [clangd] C++ API for emitting file status.
Tue, Nov 27, 1:14 AM

Mon, Nov 26

sammccall added inline comments to D54845: [clangd] Canonicalize file path in URIForFile..
Mon, Nov 26, 9:54 AM
sammccall committed rCTE347567: [clangd] Enable auto-index behind a flag..
[clangd] Enable auto-index behind a flag.
Mon, Nov 26, 8:03 AM
sammccall committed rL347567: [clangd] Enable auto-index behind a flag..
[clangd] Enable auto-index behind a flag.
Mon, Nov 26, 8:03 AM
sammccall closed D54894: [clangd] Enable auto-index behind a flag..
Mon, Nov 26, 8:02 AM
sammccall added inline comments to D54894: [clangd] Enable auto-index behind a flag..
Mon, Nov 26, 7:39 AM
sammccall updated the diff for D54894: [clangd] Enable auto-index behind a flag..

Make blockUntilIdleForTest() accept a timeout, update comment.

Mon, Nov 26, 7:04 AM
sammccall accepted D52276: [clangd] Add type boosting in code completion.

You may want to add a FIXME in SymbolIndex to include opaque type in fuzzyfind request.

Mon, Nov 26, 6:42 AM
sammccall accepted D52274: [clangd] Collect and store expected types in the index.
Mon, Nov 26, 6:37 AM
sammccall created D54894: [clangd] Enable auto-index behind a flag..
Mon, Nov 26, 6:09 AM
sammccall committed rCTE347554: [clangd] Fix missing include from r347538 - fix windows buildbots.
[clangd] Fix missing include from r347538 - fix windows buildbots
Mon, Nov 26, 5:38 AM
sammccall committed rL347554: [clangd] Fix missing include from r347538 - fix windows buildbots.
[clangd] Fix missing include from r347538 - fix windows buildbots
Mon, Nov 26, 5:38 AM
sammccall accepted D54799: [clangd] textDocument/SymbolInfo method.

LG from my side.
If you have unit tests in the next couple of days, happy to take a look, otherwise go ahead once Alex/Ben are happy.

Mon, Nov 26, 5:24 AM
sammccall committed rCTE347538: [clangd] Auto-index watches global CDB for changes..
[clangd] Auto-index watches global CDB for changes.
Mon, Nov 26, 1:55 AM
sammccall committed rL347538: [clangd] Auto-index watches global CDB for changes..
[clangd] Auto-index watches global CDB for changes.
Mon, Nov 26, 1:54 AM
sammccall closed D54865: [clangd] Auto-index watches global CDB for changes..
Mon, Nov 26, 1:54 AM
sammccall updated the diff for D54865: [clangd] Auto-index watches global CDB for changes..

If the CDB dir is unknown, don't try to write shards to disk.

Mon, Nov 26, 1:44 AM
sammccall added inline comments to D54865: [clangd] Auto-index watches global CDB for changes..
Mon, Nov 26, 1:43 AM

Fri, Nov 23

sammccall added a comment to D54799: [clangd] textDocument/SymbolInfo method.

Thanks, this looks good! Just nits, and please do port most of the test cases to unit tests.

Fri, Nov 23, 1:37 PM
sammccall created D54865: [clangd] Auto-index watches global CDB for changes..
Fri, Nov 23, 12:38 PM
sammccall added a comment to D54781: [clangd] Add 'Switch header/source' command in clangd-vscode.

This is great!

Fri, Nov 23, 6:30 AM
sammccall accepted D54851: [clangd] Tune down scope boost for global scope.
Fri, Nov 23, 6:18 AM
sammccall added inline comments to D52276: [clangd] Add type boosting in code completion.
Fri, Nov 23, 6:17 AM
sammccall updated subscribers of D54799: [clangd] textDocument/SymbolInfo method.
  • conditional return in getCursorInfo - Should we return for example data with empty USR?

Please return a symbol unless it has no SymbolID (we don't treat those as symbols in clangd).

Ok.

Sorry, this was a mistake - we do in fact use decls that don't have symbol IDs (we just don't look them up in the index).
So I think both SymbolID and USR are optional.

Fri, Nov 23, 1:01 AM

Thu, Nov 22

sammccall added inline comments to D54796: [clangd] C++ API for emitting file status.
Thu, Nov 22, 8:32 AM
sammccall accepted D54760: [clangd] Cleanup: make the diags callback global in TUScheduler.

Everything looks so much better...

Thu, Nov 22, 8:14 AM
sammccall accepted D52273: [clangd] Initial implementation of expected types.
Thu, Nov 22, 7:41 AM
sammccall accepted D54833: [clangd] Cleanup: use index file instead of header in workspace symbols lit test..
Thu, Nov 22, 6:59 AM
sammccall accepted D52311: [clangd] Add support for hierarchical documentSymbol.
Thu, Nov 22, 6:54 AM
sammccall accepted D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy.
Thu, Nov 22, 5:51 AM
sammccall added a comment to D54817: [clangd] Put direct headers into srcs section..

Going to leave awkward comments suggesting we expand the scope a bit.
Full disclosure: file dependency information is something that's come up as useful in lots of contexts.

Thu, Nov 22, 5:33 AM
sammccall added inline comments to D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy.
Thu, Nov 22, 3:29 AM
sammccall accepted D54800: [clangd] Cleanup: stop passing around list of supported URI schemes..

Awesome!

Thu, Nov 22, 2:39 AM
sammccall committed rCTE347450: [clangd] Respect task cancellation in TUScheduler..
[clangd] Respect task cancellation in TUScheduler.
Thu, Nov 22, 2:25 AM
sammccall committed rL347450: [clangd] Respect task cancellation in TUScheduler..
[clangd] Respect task cancellation in TUScheduler.
Thu, Nov 22, 2:25 AM
sammccall closed D54746: [clangd] Respect task cancellation in TUScheduler..
Thu, Nov 22, 2:25 AM
sammccall closed D54746: [clangd] Respect task cancellation in TUScheduler..
Thu, Nov 22, 2:25 AM
sammccall added a comment to D54760: [clangd] Cleanup: make the diags callback global in TUScheduler.

Just the context thing. Rest is some optional simplifications.

Thu, Nov 22, 1:42 AM

Wed, Nov 21

sammccall added a comment to D54799: [clangd] textDocument/SymbolInfo method.

Thanks for sending this! Broadly looks good, a few details to work out. I think the biggest one is multiple symbols which you've flagged.

Wed, Nov 21, 9:51 AM

Tue, Nov 20

sammccall committed rL347352: [CodeComplete] Penalize inherited ObjC properties for auto-completion.
[CodeComplete] Penalize inherited ObjC properties for auto-completion
Tue, Nov 20, 2:11 PM
sammccall committed rC347352: [CodeComplete] Penalize inherited ObjC properties for auto-completion.
[CodeComplete] Penalize inherited ObjC properties for auto-completion
Tue, Nov 20, 2:11 PM
sammccall closed D53900: [CodeComplete] Penalize inherited ObjC properties for auto-completion.
Tue, Nov 20, 2:11 PM
sammccall added a comment to D52311: [clangd] Add support for hierarchical documentSymbol.

Very nice! Mostly just a few style/structure nits.

Tue, Nov 20, 1:39 PM
sammccall abandoned D54259: [ASTMatchers] proof-of-concept: allow matching within a restricted scope..
Tue, Nov 20, 11:02 AM
sammccall abandoned D54261: [clangd] proof-of-concept: clang-tidy only runs over main-file.
Tue, Nov 20, 11:02 AM
sammccall added a comment to D54760: [clangd] Cleanup: make the diags callback global in TUScheduler.

Nice!
The biggest suggestion I have is merging the callback into ASTCallbacks. That's awkward in initialization (makeUpdateCallbacks probably needs to be called unconditionally with a maybe-null pointer) but it seems like a natural grouping in all the places it gets plumbed around to... and there are a lot!

Tue, Nov 20, 10:59 AM
sammccall added inline comments to D52276: [clangd] Add type boosting in code completion.
Tue, Nov 20, 7:35 AM
sammccall added inline comments to D52276: [clangd] Add type boosting in code completion.
Tue, Nov 20, 7:34 AM
sammccall added inline comments to D52274: [clangd] Collect and store expected types in the index.
Tue, Nov 20, 7:23 AM
sammccall added inline comments to D52273: [clangd] Initial implementation of expected types.
Tue, Nov 20, 6:43 AM
sammccall committed rL347298: [clangd] Replay preamble #includes to clang-tidy checks..
[clangd] Replay preamble #includes to clang-tidy checks.
Tue, Nov 20, 3:01 AM
sammccall committed rCTE347298: [clangd] Replay preamble #includes to clang-tidy checks..
[clangd] Replay preamble #includes to clang-tidy checks.
Tue, Nov 20, 3:01 AM
sammccall closed D54694: [clangd] Replay preamble #includes to clang-tidy checks..
Tue, Nov 20, 3:01 AM
sammccall closed D54694: [clangd] Replay preamble #includes to clang-tidy checks..
Tue, Nov 20, 3:01 AM
sammccall committed rCTE347297: [clangd] Allow observation of changes to global CDBs..
[clangd] Allow observation of changes to global CDBs.
Tue, Nov 20, 3:00 AM
sammccall committed rL347297: [clangd] Allow observation of changes to global CDBs..
[clangd] Allow observation of changes to global CDBs.
Tue, Nov 20, 3:00 AM
sammccall closed D54475: [clangd] Allow observation of changes to global CDBs..
Tue, Nov 20, 3:00 AM
sammccall created D54746: [clangd] Respect task cancellation in TUScheduler..
Tue, Nov 20, 2:53 AM

Mon, Nov 19

sammccall added a comment to rL347284: Ensure FileManagerTest expects "\\" as path separator on Windows platforms.

Thanks! Sorry for missing this.

Mon, Nov 19, 11:38 PM
sammccall added inline comments to D54475: [clangd] Allow observation of changes to global CDBs..
Mon, Nov 19, 9:59 AM
sammccall updated the diff for D54475: [clangd] Allow observation of changes to global CDBs..

Address comments, add test for Event machinery.

Mon, Nov 19, 9:52 AM
sammccall accepted D54693: [clangd] Store source file hash in IndexFile{In,Out}.
Mon, Nov 19, 9:28 AM
sammccall added inline comments to D54694: [clangd] Replay preamble #includes to clang-tidy checks..
Mon, Nov 19, 8:29 AM
sammccall updated the diff for D54694: [clangd] Replay preamble #includes to clang-tidy checks..

Address comments.

Mon, Nov 19, 8:29 AM
sammccall committed rL347205: [FileManager] getFile(open=true) after getFile(open=false) should open the file..
[FileManager] getFile(open=true) after getFile(open=false) should open the file.
Mon, Nov 19, 5:40 AM
sammccall committed rC347205: [FileManager] getFile(open=true) after getFile(open=false) should open the file..
[FileManager] getFile(open=true) after getFile(open=false) should open the file.
Mon, Nov 19, 5:40 AM
sammccall closed D54691: [FileManager] getFile(open=true) after getFile(open=false) should open the file..
Mon, Nov 19, 5:40 AM
sammccall added a comment to D54691: [FileManager] getFile(open=true) after getFile(open=false) should open the file..

Overall LG. The only concerning bit is that the initialization of the UFE is now scattered across the function body.
Grouping File and DeferredOpen into a struct or adding a comment that those are initialized separately from the rest of the fields might make it more evident what's going on. WDYT?

Mon, Nov 19, 2:42 AM
sammccall updated the diff for D54691: [FileManager] getFile(open=true) after getFile(open=false) should open the file..

Address review comments.

Mon, Nov 19, 2:42 AM
sammccall created D54694: [clangd] Replay preamble #includes to clang-tidy checks..
Mon, Nov 19, 2:32 AM
sammccall added a child revision for D54691: [FileManager] getFile(open=true) after getFile(open=false) should open the file.: D54694: [clangd] Replay preamble #includes to clang-tidy checks..
Mon, Nov 19, 2:31 AM
sammccall created D54691: [FileManager] getFile(open=true) after getFile(open=false) should open the file..
Mon, Nov 19, 1:11 AM

Fri, Nov 16

sammccall accepted D54622: [clangd] Truncate SymbolID to 8 bytes..
Fri, Nov 16, 2:50 AM
sammccall committed rCTE347036: [clangd] Initial clang-tidy diagnostics support..
[clangd] Initial clang-tidy diagnostics support.
Fri, Nov 16, 12:35 AM
sammccall committed rL347036: [clangd] Initial clang-tidy diagnostics support..
[clangd] Initial clang-tidy diagnostics support.
Fri, Nov 16, 12:35 AM
sammccall closed D54204: [clangd] Initial clang-tidy diagnostics support..
Fri, Nov 16, 12:35 AM

Thu, Nov 15

sammccall committed rCTE346961: [clang-tidy] Update checks to play nicely with limited traversal scope added in….
[clang-tidy] Update checks to play nicely with limited traversal scope added in…
Thu, Nov 15, 7:09 AM
sammccall committed rL346961: [clang-tidy] Update checks to play nicely with limited traversal scope added in….
[clang-tidy] Update checks to play nicely with limited traversal scope added in…
Thu, Nov 15, 7:09 AM
sammccall closed D54579: [clang-tidy] Update checks to play nicely with limited traversal scope added in r346847.
Thu, Nov 15, 7:08 AM
sammccall updated the diff for D54204: [clangd] Initial clang-tidy diagnostics support..

Remove clang-tidy changes, add FIXME comment.

Thu, Nov 15, 6:31 AM
sammccall added a comment to D54204: [clangd] Initial clang-tidy diagnostics support..

Moved the clang-tidy changes to D54579.
Sorry for mixing everything up!

Thu, Nov 15, 6:30 AM
sammccall updated the diff for D54579: [clang-tidy] Update checks to play nicely with limited traversal scope added in r346847.

Address comments from the last round of review in D54204.

Thu, Nov 15, 6:30 AM
sammccall created D54579: [clang-tidy] Update checks to play nicely with limited traversal scope added in r346847.
Thu, Nov 15, 6:26 AM
sammccall accepted D54269: Introduce shard storage to auto-index..

Thanks! Looks good, rest of the nits are obvious or up to you.(

Thu, Nov 15, 4:50 AM
sammccall added a comment to D53900: [CodeComplete] Penalize inherited ObjC properties for auto-completion.

As mentioned offline - sounds like someone from Apple might take a look.
If not, ping me again to land!

Thu, Nov 15, 3:08 AM
sammccall updated subscribers of D54277: Move RedirectingFileSystem interface into header..

Hi Jonas,

Thu, Nov 15, 1:54 AM