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 (138 w, 19 h)
Roles
Administrator

Recent Activity

Thu, Apr 18

sammccall added inline comments to D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path.
Thu, Apr 18, 1:57 PM · Restricted Project
sammccall added inline comments to D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path.
Thu, Apr 18, 1:00 PM · Restricted Project
sammccall committed rG3a75330f573a: [CodeComplete] Remove obsolete isOutputBinary(). (authored by sammccall).
[CodeComplete] Remove obsolete isOutputBinary().
Thu, Apr 18, 10:35 AM
sammccall committed rC358696: [CodeComplete] Remove obsolete isOutputBinary()..
[CodeComplete] Remove obsolete isOutputBinary().
Thu, Apr 18, 10:35 AM
sammccall committed rL358696: [CodeComplete] Remove obsolete isOutputBinary()..
[CodeComplete] Remove obsolete isOutputBinary().
Thu, Apr 18, 10:35 AM
sammccall committed rCTE358696: [CodeComplete] Remove obsolete isOutputBinary()..
[CodeComplete] Remove obsolete isOutputBinary().
Thu, Apr 18, 10:34 AM
sammccall committed rLLDB358696: [CodeComplete] Remove obsolete isOutputBinary()..
[CodeComplete] Remove obsolete isOutputBinary().
Thu, Apr 18, 10:34 AM
sammccall closed D60871: [CodeComplete] Remove obsolete isOutputBinary()..
Thu, Apr 18, 10:34 AM · Restricted Project, Restricted Project
sammccall added a comment to D60871: [CodeComplete] Remove obsolete isOutputBinary()..

I don't know the original motivation of this code. Since we never use it, so LGTM :)

Thu, Apr 18, 10:28 AM · Restricted Project, Restricted Project
sammccall accepted D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path.

Well done!
I never managed to track this one down, this was really annoying.

Thu, Apr 18, 10:15 AM · Restricted Project
sammccall committed rGc9e4ee9ca995: [clangd] Support relatedInformation in diagnostics. (authored by sammccall).
[clangd] Support relatedInformation in diagnostics.
Thu, Apr 18, 8:16 AM
sammccall committed rCTE358675: [clangd] Support relatedInformation in diagnostics..
[clangd] Support relatedInformation in diagnostics.
Thu, Apr 18, 8:15 AM
sammccall committed rL358675: [clangd] Support relatedInformation in diagnostics..
[clangd] Support relatedInformation in diagnostics.
Thu, Apr 18, 8:15 AM
sammccall closed D60267: [clangd] Support relatedInformation in diagnostics..
Thu, Apr 18, 8:15 AM · Restricted Project, Restricted Project
sammccall created D60871: [CodeComplete] Remove obsolete isOutputBinary()..
Thu, Apr 18, 7:58 AM · Restricted Project, Restricted Project
sammccall added a comment to D60267: [clangd] Support relatedInformation in diagnostics..

Discussed further offline - it's not clear that expressing the flattening as LSP diagnostic -> LSP diagnostic is better than the current Diag -> LSP diagnostic.

Thu, Apr 18, 5:03 AM · Restricted Project, Restricted Project
sammccall added inline comments to D60267: [clangd] Support relatedInformation in diagnostics..
Thu, Apr 18, 4:28 AM · Restricted Project, Restricted Project
sammccall committed rG99b7277d390c: [clangd] Log verbosely (LSP bodies) in lit tests. NFC (authored by sammccall).
[clangd] Log verbosely (LSP bodies) in lit tests. NFC
Thu, Apr 18, 3:31 AM
sammccall committed rCTE358655: [clangd] Log verbosely (LSP bodies) in lit tests. NFC.
[clangd] Log verbosely (LSP bodies) in lit tests. NFC
Thu, Apr 18, 3:30 AM
sammccall committed rL358655: [clangd] Log verbosely (LSP bodies) in lit tests. NFC.
[clangd] Log verbosely (LSP bodies) in lit tests. NFC
Thu, Apr 18, 3:30 AM
sammccall updated the diff for D60267: [clangd] Support relatedInformation in diagnostics..

Remove accidental copy/paste in lit test.

Thu, Apr 18, 3:27 AM · Restricted Project, Restricted Project
sammccall updated the diff for D60267: [clangd] Support relatedInformation in diagnostics..

