Page MenuHomePhabricator

sammccall (Sam McCall)
UserAdministrator

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Fri, Apr 9

sammccall added a comment to D100106: [clangd] Provide a way to disable external index.

Using server: None for this seems a bit weird and irregular (why not file: none?)

Fri, Apr 9, 7:07 AM · Restricted Project

Thu, Apr 8

sammccall added inline comments to D98499: [clangd] Introduce ASTHooks to FeatureModules.
Thu, Apr 8, 5:36 AM · Restricted Project, Restricted Project

Tue, Apr 6

sammccall accepted D99934: [clang][Syntax] Handle invalid source range in expandedTokens..

(Do we have a bug this fixes?)

Tue, Apr 6, 9:12 AM · Restricted Project
sammccall accepted D98970: [clangd] Add --check-range to restrict --check to specific lines.

Still LG

Tue, Apr 6, 9:11 AM · Restricted Project

Mon, Apr 5

sammccall added a comment to D98748: [clangd] Add support for inline parameter hints.

This is looking pretty good! Can't wait to try it, and some of my suggestions are just scope creep, so feel free to draw the line :-)

Mon, Apr 5, 6:24 AM · Restricted Project

Wed, Mar 31

sammccall added inline comments to D98748: [clangd] Add support for inline parameter hints.
Wed, Mar 31, 1:43 PM · Restricted Project
sammccall added a comment to D98748: [clangd] Add support for inline parameter hints.

Thanks for working on this! It looks like it's getting traction in VSCode/LSP now, which makes it much more compelling.

Wed, Mar 31, 8:46 AM · Restricted Project

Thu, Mar 25

sammccall accepted D99086: [clang][Syntax] Optimize expandedTokens for token ranges..

Just doc nits - I think maybe there's some confusion on what a token range is.
Code looks good though!

Thu, Mar 25, 9:47 AM · Restricted Project, Restricted Project
sammccall accepted D99326: [clangd] Fix a use-after-free.
Thu, Mar 25, 8:08 AM · Restricted Project

Wed, Mar 24

sammccall added a comment to D98499: [clangd] Introduce ASTHooks to FeatureModules.

