[PATCH] D87669: [clangd] Rainbow Semantic Highlighting

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. This is cool! We've had a couple of looks at attributes, but haven't really gotten around to polishing it. (For a submittable version, I think we should avoid introducing more modifiers at the same time, just to avoid getting bogged down in exactly which ones) I like

[PATCH] D85611: Improve dynamic AST matching diagnostics for conversion errors

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Sorry about the delay, good guess as to what was happening :-) Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.h:88 + static bool hasCorrectValue(const

[PATCH] D87080: [AST] Reduce the size of TemplateArgumentLocInfo.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Looks good apart from a minor technical issue with the traits. Would it be possible to compile some big file in LLVM (probably doesn't matter much which, Sema something?) and observe if there's a significant change in overall ASTContext size?

[PATCH] D84387: [AST][RecoveryExpr] Part4: Suppress spurious "err_typecheck_expect_scalar_operand" diagnostic

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. I'm not sure I love having the assertion for contains-errors every place that handles dependent code in C. We should certainly somehow document the reason dependence is possible, just as the possibility of overloads is documented. But the assertion seems pretty

[PATCH] D84322: [AST][RecoveryExpr] Part3: Suppress spurious "typecheck_cond_expect_scalar" diagnostic

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/test/Sema/error-dependence.c:18 + // type is required" is not emitted. + ptr > f ? ptr : f; // expected-error {{invalid operands to binary expression}} +} nit: parens would help me understand here :-) I

[PATCH] D84226: [AST][RecoveryExpr] Part1: Support dependent binary operator in C for error recovery.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:14255 + if (Opc <= BO_Assign || Opc > BO_OrAssign) +return BinaryOperator::Create(Context, LHS, RHS, Opc, Context.DependentTy, + VK_RValue, OK_Ordinary, OpLoc,

[PATCH] D84304: [AST][RecoveryExpr] Part 2: Build dependent callexpr in C for error-recovery.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:6437 + if (getLangOpts().CDependence && + (Fn->isTypeDependent() || Expr::hasAnyTypeDependentArguments(ArgExprs))) { why is CDependence a condition and not an assertion inside the

[PATCH] D84226: [AST][RecoveryExpr] Part1: Support dependent binary operator in C for error recovery.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:153 COMPATIBLE_LANGOPT(RecoveryASTType, 1, 0, "Preserve the type in recovery expressions") +COMPATIBLE_LANGOPT(CDependence, 1, 0, "Build dependent AST nodes in C for better error recovery")

[PATCH] D86765: [clang] Don't emit "no member" diagnostic if the lookup fails on an invalid record decl.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This seems like fixing a fairly specific case of a general problem: if the base class is invalid I would expect to see similar problems in other places (e.g. unqualified lookup from inside the class). But it fixes a diagnostic the

[PATCH] D83296: [clang-format] Add a MacroExpander.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Just checking this is waiting on you rather than me... If the multiple-expansion thing is blocking progress, I think we're much better off getting a limited version of this feature landed than losing momentum trying to solve it. Repository: rG LLVM Github

[PATCH] D82284: [AST][RecoveryAST] Preseve invalid return stmt, and suppress the diagnostics for missing return stmt in constexpr func.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Any interest in chasing these diagnostic regressions (they seem like they might be tractable) or should we abandon this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82284/new/ https://reviews.llvm.org/D82284

[PATCH] D76831: [AST] Preserve the DeclRefExpr when it refers to an invalid decl.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. @hokein I think this patch is obsolete, right? Richard's last comment is basically what's currently implemented: if lookup doesn't yield any valid results, we create a recoveryexpr for the failed lookup. It doesn't have a pointer to the invalid decl (which would be

[PATCH] D77527: [AST] Adjust existing tests for recovery-exprs

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall resigned from this revision. sammccall added a comment. Herald added a subscriber: sstefan1. (This is obsolete now I believe, the flag has been flipped) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77527/new/

[PATCH] D71586: [clangd] Add a test case that verifies -fimplicit-modules work in Clangd when building the AST

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. We do now have tests in unittests/ModulesTests.cpp that likely covers at least part of this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71586/new/ https://reviews.llvm.org/D71586

[PATCH] D80109: [AST][RecoveryExpr] Preserve type for broken member call expr.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. @hokein Did this get landed at some point? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80109/new/ https://reviews.llvm.org/D80109 ___ cfe-commits mailing list

[PATCH] D86077: [clangd] Add a way for exporting memory usage metrics

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Just for the record, as I think Kadir and Adam are both aware... We discussed making this a bit richer and less reliant on static state. We'd build a tree-shaped profile by passing a tree-builder recursively into various components. Then the metrics would be exported

[PATCH] D87350: [AST][RecoveryExpr] Fix a crash on undeduced type.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:12880 return Result.getValueOr(QualType()); } Wouldn't we be better to handle undeduced type here rather than in the loop? Not sure if it practically matters, but given: `auto

[PATCH] D83814: [clangd] Add Random Forest runtime for code completion.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG from my side! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83814/new/ https://reviews.llvm.org/D83814

[PATCH] D87775: [clangd] Add option for disabling AddUsing tweak on some namespaces.

2020-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! Can we add a simple test to TweakTests (I should really split up that file) and to ConfigCompileTests? (BTW, do you think we should merge Config{Compile,YAML}Tests?

[PATCH] D87775: [clangd] Add option for disabling AddUsing tweak on some namespaces.

2020-09-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Nice! Comment at: clang-tools-extra/clangd/Config.h:66 + + /// Style of the codebase. + struct { describe elements more precisely? They are namespaces with/without leading/trailing ::, and sub-namespaces are implicitly included.

[PATCH] D83814: [clangd] Add Random Forest runtime for code completion.

2020-09-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Just some naming and doc nits. This looks really solid now, nice job! In D83814#2261458 , @jkorous wrote: > Hi @usaxena95 and @sammccall, > > I am wondering about couple high-level things. > > Do you guys intend to open-source

[PATCH] D87349: [clang] adapt c++17 behavior for dependent decltype-specifiers

2020-09-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/test/CodeGenCXX/mangle.cpp:805 - // CHECK-LABEL: define weak_odr void @_ZN6test341fIiEEvDTstDTplcvT__EcvS1__EEE + // CHECK-LABEL: define weak_odr void @_ZN6test341fIiEEvm template void f(decltype(sizeof(1)));

[PATCH] D87710: [clangd] Actually parse Index section of the YAML file.

2020-09-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks Hans! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87710/new/ https://reviews.llvm.org/D87710 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D87745: [clangd] Config: handle Index block

2020-09-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for catching this! Lucky coincidence it was spotted twice just before rc3! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87745/new/ https://reviews.llvm.org/D87745 ___

[PATCH] D87349: [clang] adapt c++17 behavior for dependent decltype-specifiers

2020-09-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Ugh, the hole goes deeper, because the Itanium ABI unambiguously follows the C++11 (instantiation-dependent) behavior. Definitely need Richard's input here as I'm fairly sure this will break things as-is. Comment at:

[PATCH] D87349: [clang] adapt c++17 behavior for dependent decltype-specifiers

2020-09-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. We should check if the mangling matches GCC (or if it depends on -std), if we can. Comment at: clang/lib/AST/Type.cpp:3429 toTypeDependence(E->getDependence()) | - (E->isInstantiationDependent() ? TypeDependence::Dependent

[PATCH] D87686: [clang-tidy] Improve documentation on Clangd integration

2020-09-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for doing this! (Agree with Haojian's comments too) Comment at: clang-tools-extra/docs/clang-tidy/Integrations.rst:10 Apart from being a standalone tool, :program:`clang-tidy` is integrated into -various IDEs, code analyzers, and editors.

[PATCH] D87673: [clangd] Don't use zlib when it's unavailable.

2020-09-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. Thanks, this seems better than crashing. The practical result isn't wonderful: the two are going to fight over index files to some extent. But this can happen with different clangd versions (that use different index format versions)

[PATCH] D87710: [clangd] Actually parse Index section of the YAML file.

2020-09-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a subscriber: hans. sammccall added a comment. This revision is now accepted and ready to land. Argh, sorry. I think this might be too late to get into 11 - we're in the "soon=final" stage and maybe any changes at all cause logistical problems?

[PATCH] D83419: [clangd] Add error() function for creating formatv-style llvm::Errors. NFC

2020-09-14 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG30667c967d3f: [clangd] Add error() function for creating formatv-style llvm::Errors. NFC (authored by sammccall). Changed prior to commit:

[PATCH] D87080: [AST] Reduce the size of TemplateArgumentLocInfo.

2020-09-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Nit on the commit message: I think this is TemplateArgumentLoc 48 -> 32 bytes, and DynTypedNode 56 -> 40 bytes. Comment at: clang/include/clang/AST/TemplateBase.h:415 -public: - constexpr TemplateArgumentLocInfo() : Template({nullptr, nullptr, 0,

[PATCH] D86048: [AST][RecoveryExpr] Popagate the error-bit from a VarDecl's initializer to DeclRefExpr.

2020-09-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a subscriber: rsmith. sammccall added a comment. Neither of the testcases look like the right behavior to me, and I think the bug is elsewhere in clang. The crux is we're forcing `decltype(N)` to be a (unique) dependent type, which feels wrong. This isn't specific to

[PATCH] D86936: [clang] Limit the maximum level of fold-expr expansion.

2020-09-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5096 +def err_fold_expression_limit_exceeded: Error< + "instantiating fold expression with %0

[PATCH] D86936: [clang] Limit the maximum level of fold-expr expansion.

2020-09-01 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5092 "expression not permitted as operand of fold expression">; +def err_fold_expression_expansion_exceeded: Error< + "fold expression expansion level %0 exceeded maximum of %1">;

[PATCH] D86964: [ASTMatchers] Avoid recursion in ancestor matching to save stack space.

2020-09-01 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. sammccall added a reviewer: hokein. Herald added a project: clang. Herald added a subscriber: cfe-commits. sammccall requested review of this revision. A recent change increased the stack size of memoizedMatchesAncestorOfRecursively leading to stack overflows on

[PATCH] D86597: [Tooling][Format] Treat compound extensions (foo.bar.cc) as matching foo.h

2020-08-27 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG266825620c7f: [Tooling][Format] Treat compound extensions (foo.bar.cc) as matching foo.h (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86688: [RecoveryExpr] Add 11.0.0 release note.

2020-08-27 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/docs/ReleaseNotes.rst:65 + + .. code-block:: c++ + Oops, I missed this, I think this should be three code blocks: one for C++, one for the first AST and one for the second AST. Tex like "clang-10 produces the

[PATCH] D86688: [RecoveryExpr] Add 11.0.0 release note.

2020-08-27 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/docs/ReleaseNotes.rst:51 +Recovery ASTs +^ nit: AST rather than ASTs (matching the flag, and also it's AST is

[PATCH] D86624: [clang] Exclude invalid destructors from lookups.

2020-08-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/DeclBase.cpp:1489 return true; + if (isa(D) && D->isInvalidDecl()) +return true; split into separate block,

[PATCH] D86604: [clangd] Use string[] for allCommitCharacters

2020-08-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Oops, nice catch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86604/new/ https://reviews.llvm.org/D86604

[PATCH] D86597: [Tooling][Format] Treat compound extensions (foo.bar.cc) as matching foo.h

2020-08-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. sammccall added reviewers: krasimir, MyDeveloperDay. Herald added a project: clang. Herald added a subscriber: cfe-commits. sammccall requested review of this revision. Motivating use case is ".cu.cc" extensions used in some bazel projects. Alternative is to work

[PATCH] D82657: [AST][RecoveryAST] Preserve the type by default for recovery expression.

2020-08-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. New diagnostics are really good :-) Comment at: clang/test/SemaCXX/abstract.cpp:282 - void foo( C& c ) {} + void foo( C& c ) {} // expected-note {{candidate

[PATCH] D85716: [AST][RecoveryExpr] Fix a bogus unused diagnostic when the type is preserved.

2020-08-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/SemaCXX/recovery-expr-type.cpp:80 +namespace test6 { +struct Base { +private: nit: can the testcase be simplified? Isn't

[PATCH] D85635: [clangd] Compute the inactive code range for semantic highlighting.

2020-08-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. I think the code could be a bit clearer, but up to you. The tests are good so feel free to submit any version you're happy with. Comment at:

[PATCH] D85354: [clangd] Reduce availability of extract function

2020-08-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:281 + + // Make sure declarations inside extraction zone are not accessed afterwards. +

[PATCH] D85923: [clangd] Fix crash-bug in preamble indexing when using modules.

2020-08-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/unittests/TestTU.cpp:77 + CI.getHeaderSearchOpts().ModuleCachePath = ModuleCachePath.c_str(); + llvm::errs() << "MC: " << ModuleCachePath << "\n"; + llvm::errs().flush(); errs should

[PATCH] D83296: [clang-format] Add a MacroExpander.

2020-08-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:186 +Tok->MacroCtx = MacroContext(MR_ExpandedArg); + pushToken(Tok); +} klimek wrote: > sammccall wrote: > > you're pushing here without copying. This means the

[PATCH] D83296: [clang-format] Add a MacroExpander.

2020-08-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Somehow I missed the email from your replies. Mostly nits that you can take or leave, but I think potential bugs around functionlike-vs-objectlike and multiple-expansion of args. Comment at: clang/lib/Format/FormatToken.h:177 +/// EndOfExpansion:

[PATCH] D86069: [clang] When loading preamble from AST file, re-export modules in Sema.

2020-08-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Sorry, I was just not reading the test clearly. LGTM Comment at: clang/lib/Serialization/ASTReader.cpp:4990 PP.makeModuleVisible(Imported, Import.ImportLoc);

[PATCH] D86069: [clang] When loading preamble from AST file, re-export modules in Sema.

2020-08-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/test/PCH/preamble-modules.cpp:8 + +#ifndef MAIN_FILE +#define MAIN_FILE what are these ifdefs about? you never seem to define this symbol (Possible you're copying from a test that is doing something tricky like

[PATCH] D84387: [AST][RecoveryExpr] Suppress spurious "err_typecheck_expect_scalar_operand" diagnostic

2020-08-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Sorry for losing track of this. Where is CDependence defined? Is this stacked on another patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84387/new/ https://reviews.llvm.org/D84387

[PATCH] D83157: [clangd] Extract BackgroundIndex::Options struct. NFC

2020-08-13 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb36e22d64458: [clangd] Extract BackgroundIndex::Options struct. NFC (authored by sammccall). Changed prior to commit:

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

2020-08-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:120 + // clangd doesn't replay those when using a preamble. + "-llvm-header-guard"); + static const std::string CrashingChecks =

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

2020-08-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:113 + +// Set of clang-tidy checks that are not suitable to be run through clangd, +// either due to crashes

[PATCH] D85883: [clangd] Add ClangdServer::customAction() extension point

2020-08-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Yup sorry @adamcz, I was just confused. Likewise, happy to change to a different structure or revert entirely if you have concerns. 41d0edd54e29e994fa7d40961a38e8fca27addac eliminates dumpAST

[PATCH] D85883: [clangd] Add ClangdServer::customAction() extension point

2020-08-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Oops sorry, misread "mostly lgtm" and committed. Changed to use the "InputsAndAST" struct to provide the rest of the info. Happy to address any further comments... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85883/new/

[PATCH] D85883: [clangd] Add ClangdServer::customAction() extension point

2020-08-13 Thread Sam McCall via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG32626bcc0b9b: [clangd] Add ClangdServer::customAction() extension point (authored by sammccall). Changed prior to

[PATCH] D85883: [clangd] Add ClangdServer::customAction() extension point

2020-08-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. sammccall added reviewers: adamcz, kadircet. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous. Herald added a project: clang. sammccall requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. This lets basic

[PATCH] D85789: [Parser] Suppress -Wempty-translation-unit if this is a header file

2020-08-13 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc286d6fdeeb2: [Parser] Suppress -Wempty-translation-unit if this is a header file (authored by sammccall). Repository: rG LLVM Github Monorepo

[PATCH] D85834: [AST] Fix a crash on calling getASTRecordLayout on an invalid RecordDecl.

2020-08-12 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/ASTContext.cpp:2463 -unsigned PreferredAlign = static_cast( -toBits(getASTRecordLayout(RT->getDecl()).PreferredAlignment));

[PATCH] D85753: [clangd] Discard diagnostics from another SourceManager.

2020-08-12 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Yeah I think this is the right approach for now, because the cached modules don't include diagnostics and ensuring that they're persisted and available is a larger project.

[PATCH] D85789: [Parser] Suppress -Wempty-translation-unit if this is a header file

2020-08-11 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. sammccall added a reviewer: kadircet. Herald added subscribers: cfe-commits, usaxena95. Herald added a project: clang. sammccall requested review of this revision. Herald added a subscriber: ilya-biryukov. This is motivated by tooling (clangd, libclang etc) -

[PATCH] D85716: [AST][RecoveryExpr] Fix a bogus unused diagnostic when the type is preserved.

2020-08-11 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:353 return; +if (isa(E)) + return; This seems like a very specific patch to a special case of a potentially more general problem... Comment at:

[PATCH] D85635: [clangd] Compute the inactive code range for semantic highlighting.

2020-08-11 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Yup, this is a nice improvement, though there are further things we could do. As discussed offline, we could consider mapping "disabled" to an attribute but we can't really consume that well in VSCode (or any other editor yet) so let's leave it.

[PATCH] D83501: [clangd][ObjC] Improve xrefs for protocols and classes

2020-08-11 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Ugh, I forgot to hit submit and went on vacation :-\ Really sorry again. As much as we can simplify/unify the tests that helps, but let's not block on this anymore, up to you.

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

2020-08-10 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. Comment at: clang-tools-extra/clangd/unittests/TestFS.h:51 + // This is useful for testing module support. + bool OverlayRealFileSystem = false; }; also ForModules here?

[PATCH] D85426: [clangd] Implement FileFilter for the indexer

2020-08-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D85426#2199501 , @kbobyrev wrote: > @sammccall This is the change I was talking about during the standup: the > code looks legit, but `BackgroundIndexTests.IndexTwoFiles` fails because with > the following code path in

[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.

2020-08-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:997 +)cpp", +R"cpp( + class Foo {}; this is basically the same as

[PATCH] D84697: [clangd] Use elog instead of llvm::errs, log instead of llvm::outs

2020-07-28 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:202 + // Use buffered stream to stderr. + llvm::errs().SetBuffered();

[PATCH] D84697: [clangd] Use elog instead of llvm::errs, log instead of llvm::outs

2020-07-27 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for sending this... after looking at the examples and thinking again about our conversation I may have shifted a bit :-\ I think it's important *log() should only be used for messages intended to be logged! (i.e. read later as a sequence of log messages). We

[PATCH] D84499: [clangd] Add more logs and attach tracers to remote index server routines

2020-07-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:96 +} Counter(Request->DebugString()); +return std::forward(Call)( +*Index, ClangdRequest, [&](const StreamType ) { kbobyrev wrote: > sammccall

[PATCH] D83501: [clangd][ObjC] Improve xrefs for protocols and classes

2020-07-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. (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)

[PATCH] D83501: [clangd][ObjC] Improve xrefs for protocols and classes

2020-07-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:360 + dyn_cast(D)) { + // Objective-C implementation should map back to its interface. + D = IID->getClassInterface(); This just describes what the

[PATCH] D84499: [clangd] Add more logs and attach tracers to remote index server routines

2020-07-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:73 private: + template you don't need both RequestT and ClangdRequest as template params: - the things you're currently doing are all available through

[PATCH] D83814: [clangd] Add Random Forest runtime for code completion.

2020-07-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D83814#2166349 , @usaxena95 wrote: > This is still WIP and I will provide more details in the description once > this is finalized. This really should have high level documentation - I don't think Nathan will be the only

[PATCH] D83296: [clang-format] Add a MacroExpander.

2020-07-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Sorry for the long delay, I've made up for it with extra comments :-\ This looks really well-thought-out and I'm rationalizing my pickiness as: - this is conceptually complicated - I expect this code to live a long time and be read and "modified around" by lots of

[PATCH] D84222: [AST][RecoveryExpr] Error-dependent expression should not be treat as a nullptr pointer constant.

2020-07-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/Expr.cpp:3747 case NPC_ValueDependentIsNull: - if (isTypeDependent() || getType()->isIntegralType(Ctx)) + if

[PATCH] D84172: [clangd] Fix conversion from Windows UNC paths to file URI format.

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! Do you have commit access, or should I land this for you? Comment at: clang-tools-extra/clangd/URI.cpp:29 +bool isWindowsPath(llvm::StringRef Path) { +

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/index/remote/Client.cpp:68 +DeadlineWaitingTime(DeadlineTime) { +ProtobufMarshaller = std::unique_ptr( +

[PATCH] D84172: [clangd] Fix conversion from Windows UNC paths to file URI format.

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for doing this! And sorry about the shaky windows support... (There are potentially other lurking issues due to filenames being used as keys internally, particularly case-insensitivity issues...) Comment at:

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Herald added a subscriber: kbobyrev. In D83508#2157954 , @ArcsinX wrote: > In D83508#2157859 , @sammccall wrote: > > > Tried this out in D84012 /D84009 >

[PATCH] D84012: [clangd] Exclude preprocessed-to-nothing tokens from selection

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG72f2fb1db4ea: [clangd] Exclude preprocessed-to-nothing tokens from selection (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D84012: [clangd] Exclude preprocessed-to-nothing tokens from selection

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done. sammccall added inline comments. Herald added a subscriber: kbobyrev. Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:568 +#if 0 +void fu^nc(); +#endif kadircet wrote: > nit: i am not sure

[PATCH] D84009: [Syntax] expose API for expansions overlapping a spelled token range.

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf0ab336e7455: [Syntax] expose API for expansions overlapping a spelled token range. (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D84146: [AST][RecoveryExpr] Add recovery-ast tests for C language, NFC.

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/AST/ast-dump-recovery.c:20 +// CHECK-NEXT: `-DeclRefExpr {{.*}} 'a' +int prefix_inc = ++a; + why is this an interesting

[PATCH] D84140: [clang] Set the error-bit for ill-formed semantic InitListExpr.

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/include/clang/AST/Expr.h:4697 +assert(isSemanticForm()); +setDependence(getDependence() | ExprDependence::Error | + ExprDependence::ValueInstantiation); Hmm, actually hardcoding the

[PATCH] D84140: [clang] Set the error-bit for ill-formed semantic InitListExpr.

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaInit.cpp:965 } + if (hadError && FullyStructuredList) +FullyStructuredList->markError(); Can we have a

[PATCH] D83914: [clangd] Plan features for FoldingRanges

2020-07-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:329 + )cpp", + // Argument lists. + R"cpp( kbobyrev wrote: > sammccall wrote: > > missing lambdas, blocks, objc methods. > > cover bodies in

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd19f0666bcd8: [clang][Tooling] Try to avoid file system access if there is no record for theā€¦ (authored by ArcsinX, committed by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D83621#2153711 , @ArcsinX wrote: > As far as I do not have commit access, could you please commit for me? > Aleksandr Platonov Oops, missed this! Committed as d19f0666bcd8f7d26aaf4019244c3ed91e47b1b7

[PATCH] D83822: [clangd] Support config over LSP.

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. There are lots of choices and overlapping ideas for how the protocol should look, little urgency (it isn't going to make clangd 11), and few concrete use cases to evaluate the options. Let's table this for now and let the ideas sink in and more use cases crop up. I

[PATCH] D84009: [Syntax] expose API for expansions overlapping a spelled token range.

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 278748. sammccall added a comment. expansionsAffecting->expansionsOverlapping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84009/new/ https://reviews.llvm.org/D84009 Files:

[PATCH] D84009: [Syntax] expose API for expansions overlapping a spelled token range.

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked 4 inline comments as done. sammccall added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:281 + std::vector + expansionsAffecting(llvm::ArrayRef Spelled) const; kadircet wrote: > this sounds more like

[PATCH] D83822: [clangd] Support config over LSP.

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked 2 inline comments as done. sammccall added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1220 CDB->setCompileCommand(File, std::move(New)); ModifiedFiles.insert(File); } kadircet wrote: > nit: maybe

[PATCH] D83501: [clangd][ObjC] Improve xrefs for protocols and classes

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D83501#2154300 , @dgoldman wrote: > In D83501#2153605 , @sammccall wrote: > > > In D83501#2148671 , @dgoldman > > wrote: > > > > > I

[PATCH] D84012: [clangd] Exclude preprocessed-to-nothing tokens from selection

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 278717. sammccall marked 3 inline comments as done. sammccall edited the summary of this revision. sammccall added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84012/new/

[PATCH] D84009: [Syntax] expose API for expansions overlapping a spelled token range.

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 278716. sammccall marked 2 inline comments as done. sammccall added a comment. auto Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84009/new/ https://reviews.llvm.org/D84009 Files:

[PATCH] D84009: [Syntax] expose API for expansions overlapping a spelled token range.

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:431 +std::vector +TokenBuffer::expansionsAffecting(llvm::ArrayRef Spelled) const { + if (Spelled.empty()) ArcsinX wrote: > Will it be useful to have similar API with

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Tried this out in D84012 /D84009 . Works pretty well, and I think the API is a useful and natural addition to TokenBuffer. Maybe this is too much complexity though? (Mostly here i'm worrying about the

[PATCH] D84012: [clangd] Exclude preprocessed-to-nothing tokens from selectionThis prevents selection of empty preprocessor entities (like #define directives,or text in disabled sections) creating a s

2020-07-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. sammccall added reviewers: ArcsinX, kadircet. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Based on D83508 by Aleksandr Platonov. Repository: rG

  1   2   3   4   5   6   7   8   9   10   >