Propagate the capability to Diagnostics, add lit test.

Thu, Apr 18, 3:27 AM · Restricted Project, Restricted Project
sammccall accepted D60821: [clangd] Emit better error messages when rename fails..

I do think it can be further simplified, but if not then land as-is.

Thu, Apr 18, 3:22 AM · Restricted Project
sammccall updated the diff for D60267: [clangd] Support relatedInformation in diagnostics..

Rebase and address comments.

Thu, Apr 18, 2:22 AM · Restricted Project, Restricted Project
sammccall added inline comments to D60267: [clangd] Support relatedInformation in diagnostics..
Thu, Apr 18, 2:22 AM · Restricted Project, Restricted Project
sammccall added inline comments to D60821: [clangd] Emit better error messages when rename fails..
Thu, Apr 18, 1:00 AM · Restricted Project

Wed, Apr 17

sammccall added a comment to D60605: [clangd] Revamp textDocument/onTypeFormatting..

@kadircet if you're interested in the behavior here, you can patch this in and try out with vscode.

Wed, Apr 17, 1:21 PM · Restricted Project
sammccall committed rGaa4eb10a7abe: [clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The… (authored by sammccall).
[clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The…
Wed, Apr 17, 1:15 PM
sammccall committed rL358612: [clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The….
[clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The…
Wed, Apr 17, 1:13 PM
sammccall committed rCTE358612: [clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The….
[clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The…
Wed, Apr 17, 1:13 PM
sammccall closed D60819: [clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The check name is reported in Diagnostic.code..
Wed, Apr 17, 1:13 PM · Restricted Project
sammccall committed rGd98170c324b1: [clangd] Use shorter, more recognizable codes for diagnostics. (authored by sammccall).
[clangd] Use shorter, more recognizable codes for diagnostics.
Wed, Apr 17, 1:12 PM
sammccall committed rL358611: [clangd] Use shorter, more recognizable codes for diagnostics..
[clangd] Use shorter, more recognizable codes for diagnostics.
Wed, Apr 17, 1:12 PM
sammccall committed rCTE358611: [clangd] Use shorter, more recognizable codes for diagnostics..
[clangd] Use shorter, more recognizable codes for diagnostics.
Wed, Apr 17, 1:12 PM
sammccall closed D60822: [clangd] Use shorter, more recognizable codes for diagnostics..
Wed, Apr 17, 1:12 PM · Restricted Project
sammccall added a comment to D60822: [clangd] Use shorter, more recognizable codes for diagnostics..

I agree that these are more useful. What about also adding "-W" in front of warning flags to make them more explicit and stand out from "non-flag" error names?

Wed, Apr 17, 1:12 PM · Restricted Project
sammccall updated the diff for D60822: [clangd] Use shorter, more recognizable codes for diagnostics..

-Wfoo instead of foo

Wed, Apr 17, 1:03 PM · Restricted Project
sammccall committed rGa96efb654e91: [clangd] Recognize "don't include me directly" pattern, and suppress include… (authored by sammccall).
[clangd] Recognize "don't include me directly" pattern, and suppress include…
Wed, Apr 17, 11:32 AM
sammccall committed rL358605: [clangd] Recognize "don't include me directly" pattern, and suppress include….
[clangd] Recognize "don't include me directly" pattern, and suppress include…
Wed, Apr 17, 11:31 AM
sammccall committed rCTE358605: [clangd] Recognize "don't include me directly" pattern, and suppress include….
[clangd] Recognize "don't include me directly" pattern, and suppress include…
Wed, Apr 17, 11:31 AM
sammccall closed D60815: [clangd] Recognize "don't include me directly" pattern, and suppress include insertion..
Wed, Apr 17, 11:31 AM · Restricted Project
sammccall added inline comments to D60815: [clangd] Recognize "don't include me directly" pattern, and suppress include insertion..
Wed, Apr 17, 7:04 AM · Restricted Project
sammccall updated the diff for D60815: [clangd] Recognize "don't include me directly" pattern, and suppress include insertion..

DontIncludeMePattern

Wed, Apr 17, 7:01 AM · Restricted Project
sammccall created D60822: [clangd] Use shorter, more recognizable codes for diagnostics..
Wed, Apr 17, 6:32 AM · Restricted Project
sammccall created D60819: [clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The check name is reported in Diagnostic.code..
Wed, Apr 17, 5:50 AM · Restricted Project
sammccall committed rG641caa57cc14: [clangd] Include textual diagnostic ID as Diagnostic.code. (authored by sammccall).
[clangd] Include textual diagnostic ID as Diagnostic.code.
Wed, Apr 17, 5:34 AM
sammccall committed rL358575: [clangd] Include textual diagnostic ID as Diagnostic.code..
[clangd] Include textual diagnostic ID as Diagnostic.code.
Wed, Apr 17, 5:34 AM
sammccall committed rCTE358575: [clangd] Include textual diagnostic ID as Diagnostic.code..
[clangd] Include textual diagnostic ID as Diagnostic.code.
Wed, Apr 17, 5:34 AM
sammccall closed D58291: [clangd] Include textual diagnostic ID as Diagnostic.code..
Wed, Apr 17, 5:34 AM · Restricted Project
sammccall added a comment to D58291: [clangd] Include textual diagnostic ID as Diagnostic.code..

It's a good question, it depends how this is surfaced, and we may want to tweak the behavior or suppress entirely in some cases.
I think at least some are useful:

  • clang-tidy check names are things users need to know about (used for configuration)
  • for warnings, we quite likely should replace with the most specific warning category (e.g. "unreachable-code-loop-increment"), again these are used for configuration (-W)
  • for others, maybe we should at least trim the err_ prefix, or maybe drop them entirely.

I see, I believe we can decide on what tweaks to perform after landing this patch and seeing behaviors in different editors, but up to you.

As for the lit-tests, it would be great to have a diag with source clang-tidy if it is not too much of a hustle.

Wed, Apr 17, 5:34 AM · Restricted Project
sammccall added inline comments to D60815: [clangd] Recognize "don't include me directly" pattern, and suppress include insertion..
Wed, Apr 17, 3:43 AM · Restricted Project
sammccall updated the diff for D60815: [clangd] Recognize "don't include me directly" pattern, and suppress include insertion..

remove leftover debugging

Wed, Apr 17, 3:42 AM · Restricted Project
sammccall updated the diff for D60815: [clangd] Recognize "don't include me directly" pattern, and suppress include insertion..

unconfusing my git repo

Wed, Apr 17, 3:40 AM · Restricted Project
sammccall created D60815: [clangd] Recognize "don't include me directly" pattern, and suppress include insertion..
Wed, Apr 17, 3:39 AM · Restricted Project
sammccall committed rG62e2472321b7: [clangd] Include insertion: require header guards, drop other heuristics, treat… (authored by sammccall).
[clangd] Include insertion: require header guards, drop other heuristics, treat…
Wed, Apr 17, 3:34 AM
sammccall committed rCTE358571: [clangd] Include insertion: require header guards, drop other heuristics, treat….
[clangd] Include insertion: require header guards, drop other heuristics, treat…
Wed, Apr 17, 3:34 AM
sammccall committed rL358571: [clangd] Include insertion: require header guards, drop other heuristics, treat….
[clangd] Include insertion: require header guards, drop other heuristics, treat…
Wed, Apr 17, 3:34 AM
sammccall closed D60316: [clangd] Include insertion: require header guards, drop other heuristics, treat .def like .inc..
Wed, Apr 17, 3:34 AM · Restricted Project, Restricted Project
sammccall added a comment to D58291: [clangd] Include textual diagnostic ID as Diagnostic.code..

LG but is this information really useful to users? According to LSP The diagnostic's code, which might appear in the user interface., I think seeing this will be mostly noise for users.

Wed, Apr 17, 2:22 AM · Restricted Project
sammccall updated the diff for D58291: [clangd] Include textual diagnostic ID as Diagnostic.code..

Rebase to head and expand scope a bit:

  • now also setting code for clang-tidy checks
  • to enable this to be used from the C++ API, the string code is now on Diag
  • and also expose Source over LSP (just an oversight?)
Wed, Apr 17, 2:14 AM · Restricted Project
sammccall updated the diff for D60316: [clangd] Include insertion: require header guards, drop other heuristics, treat .def like .inc..

address review comments

Wed, Apr 17, 1:25 AM · Restricted Project, Restricted Project

Tue, Apr 16

sammccall committed rG277754c71da7: [clangd] lower_bound -> bsearch, NFC (authored by sammccall).
[clangd] lower_bound -> bsearch, NFC
Tue, Apr 16, 11:59 PM
sammccall committed rL358561: [clangd] lower_bound -> bsearch, NFC.
[clangd] lower_bound -> bsearch, NFC
Tue, Apr 16, 11:58 PM
sammccall committed rCTE358561: [clangd] lower_bound -> bsearch, NFC.
[clangd] lower_bound -> bsearch, NFC
Tue, Apr 16, 11:58 PM
sammccall committed rG6b44291b5c4d: [ADT] llvm::bsearch, binary search for mere mortals (authored by sammccall).
[ADT] llvm::bsearch, binary search for mere mortals
Tue, Apr 16, 4:57 PM
sammccall committed rL358540: [ADT] llvm::bsearch, binary search for mere mortals.
[ADT] llvm::bsearch, binary search for mere mortals
Tue, Apr 16, 4:57 PM
sammccall closed D60779: [ADT] llvm::bsearch, binary search for mere mortals.
Tue, Apr 16, 4:57 PM · Restricted Project
sammccall updated the diff for D60804: [Support] Add JSON streaming output API, faster where the heavy value types aren't needed..

OStream constructor is explicit

Tue, Apr 16, 4:43 PM · Restricted Project
sammccall updated the diff for D60804: [Support] Add JSON streaming output API, faster where the heavy value types aren't needed..

tweak comments

Tue, Apr 16, 4:43 PM · Restricted Project
sammccall created D60804: [Support] Add JSON streaming output API, faster where the heavy value types aren't needed..
Tue, Apr 16, 4:38 PM · Restricted Project
sammccall added a comment to D60609: Use native llvm JSON library for time profiler output.

Just wanted to note I'm going to look at drafting a lower-level streaming output API so cases like this can avoid materializing all those expensive objects.

Tue, Apr 16, 1:07 PM · Restricted Project
sammccall accepted D60788: [Support][JSON] Add reserve() to json Array.

Thanks for gathering the numbers here, it is really useful to see how people are using this in performance-sensitive contexts. Definitely makes sense to fix this.

Tue, Apr 16, 12:38 PM · Restricted Project
sammccall added a comment to D60539: Add -std=c++14 language standard option to tests that require C++14 default.

Do you need to build clangd? We explicitly don't aim to support building everywhere clang can be built, maybe we should just disable in this case?

Our environment includes various OS levels running on PowerPC. We certainly wouldn't want to disable building/testing clangd on all our PowerPC machines. Is there a way to disable it only on certain OS levels?

Furthermore, it seems a little too intrusive to disable an otherwise functional component simply because some test cases rely on a specific language standard default.

Would it be an acceptable solution to add another StringRef parameter to ShouldCollectSymbolTest::build() - let's call it ExtraArgs, to which we can add options such as -std=c++14 if the test being built relies on that option?

Tue, Apr 16, 12:08 PM · Restricted Project, Restricted Project
sammccall updated the diff for D60779: [ADT] llvm::bsearch, binary search for mere mortals.

Predicate, not compare.

Tue, Apr 16, 9:14 AM · Restricted Project
sammccall created D60779: [ADT] llvm::bsearch, binary search for mere mortals.
Tue, Apr 16, 9:14 AM · Restricted Project
sammccall added inline comments to D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library.
Tue, Apr 16, 7:59 AM · Restricted Project
sammccall added inline comments to D60687: [clangd] Check file path of declaring header when deciding whether to insert include..
Tue, Apr 16, 2:09 AM · Restricted Project, Restricted Project
sammccall accepted D60687: [clangd] Check file path of declaring header when deciding whether to insert include..

IIUC, this already fixes the cases we'd seen of include-insertion of a header into itself.
Is it feasible to add a test case for that?

Tue, Apr 16, 1:52 AM · Restricted Project, Restricted Project
sammccall updated the diff for D60605: [clangd] Revamp textDocument/onTypeFormatting..

Lots more test cases and better handling of braced blocks.

Tue, Apr 16, 1:47 AM · Restricted Project

Mon, Apr 15

sammccall accepted D60689: [clangd] Fallback to OrigD when SLoc is invalid.
Mon, Apr 15, 7:34 AM · Restricted Project

Fri, Apr 12

sammccall updated the diff for D60605: [clangd] Revamp textDocument/onTypeFormatting..

Unit tests.

Fri, Apr 12, 5:45 AM · Restricted Project
sammccall requested changes to D60539: Add -std=c++14 language standard option to tests that require C++14 default.

I don't think this is a suitable fix :-(

Fri, Apr 12, 5:05 AM · Restricted Project, Restricted Project
sammccall created D60605: [clangd] Revamp textDocument/onTypeFormatting..
Fri, Apr 12, 3:49 AM · Restricted Project

Thu, Apr 11

sammccall accepted D60126: [clangd] Use identifiers in file as completion candidates when build is not ready..
Thu, Apr 11, 2:26 AM · Restricted Project
sammccall committed rGc218813cba1e: [clangd] Include compile command heuristic in logs (authored by sammccall).
[clangd] Include compile command heuristic in logs
Thu, Apr 11, 1:17 AM
sammccall committed rL358157: [clangd] Include compile command heuristic in logs.
[clangd] Include compile command heuristic in logs
Thu, Apr 11, 1:17 AM
sammccall committed rCTE358157: [clangd] Include compile command heuristic in logs.
[clangd] Include compile command heuristic in logs
Thu, Apr 11, 1:17 AM

Wed, Apr 10

sammccall committed rG2d02c6df6b2b: [clangd] Fix non-indexing of builtin functions like printf when the TU is C (authored by sammccall).
[clangd] Fix non-indexing of builtin functions like printf when the TU is C
Wed, Apr 10, 9:27 AM
sammccall committed rCTE358098: [clangd] Fix non-indexing of builtin functions like printf when the TU is C.
[clangd] Fix non-indexing of builtin functions like printf when the TU is C
Wed, Apr 10, 9:26 AM
sammccall committed rL358098: [clangd] Fix non-indexing of builtin functions like printf when the TU is C.
[clangd] Fix non-indexing of builtin functions like printf when the TU is C
Wed, Apr 10, 9:26 AM
sammccall accepted D60510: [ADT] Fix template parameter names of llvm::{upper|lower}_bound.
Wed, Apr 10, 8:20 AM · Restricted Project
sammccall committed rGb814e57ffba0: [clangd] Don't insert extra namespace qualifiers when Sema gets lost. (authored by sammccall).
[clangd] Don't insert extra namespace qualifiers when Sema gets lost.
Wed, Apr 10, 8:16 AM
sammccall committed rCTE358091: [clangd] Don't insert extra namespace qualifiers when Sema gets lost..
[clangd] Don't insert extra namespace qualifiers when Sema gets lost.
Wed, Apr 10, 8:15 AM
sammccall committed rL358091: [clangd] Don't insert extra namespace qualifiers when Sema gets lost..
[clangd] Don't insert extra namespace qualifiers when Sema gets lost.
Wed, Apr 10, 8:15 AM
sammccall closed D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost..
Wed, Apr 10, 8:15 AM · Restricted Project, Restricted Project
sammccall committed rG71660b032164: Revert "[LLVM-C] Correct The Current Debug Location Accessors" (authored by sammccall).
Revert "[LLVM-C] Correct The Current Debug Location Accessors"
Wed, Apr 10, 6:29 AM
sammccall committed rL358082: Revert "[LLVM-C] Correct The Current Debug Location Accessors".
Revert "[LLVM-C] Correct The Current Debug Location Accessors"
Wed, Apr 10, 6:28 AM
sammccall added a comment to D60484: [LLVM-C] Correct The Current Debug Location Accessors.

These symbols conflict with identically-named symbols in llvm/bindings/go/llvm/IRBindings.h, going to revert (I don't know what the right name is, and in the C API that's probably important)

Wed, Apr 10, 6:28 AM · Restricted Project
sammccall added a comment to D60126: [clangd] Use identifiers in file as completion candidates when build is not ready..

Very nice! D60503 will conflict, feel free to stall that until this is landed.
On the other hand it will simplify some things, e.g. the prefix is already calculated, and typed scope is available if you want that (no enclosing namespaces though).

Wed, Apr 10, 5:38 AM · Restricted Project
sammccall committed rG9b765de6dd17: [clangd] Add -header-insertion=never flag to disable include insertion in code… (authored by sammccall).
[clangd] Add -header-insertion=never flag to disable include insertion in code…
Wed, Apr 10, 5:14 AM