My model for modules using this diagnostic stuff (apart from the build-system stuff which sadly can't be meaningfully upstreamed) are IncludeFixer, ClangTidy, and IWYU - worth thinking about how we'd build those on top of this model. (Doesn't mean we need to add a hook to emit diagnostics, but it probably means we should know where it would go)

Agreed. I believe they all would need extra end points to ASTHooks though.

Yup. Let's not bite off more for now.

  • make ASTHooks own it and instantiate a new one on every parse (i think the cleanest and most explicit)

Agreed. (And this only one that overlaps this patch a lot)

Wed, Mar 24, 10:21 AM · Restricted Project, Restricted Project
sammccall accepted D98538: [clangd] Perform merging for stale symbols in MergeIndex.
Wed, Mar 24, 10:00 AM · Restricted Project, Restricted Project
sammccall accepted D98505: [clangd] Propagate data in diagnostics.
Wed, Mar 24, 9:48 AM · Restricted Project, Restricted Project
sammccall accepted D98364: [clangd] Use ref counted strings throughout for File Contents.
Wed, Mar 24, 6:33 AM · Restricted Project, Restricted Project
sammccall accepted D99165: [clang] Treat variable-length array of incomplete element type as incomplete type..

Thanks for digging!

Wed, Mar 24, 4:50 AM · Restricted Project

Tue, Mar 23

sammccall updated subscribers of D99086: [clang][Syntax] Optimize expandedTokens for token ranges..
Tue, Mar 23, 11:45 AM · Restricted Project, Restricted Project
sammccall accepted D98970: [clangd] Add --check-range to restrict --check to specific lines.

Thanks! Just some random nits/ideas.

Tue, Mar 23, 9:38 AM · Restricted Project
sammccall added inline comments to D99086: [clang][Syntax] Optimize expandedTokens for token ranges..
Tue, Mar 23, 5:24 AM · Restricted Project, Restricted Project
sammccall added a comment to D99086: [clang][Syntax] Optimize expandedTokens for token ranges..

Thanks for finding and working on this hotspot. (I'm curious *why* isBeforeInTranslationUnit is slow if you have any insight - it has sad worst-case but I thought we'd hit the one-element cache often enough in practice).

Tue, Mar 23, 4:47 AM · Restricted Project, Restricted Project
sammccall updated subscribers of D99165: [clang] Treat variable-length array of incomplete element type as incomplete type..

The fix doesn't look obviously correct: the side effect of marking the destructor reference seems important if we actually generate code. It's not obvious to me why the type can only be incomplete if there are errors.

Tue, Mar 23, 4:07 AM · Restricted Project

Wed, Mar 17

sammccall added a comment to D96626: Support: mapped_file_region: Pass MAP_NORESERVE to mmap.

Sorry about the lack of response here.

Wed, Mar 17, 8:30 AM · Restricted Project

Tue, Mar 16

sammccall committed rG128ce70eef99: [CodeCompletion] Avoid spurious signature help for init-list args (authored by sammccall).
[CodeCompletion] Avoid spurious signature help for init-list args
Tue, Mar 16, 4:47 AM
sammccall closed D98488: [CodeCompletion] Avoid spurious signature help for init-list args.
Tue, Mar 16, 4:47 AM · Restricted Project, Restricted Project
sammccall committed rGca13f5595ae8: [clangd] Add `limit` extension on completion and workspace-symbols (authored by sammccall).
[clangd] Add `limit` extension on completion and workspace-symbols
Tue, Mar 16, 4:30 AM
sammccall closed D97801: [clangd] Add `limit` extension on completion and workspace-symbols.
Tue, Mar 16, 4:30 AM · Restricted Project, Restricted Project
sammccall committed rG3b99731c4e7b: [clangd] Turn off implicit cancellation based on client capabilities (authored by sammccall).
[clangd] Turn off implicit cancellation based on client capabilities
Tue, Mar 16, 4:28 AM
sammccall closed D98414: [clangd] Turn off implicit cancellation based on client capabilities.
Tue, Mar 16, 4:27 AM · Restricted Project, Restricted Project
sammccall committed rG43d0b1c9c16c: [clangd] Reject renames to non-identifier characters (authored by sammccall).
[clangd] Reject renames to non-identifier characters
Tue, Mar 16, 4:19 AM
sammccall closed D98424: [clangd] Reject renames to non-identifier characters.
Tue, Mar 16, 4:18 AM · Restricted Project, Restricted Project
sammccall committed rGa92693dac459: [CodeCompletion] Don't track preferred types if code completion is disabled. (authored by sammccall).
[CodeCompletion] Don't track preferred types if code completion is disabled.
Tue, Mar 16, 4:17 AM
sammccall closed D98459: [CodeCompletion] Don't track preferred types if code completion is disabled..
Tue, Mar 16, 4:16 AM · Restricted Project
sammccall accepted D98623: [clangd] Introduce pullDiags endpoint.
Tue, Mar 16, 3:29 AM · Restricted Project

Mon, Mar 15

sammccall added a comment to D98621: [clangd] Keep count of delayed diagnostics.

This seems like something we can delay until we're sure it'll do what we want? (e.g. we see that we either usually know synchronously whether lazy fixes will be present, or can predict it with high confidence)

Mon, Mar 15, 11:36 AM · Restricted Project
Herald added a project to D97555: [clangd] Add diagnostic augmentation hook to Modules: Restricted Project.

Is this entirely obsoleted by the approach in D98499?

Mon, Mar 15, 11:33 AM · Restricted Project, Restricted Project
sammccall accepted D98498: [clangd] Enable modules to contribute tweaks..

Should we have a test somewhere that tweaks defined in modules actually work?

Mon, Mar 15, 11:32 AM · Restricted Project, Restricted Project
Herald added a project to D98505: [clangd] Propagate data in diagnostics: Restricted Project.
Mon, Mar 15, 10:09 AM · Restricted Project, Restricted Project
Herald added a project to D98499: [clangd] Introduce ASTHooks to FeatureModules: Restricted Project.

I'm getting a little nervous about the amount of stuff we're packing into modules without in-tree examples.
I should split out some of the "standard" features into modules as that's possible already.

Mon, Mar 15, 9:57 AM · Restricted Project, Restricted Project
sammccall requested changes to D98618: [clang-tidy] Add --skip-headers, part 1.

Can I ask what is the purpose of --skip-headers?
What does it offer that --header-filter doesn't already offer?

Mon, Mar 15, 4:20 AM · Restricted Project
sammccall added a comment to D98623: [clangd] Introduce pullDiags endpoint.

Thanks, nice to see this isn't going to require deep changes, and looking forward to seeing the diagnostics performance improvements.
(I wonder if we'll end up in the perverse situation where clangd is faster for most operations when your preamble is stale :-D Probably not...)

Mon, Mar 15, 4:10 AM · Restricted Project

Fri, Mar 12

sammccall updated the diff for D98488: [CodeCompletion] Avoid spurious signature help for init-list args.

oops, enable clangd test

Fri, Mar 12, 5:49 AM · Restricted Project, Restricted Project
sammccall accepted D98483: [clang] Mark re-injected tokens appropriately during pragma handling.
Fri, Mar 12, 4:59 AM · Restricted Project
sammccall requested review of D98488: [CodeCompletion] Avoid spurious signature help for init-list args.
Fri, Mar 12, 4:03 AM · Restricted Project, Restricted Project

Mar 12 2021

sammccall updated the diff for D98459: [CodeCompletion] Don't track preferred types if code completion is disabled..

Omit narrower, now-redundant check

Mar 12 2021, 12:09 AM · Restricted Project

Mar 11 2021

sammccall requested review of D98459: [CodeCompletion] Don't track preferred types if code completion is disabled..
Mar 11 2021, 3:05 PM · Restricted Project
sammccall accepted D98165: [clangd] Make ProjectAwareIndex optionally sync.
Mar 11 2021, 11:08 AM · Restricted Project
sammccall accepted D98037: [clangd] Add config block for Completion and option for AllScopes.
Mar 11 2021, 11:07 AM · Restricted Project
sammccall added inline comments to D98414: [clangd] Turn off implicit cancellation based on client capabilities.
Mar 11 2021, 8:05 AM · Restricted Project, Restricted Project
sammccall added inline comments to D98424: [clangd] Reject renames to non-identifier characters.
Mar 11 2021, 7:24 AM · Restricted Project, Restricted Project
sammccall requested review of D98424: [clangd] Reject renames to non-identifier characters.
Mar 11 2021, 6:44 AM · Restricted Project, Restricted Project
sammccall requested review of D98414: [clangd] Turn off implicit cancellation based on client capabilities.
Mar 11 2021, 4:44 AM · Restricted Project, Restricted Project
sammccall added a comment to D98242: [clangd] Store system relative includes as verbatim.

I've landed the caching patch so this is good to go.
The merge is *conceptually* trivial but the enclosing function has moved so you may need to reapply it by hand, sorry...

Mar 11 2021, 4:06 AM · Restricted Project, Restricted Project
sammccall committed rGb8c58374f66b: [clangd] Group filename calculations in SymbolCollector, and cache mroe. (authored by sammccall).
[clangd] Group filename calculations in SymbolCollector, and cache mroe.
Mar 11 2021, 3:59 AM
sammccall closed D98371: [clangd] Group filename calculations in SymbolCollector, and cache mroe..
Mar 11 2021, 3:59 AM · Restricted Project

Mar 10 2021

sammccall added a comment to D98364: [clangd] Use ref counted strings throughout for File Contents.

Avoiding copies seems nice, but this makes some interfaces more awkward. Do you have any measurements that some of these copies matter?

Mar 10 2021, 5:15 PM · Restricted Project, Restricted Project
sammccall updated the diff for D98377: [clangd] Show padding following a field on field hover..

Stop reporting offset for fields in a union, too

Mar 10 2021, 4:52 PM · Restricted Project, Restricted Project
sammccall added inline comments to D98377: [clangd] Show padding following a field on field hover..
Mar 10 2021, 4:48 PM · Restricted Project, Restricted Project
sammccall requested review of D98377: [clangd] Show padding following a field on field hover..
Mar 10 2021, 4:21 PM · Restricted Project, Restricted Project
sammccall added a comment to D98242: [clangd] Store system relative includes as verbatim.

I measured this as a mild (3%) increase in preamble indexing time when working on clangd itself, more details in D98329

Mar 10 2021, 2:49 PM · Restricted Project, Restricted Project
sammccall added a comment to D98329: [clangd] Add cache for FID to Header mappings.

Some performance measurements...
(workload is PreambleCallback for clangd/XRefs.cpp in sync mode with a fixed baseline of 99b01cf28db9db1a3ec0e25367bd325b7aca6d43, opt build without asserts)

Mar 10 2021, 2:42 PM · Restricted Project
sammccall requested review of D98371: [clangd] Group filename calculations in SymbolCollector, and cache mroe..
Mar 10 2021, 2:23 PM · Restricted Project
sammccall added inline comments to D98246: [clangd] Add basic monitoring info request for remote index server.
Mar 10 2021, 12:51 AM · Restricted Project, Restricted Project, Restricted Project

Mar 9 2021

sammccall added inline comments to D98246: [clangd] Add basic monitoring info request for remote index server.
Mar 9 2021, 2:05 PM · Restricted Project, Restricted Project, Restricted Project
sammccall accepted D96316: [ASTMatcher] Add AST Matcher support for C++20 coroutine keywords.

LGTM with the doc nits from njames93 (and the doc generator script re-run)

Mar 9 2021, 1:50 PM · Restricted Project
sammccall accepted D98242: [clangd] Store system relative includes as verbatim.

I think this is going to be more good than bad, and the alternatives are terrifically complicated.

Mar 9 2021, 12:37 PM · Restricted Project, Restricted Project
sammccall accepted D94554: [clangd] Add a Filesystem that overlays Dirty files..

Still LG btw!

Mar 9 2021, 3:32 AM · Restricted Project
sammccall added inline comments to D98242: [clangd] Store system relative includes as verbatim.
Mar 9 2021, 3:25 AM · Restricted Project, Restricted Project
sammccall accepted D98241: [clangd] Move logging out of LSPTest base class into a separate one..

Thanks for working this out!

Mar 9 2021, 2:43 AM · Restricted Project
sammccall added inline comments to D94554: [clangd] Add a Filesystem that overlays Dirty files..
Mar 9 2021, 2:40 AM · Restricted Project
sammccall accepted D95043: [clangd] Use Dirty Filesystem for cross file rename..

This looks pretty good, great cleanup!

Mar 9 2021, 12:21 AM · Restricted Project

Mar 8 2021

sammccall accepted D94554: [clangd] Add a Filesystem that overlays Dirty files..

Thanks, LG!

Mar 8 2021, 8:33 AM · Restricted Project
sammccall added inline comments to D96975: [Sema] Add some basic lambda capture fix-its.
Mar 8 2021, 8:24 AM · Restricted Project
sammccall added a comment to D96975: [Sema] Add some basic lambda capture fix-its.

Nice fixes! A few drive-by comments from me, up to you though.

Mar 8 2021, 5:05 AM · Restricted Project
sammccall accepted D97688: clang-format: use `pb` as a canonical raw string delimiter for google style.

Also fixes a bug where the canonical delimiter was not applied for raw strings with empty delimiters detected via well-known enclosing functions that expect a text proto.

Mar 8 2021, 1:51 AM · Restricted Project

Mar 5 2021

sammccall accepted D98034: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace..

Nice! Only substantive suggestion: I think requiring the base type to be exactly a standard map type is too conservative.

Mar 5 2021, 4:44 AM · Restricted Project, Restricted Project
sammccall accepted D98029: [clangd] Introduce a CommandLineConfigProvider.

Both side-effects seem fine to me.

Mar 5 2021, 4:14 AM · Restricted Project
sammccall committed rGa60d06d8b757: [clangd] Rename Module -> FeatureModule to avoid confusion. NFC (authored by sammccall).
[clangd] Rename Module -> FeatureModule to avoid confusion. NFC
Mar 5 2021, 1:15 AM
sammccall closed D97950: [clangd] Rename Module -> FeatureModule to avoid confusion. NFC.
Mar 5 2021, 1:15 AM · Restricted Project

Mar 4 2021

sammccall added a comment to D97950: [clangd] Rename Module -> FeatureModule to avoid confusion. NFC.

I know we talked about names a bunch and i'm being a bit intransigent here.
Happy to go with Component if you think it's a better name, but wanted to see how this looks.

Mar 4 2021, 7:26 AM · Restricted Project
sammccall requested review of D97950: [clangd] Rename Module -> FeatureModule to avoid confusion. NFC.
Mar 4 2021, 7:24 AM · Restricted Project
sammccall added a comment to D96245: [clangd] Propagate CodeActions in addition to Fixes for diagnostics.

Agreed, I was going this way because currently there was no way to provide a code action to clangd::diagnostic apart from storing it in the fixes map while generating it. I suppose it is more sensible to just go with a new hook on modules that can provide a codeaction given a clangd::diagnostic.

Mar 4 2021, 5:01 AM · Restricted Project, Restricted Project

Mar 3 2021

sammccall added inline comments to D97555: [clangd] Add diagnostic augmentation hook to Modules.
Mar 3 2021, 9:49 PM · Restricted Project, Restricted Project
sammccall added a comment to D96245: [clangd] Propagate CodeActions in addition to Fixes for diagnostics.

Will the fixes/actions this enables really be available synchronously?

Mar 3 2021, 9:36 PM · Restricted Project, Restricted Project
sammccall accepted D97548: [clangd] Introduce client state invalidation.
Mar 3 2021, 9:27 PM · Restricted Project
sammccall committed rG7d2fba8ddb90: [clangd] ObjC fixes for semantic highlighting and xref highlights (authored by sammccall).
[clangd] ObjC fixes for semantic highlighting and xref highlights
Mar 3 2021, 11:16 AM
sammccall closed D97617: [clangd] ObjC fixes for semantic highlighting and xref highlights.
Mar 3 2021, 11:16 AM · Restricted Project
sammccall committed rG1a4990a4f71a: [clangd] Fix uninit member (authored by sammccall).
[clangd] Fix uninit member
Mar 3 2021, 2:45 AM

Mar 2 2021

sammccall committed rGbca3e24139cc: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer. (authored by sammccall).
[clangd] Move DraftStore from ClangdLSPServer into ClangdServer.
Mar 2 2021, 1:59 PM
sammccall closed D97738: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer..
Mar 2 2021, 1:59 PM · Restricted Project
sammccall updated the diff for D97803: [clangd] Overload bundles are only deprecated if each overloads is..

simplify

Mar 2 2021, 1:50 PM · Restricted Project
sammccall updated the diff for D97803: [clangd] Overload bundles are only deprecated if each overloads is..

fix bug link

Mar 2 2021, 1:28 PM · Restricted Project
sammccall requested review of D97803: [clangd] Overload bundles are only deprecated if each overloads is..
Mar 2 2021, 1:26 PM · Restricted Project
sammccall requested review of D97801: [clangd] Add `limit` extension on completion and workspace-symbols.
Mar 2 2021, 1:16 PM · Restricted Project, Restricted Project
sammccall accepted D97773: [clangd] Make WorkspaceSymbols request work with empty queries.

OK, I think we've established that the request was for a way to dump out clangd's whole index, and workspace/symbol isn't actually a good way to do that.
Nevertheless, this seems useful and endorsed by the spec.

Mar 2 2021, 12:53 PM · Restricted Project
sammccall added a comment to D97738: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer..

as discussed offline, i am hesitant about validateEdits layering at the moment. but i suppose the best thing to do is move it into clangdserver and expose a structrual api, as you proposed, which can be done independently.

Mar 2 2021, 9:08 AM · Restricted Project
sammccall updated the diff for D97738: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer..

address comments
remove one more redundant getDraft() check

Mar 2 2021, 9:08 AM · Restricted Project
sammccall committed rG91679c95bbed: [clangd] Include macro expansions in documentSymbol hierarchy (authored by sammccall).
[clangd] Include macro expansions in documentSymbol hierarchy
Mar 2 2021, 8:52 AM
sammccall closed D97615: [clangd] Include macro expansions in documentSymbol hierarchy.
Mar 2 2021, 8:52 AM · Restricted Project
sammccall added a comment to D97773: [clangd] Make WorkspaceSymbols request work with empty queries.

I've asked on the bug for some more context for the request.
I don't see a problem with this change per se but I'm not sure it'll actually fix the bug.

Mar 2 2021, 8:49 AM · Restricted Project
sammccall accepted D97771: [cte][NFC] Remove all references to stdlib stream headers..

LGTM (apart from Aaron's comment!)

Mar 2 2021, 8:23 AM · Restricted Project
sammccall committed rG289fee4ab762: [clangd] Show hex value of numeric constants (authored by sammccall).
[clangd] Show hex value of numeric constants
Mar 2 2021, 7:33 AM
sammccall closed D97226: [clangd] Show hex value of numeric constants.
Mar 2 2021, 7:33 AM · Restricted Project