[PATCH] D138505: [clangd] Don't run slow clang-tidy checks by default

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb99f7e695469: [clangd] Dont run slow clang-tidy checks by default

[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

sammccall added a comment. Hi, I'm afraid this breaks valid code with false diagnostics, I need to revert. Specifically it complains about missing lambda captures for values that are in fact arguments to the lambda: template constexpr void inner(F f) { return f(0); } template

[PATCH] D153485: [dataflow] use true/false literals in formulas, rather than variables

This revision was automatically updated to reflect the committed changes. Closed by commit rG36bd5bd888f1: [dataflow] use true/false literals in formulas, rather than variables (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D153485?vs=537304=557245#toc Repository:

[PATCH] D159450: Initialize `ConceptReference` of new `AutoTypeLoc` with nullptr.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Days since learning something new and horrifying about TypeLoc: zero You might consider deleting the '= nullptr' from `AutoTypeLocInfo::CR` since it appears we never actually run the

[PATCH] D159299: [clangd] Support ConceptReference in generic AST wrangling code

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa8c9b9f1402c: [clangd] Support ConceptReference in generic AST wrangling code (authored by sammccall). Changed prior to commit:

[PATCH] D159300: [AST] Support ConceptReference in DynTypedNode, add dump().

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGda5e11cd1ce0: [AST] Support ConceptReference in DynTypedNode, add dump(). (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D159309: [ASTMatchers] basic matcher support for ConceptReference

sammccall updated this revision to Diff 555709. sammccall added a comment. regenerate HTML after rebasing on 415d9e8ca39c0b42f351cc532ccfb48b6ac97f7f Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D159338: [clangd][tests] Bump timeouts in TUSchedulerTests to 60 secs

sammccall added a comment. In D159338#4635010 , @hiraditya wrote: > not sure if this is the right way to fix these tests. The problem is if a > device is constraiend, this will further slow down the device and create more > backlogs. Can we allow a way

[PATCH] D159363: [clangd] SIGSEGV at clangd: DiagnosticConsumer Is Used After Free

sammccall added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:610-619 StoreDiags PreambleDiagnostics; PreambleDiagnostics.setDiagCallback( [](const clang::Diagnostic , clangd::Diag ) { llvm::for_each(ASTListeners,

[PATCH] D159363: [clangd] SIGSEGV at clangd: DiagnosticConsumer Is Used After Free

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Nice catch! Comment at: clang-tools-extra/clangd/Preamble.cpp:709 Ctx->setStatCache(Result->StatCache); + // We have to setup DiagnosticConsumer that will

[PATCH] D158967: [clang][clangd] Ensure the stack bottom before building AST

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! Sorry about the layering mess... Comment at: clang-tools-extra/clangd/support/Threading.cpp:11 #include "support/Trace.h" +#include "clang/Basic/Stack.h"

[PATCH] D158407: [clang][dataflow] #llvm #flow-analysis Simplify formula at CNF construction time, and short-cut solving of known contradictory formulas.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp:153 /// All literals in the input that are not `NullLit` must be distinct. - void

[PATCH] D159309: [ASTMatchers] basic matcher support for ConceptReference

sammccall created this revision. sammccall added reviewers: massberg, aaron.ballman. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This adds only the trivial conceptReference() matcher. (Also adds

[PATCH] D122529: [ASTMatchers] Output currently matching node on crash

sammccall added a comment. Herald added a subscriber: wangpc. In D122529#3422158 , @aaron.ballman wrote: > This looks good? to me. :-) > > Despite this complicating things by a fair amount, I don't have a better > suggestion to offer. LGTM

[PATCH] D158967: [clang][clangd] Ensure the stack bottom before building AST

sammccall added inline comments. Comment at: clang-tools-extra/clangd/support/Threading.cpp:102 llvm::set_thread_name(Name); +// Mark the bottom of the stack for clang to be aware of the stack usage and +// prevent stack overflow. ugh, I forgot:

[PATCH] D159300: [AST] Support ConceptReference in DynTypedNode, add dump().

sammccall created this revision. sammccall added a reviewer: massberg. Herald added a subscriber: kadircet. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. The dump() is not actually

[PATCH] D159299: [clangd] Support ConceptReference in generic AST wrangling code

sammccall created this revision. sammccall added a reviewer: massberg. Herald added subscribers: kadircet, arphaman. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.

[PATCH] D159268: [c++20][clangd] Simplify code using the new `ConceptReference` nodes.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/FindTarget.cpp:1059 - bool TraverseTypeConstraint(const TypeConstraint *TC) { -// We want to handle all

[PATCH] D158842: [include-cleaner] Handle decls/refs to concepts.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LGTM Unclear to me whether the FIXME will be addressed by an AST bug or a local change, either way continuing to look into it SG but this is fine to land as-is. Repository: rG LLVM

[PATCH] D158842: [include-cleaner] Handle decls/refs to concepts.

sammccall added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:244 + bool VisitConceptDecl(ConceptDecl *CD) { +report(CD->getLocation(), CD); I don't know why we're doing this, decls in general are not considered

[PATCH] D159263: [clang-tidy] misc-include-cleaner: remove duplicated includes & fixes

sammccall added a comment. Thanks! @kadircet should make the call on these features (they look good to me), I have some implementation notes. This is making two independent changes, one is a policy change that should be applied to the include-cleaner library, and one is a bugfix to the

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

sammccall added a comment. In D153114#4630318 , @ChuanqiXu wrote: > @sammccall @nridge while I am looking for the initial support for modules in > clangd, I failed to find the mechanism to update files after I update a > header file. > > e.g., when I

[PATCH] D155858: Add a concept AST node.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LGTM, just nits! The SourceRanges are actually correct :-) Comment at: clang/include/clang/AST/ASTConcept.h:124 class ConceptReference { protected: // \brief The

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

sammccall added a comment. In D158872#4626095 , @aaron.ballman wrote: > Out of curiosity, are you testing on Windows with MSVC? My understanding on > build time performance impacts was that it's debug builds with MSVC that get > hit especially hard.

[PATCH] D158967: [clang][clangd] Ensure the stack bottom before building AST

sammccall added a comment. In D158967#4626156 , @rsmith wrote: > Adding another fallback in `ASTFrontendAction::ExecuteAction` seems OK to me > if that's an entry point that's commonly used by tools. (Where is that being > called from in clangd's case?

[PATCH] D155858: Add a concept AST node.

sammccall added a comment. OK, so I learned a bit about initializeLocal/trivial TypeSourceInfo. Mostly things I didn't want to know :-) TL;DR I think the current version of the patch is right. The crash is caused by: 1. `CheckTemplateArgument` calls `SubstType` without passing a `TypeLoc`,

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

sammccall added a comment. I did some testing on my machine (host compiler clang-14), and I'm not sure the specifics justify the level of caution here: ASTMatchers.h parses in 6.5s (with `clang-check`) If I comment out the code + internal ASTMatcher includes, then just the deps (mostly AST

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

sammccall added a comment. Thanks for the history, I wasn't aware this was such a big deal for such a long time! In D158872#462 , @aaron.ballman wrote: >> A few ideas: >> >> - could we move these (and possibly other rarely-used matchers) to >>

[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

sammccall added a comment. In D158872#4624821 , @aaron.ballman wrote: > In D158872#4623193 , @danix800 > wrote: > >> In D158872#4622124 , >> @aaron.ballman wrote: >>

[PATCH] D155858: Add a concept AST node.

sammccall added inline comments. Comment at: clang/lib/AST/TypeLoc.cpp:625 -DeclarationNameInfo AutoTypeLoc::getConceptNameInfo() const { - return DeclarationNameInfo(getNamedConcept()->getDeclName(), - getLocalData()->ConceptNameLoc);

[PATCH] D158842: [include-cleaner] Handle decls/refs to concepts.

sammccall added a comment. I think we're not *that* far away from landing the AST change, and changing RecursiveASTVisitor now is probably confusing and not worth the churn. So I'd prefer to wait for now... Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:240 + +

[PATCH] D153152: Adds tweak to add declarations for pure virtuals

sammccall added a comment. In D153152#4617391 , @robot wrote: >> so you could replace the logic that prints the return type + arg list with >> `printType(functype, class, methodname)` (from AST.h). That should pick up >> noexcept etc. > > I've tried it

[PATCH] D158873: [clangd] Do not ignore qualifier in heuristic resolution of dependent MemberExpr

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Oh, this is surprising! Comment at: clang-tools-extra/clangd/HeuristicResolver.cpp:142 +// Do not proceed to try resolving the member in the expression's base type

[PATCH] D158851: [clangd] Avoid null result in FindRecordTypeAt()

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! (Both assigned reviewers are currently out on vacation) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158851/new/

[PATCH] D154382: [ClangRepl] support code completion at a REPL

sammccall added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:845 case CodeCompletionContext::CCC_NewName: + case CodeCompletionContext::CCC_TopLevelOrExpression: return false; This should be `true` rather than `false`, since

[PATCH] D158766: [AST] TypeSourceInfo for params of inherited constructor should be null

sammccall updated this revision to Diff 553234. sammccall marked 2 inline comments as done. sammccall added a comment. address review comments, also null the TSI of the constructor itself Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158766/new/

[PATCH] D158766: [AST] TypeSourceInfo for params of inherited constructor should be null

sammccall added a reviewer: aaron.ballman. sammccall marked 4 inline comments as done. sammccall added a subscriber: aaron.ballman. sammccall added a comment. +Aaron for guidance & in case I'm making things up, TypeLocs/TSI have always confused me... Comment at:

[PATCH] D158766: [AST] TypeSourceInfo for params of inherited constructor should be null

sammccall created this revision. sammccall added a reviewer: ymandel. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This matches behavior of other synthetic members like copy constructors, and

[PATCH] D158407: #llvm #flow-analysis Simplify formula at CNF construction time, and short-cut solving of known contradictory formulas.

sammccall added a comment. This simplification makes sense, but if we're adding the layer, we're missing an opportunity to apply it completely. (Intuitively, I don't see any reason that two passes is "enough") To do this, we'd want to simplify existing formulas when we learn something new.

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

sammccall added a comment. Aside: I've been doing some investigation into how modules+clangd could work in our huge monorepo (specifically bazel + distributed build cluster). It looks feasible (with some serious effort) to get all BMI/index/etc data we need for transitive modules to be

[PATCH] D158515: [include-cleaner] Make handling of enum constants similar to members

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:135 +} +// For refs to member-like decls, report an implicit ref to the container. +if

[PATCH] D158426: [clangd] Bump timeouts for LSPServerTests

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/unittests/LSPClient.cpp:42 [this] { return Value.has_value(); })) { ADD_FAILURE() << "No result from

[PATCH] D155858: Add a concept AST node.

sammccall added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:6824-6836 + unsigned size = TL.getTypePtr()->getTypeConstraintArguments().size(); + TemplateArgumentLocInfo *TALI = new TemplateArgumentLocInfo[size]; +

[PATCH] D158439: [Lex] Preambles should contain the global module fragment.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG23459f13fcd9: [Lex] Preambles should contain the global module fragment. (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D158426: [clangd] Bump timeouts for LSPServerTests

sammccall added inline comments. Comment at: clang-tools-extra/clangd/unittests/LSPClient.cpp:27 std::unique_lock Lock(Mu); - if (!clangd::wait(Lock, CV, timeoutSeconds(10), [this] { return Value.has_value(); })) { I think we're better

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

sammccall added a comment. In D153114#4604438 , @nridge wrote: > In D153114#4603579 , @sammccall > wrote: > >> - to identify what module names we must have PCMs for in order to build a >> given TU (either an

[PATCH] D158439: [Lex] Preambles should contain the global module fragment.

sammccall created this revision. sammccall added a reviewer: ChuanqiXu. Herald added subscribers: kadircet, arphaman. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added projects: clang, clang-tools-extra.

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

sammccall added a comment. In D153114#4602414 , @ChuanqiXu wrote: >> Don't attempt any cross-file or cross-version coordination: i.e. don't try >> to reuse BMIs between different files, don't try to reuse BMIs between >> (preamble) reparses of the same

[PATCH] D153152: Adds tweak to add declarations for pure virtuals

sammccall added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp:58 + + const llvm::ArrayRef Tokens = + TokBuf.expandedTokens(MethodDeclRange); robot wrote: > sammccall wrote: > > copying the tokens directly

[PATCH] D158093: [Sema] Clean up ActionResult type a little. NFCI

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGefe4a54884cb: [Sema] Clean up ActionResult type a little. NFCI (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D158093: [Sema] Clean up ActionResult type a little. NFCI

sammccall marked 2 inline comments as done. sammccall added inline comments. Comment at: clang/include/clang/Sema/Ownership.h:204 + PtrTy get() const { +void *VP = reinterpret_cast(Value & ~0x01); +return PtrTraits::getFromVoidPointer(VP); kadircet

[PATCH] D154382: [ClangRepl] support code completion at a REPL

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang/include/clang/Sema/CodeCompleteConsumer.h:341 + +/// Code completion at a top level in a REPL session. +

[PATCH] D149236: [clangd] Bail gracefully if given an assembly or IR source file

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. In D149236#4597600 , @nridge wrote: > Adding Sam, since you're on a review roll ;) back from vacation :-) Comment at:

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

sammccall added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:466 +// so that we can recover argument names from it. +// FIXME: This function is mostly duplicated in SemaCodeComplete.cpp; unify. +static FunctionProtoTypeLoc getPrototypeLoc(Expr *Callee) {

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/InlayHints.cpp:508 + // Only one of Callee or ProtoTypeLoc is set. + const FunctionDecl *Callee = nullptr; +

[PATCH] D158248: [clangd] Fix incorrect RecursiveASTVisitor usage in summarizeExpr()

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Oops, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158248/new/ https://reviews.llvm.org/D158248

[PATCH] D157905: [include-cleaner] Filter references to identity macros

sammccall accepted this revision. sammccall added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:44 + // not the macro definition. So ignore such macros. + return MI && MI->getNumTokens() == 1 && MI->isObjectLike() && +

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

sammccall added a comment. Sorry for the long radio silence here. There's a lot to chew on, and I put it off too long. Thanks for your patience! I agree we should get experimental modules support landed in some form on the LLVM 18 timeline. It's fine if this doesn't have extension points for

[PATCH] D151315: [clangd] Add a switch to specify a default clangd configuration file

sammccall added a comment. Sorry about the delay, I think this is OK to add. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:500 +"User config is from clangd/config.yaml in the following directories \n" +"(unless specified with --default-config):\n"

[PATCH] D153152: Adds tweak to add declarations for pure virtuals

sammccall added a comment. Thanks for working on this! FWIW I had an old draft of this feature that may have interesting ideas: https://reviews.llvm.org/D122827 Comment at: clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp:44 +for (const auto : +

[PATCH] D158093: [Sema] Clean up ActionResult type a little. NFCI

sammccall created this revision. sammccall added reviewers: kadircet, aaron.ballman. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. - Document the valid states: invalid/unset/pointer (Currently both

[PATCH] D154853: [clangd][c++20]Consider the constraint of a constrained auto in FindTarget.

sammccall requested changes to this revision. sammccall added a comment. This revision now requires changes to proceed. (This will look different/better after D155858 , taking it off my radar until then) Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Agree this is a mess: the reason we think objective-c++-header is safe is that we have built-in support, but in the presence of query-driver that's not enough. In terms of testing this:

[PATCH] D156441: [clangd]Fix addUsing tweak doesn't traverse same-level anon namespaces

sammccall added a comment. This would be a great fix to have! However I don't think the specific changes to the RecursiveASTVisitor are correct. I can see a couple of approaches: 1. prevent the RecursiveASTVisitor from traversing into uninteresting contexts, and drop the Encloses check in

[PATCH] D156605: [clangd][CodeComplete] Improve FunctionCanBeCall

sammccall added a comment. Thanks, this does look better. I agree with Nathan's comments and will leave final stamp to him. Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:144 + + // This variable would be discarded directly at the end of this function. We +

[PATCH] D155858: Add a concept AST node.

sammccall added a comment. (There are still outstanding comments e.g. in ASTImport) I think it would be useful to add to the patch description: - the current deficiencies of ConceptReference that make it not a well-behaved AST node now - which of these are addressed in this patch

[PATCH] D157905: [include-cleaner] Filter references to identity macros

sammccall accepted this revision. sammccall added a comment. still LG, comments are still confusing me a little Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:43 + // This results in surprising behavior from users point of view (we + // generate a

[PATCH] D157952: [clang] Support function pointer types with attributes when extracting parameter names for signature help

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:6073 + if (auto A = Target.getAs()) { +Target = A.getModifiedLoc(); By unwrapping these in

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

sammccall accepted this revision. sammccall added a comment. Still LG once you're happy Comment at: clang/include/clang/Sema/Sema.h:3026 + void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc, + ExprResult DefaultArg);

[PATCH] D157905: [include-cleaner] Filter references to identity macros

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:37 +namespace { +bool shouldSkipMacro(const Macro ) { + static const auto *MacroNamesToIgnore = new

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Nice, recoveryexpr is a better fit here. These changes tend to cause occasional new error-handling crashes on under-tested paths, but I guess it's a good time in the release cycle for

[PATCH] D157080: [clangd] Symbol search includes parameter types for (member) functions

sammccall added a comment. LSP is fairly clear here and our behavior here seems to be pretty correct. - `name` is "the name of this symbol", - detail is "More detail for this symbol, e.g the signature of a function.". We're providing the type as detail: `void()` (for functions just `()` might

[PATCH] D154382: [ClangRepl] support code completion at a REPL

sammccall added a comment. This mostly looks good. My main concern is adding code to the parser to suppress errors when code completing: this is unlikely to be the only such place. Existing consumers seem happy to ignore errors, if this doesn't work for clang-repl it'd be good to understand

[PATCH] D156781: Use the canonical path for the include header search.

sammccall added a comment. I'm out on vacation the next two weeks, happy to look once back, otherwise @kadircet would be the person to review this on the clangd side. A couple of general notes: - people use symlinks in different ways, to give files multiple names, the idea that we can tell

[PATCH] D155878: [clangd] Loose include-cleaner matching for verbatim headers

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sammccall marked 2 inline comments as done. Closed by commit rGd97a3419c0a3: [clangd] Loose include-cleaner matching for verbatim headers (authored by sammccall).

[PATCH] D155878: [clangd] Loose include-cleaner matching for verbatim headers

sammccall marked 4 inline comments as done. sammccall added inline comments. Comment at: clang-tools-extra/clangd/Headers.h:179 + // All paths are canonical (FileManager::getCanonicalPath()). + std::vector SearchPathCanonical; + kadircet wrote: >

[PATCH] D155819: [include-cleaner] Loose matching for verbatim headers

This revision was automatically updated to reflect the committed changes. Closed by commit rG16b5e1897b7e: [include-cleaner] Loose matching for verbatim headers (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155819/new/

[PATCH] D153058: [clang][CFG] Support construction of a weak topological ordering of the CFG.

sammccall accepted this revision. sammccall added a comment. Thanks, this is much simpler! Just nits & apologies for the delay. Comment at: clang/include/clang/Analysis/Analyses/IntervalPartition.h:24 #include "llvm/ADT/DenseSet.h" +#include +#include some

[PATCH] D155878: [clangd] Loose include-cleaner matching for verbatim headers

sammccall added inline comments. Comment at: clang-tools-extra/clangd/Headers.cpp:185 + // The entries will be the same, but canonicalizing to find out is expensive! + if (SearchPathCanonical.empty()) { +for (const auto : kadircet wrote: > nit: early exit

[PATCH] D155819: [include-cleaner] Loose matching for verbatim headers

sammccall added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Types.cpp:110-112 + while (!P.empty() && path::is_separator(P.back())) +P.pop_back(); + return P; kadircet wrote: > nit: `return P.rtrim('/');` // only separator we can

[PATCH] D155819: [include-cleaner] Loose matching for verbatim headers

sammccall updated this revision to Diff 544323. sammccall marked 6 inline comments as done. sammccall added a comment. address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155819/new/ https://reviews.llvm.org/D155819 Files:

[PATCH] D155885: [include-cleaner] Switch Include from FileEntry* -> FileEntryRef

This revision was automatically updated to reflect the committed changes. Closed by commit rGf6307b260b0c: [include-cleaner] Switch Include from FileEntry* - FileEntryRef (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155819: [include-cleaner] Loose matching for verbatim headers

sammccall added a comment. (ping - also happy to send this to someone else if you prefer!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155819/new/ https://reviews.llvm.org/D155819 ___ cfe-commits

[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction

This revision was automatically updated to reflect the committed changes. Closed by commit rG0478ef2d366c: [clangd] Exclude builtin headers from system include extraction (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D156230: [clang][dataflow][NFC] Eliminate variable only used in assertion.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. (if you prefer to keep the named variable, `(void)StructVal2` is also fine) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156230/new/

[PATCH] D155992: [clangd] Use xxh3_64bits for background index file digests

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Agree - LGTM but please land once the branch `release/17.x` exists. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155992/new/

[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction

sammccall updated this revision to Diff 543291. sammccall added a comment. fix incomplete cleanup of dead option Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156044/new/ https://reviews.llvm.org/D156044 Files:

[PATCH] D154903: clangd: Provide the resource dir via environment variable

sammccall added a comment. D156044 attempts to have clangd always exclude the builtin headers when querying the driver. Would be great to know if this fixes your problem. (But if you're not in a position to try a patch like that, filtering the path out in

[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction

sammccall updated this revision to Diff 543253. sammccall added a comment. remove (broken) support for -nobuiltininc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156044/new/ https://reviews.llvm.org/D156044 Files:

[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction

sammccall created this revision. sammccall added reviewers: kadircet, madscientist. Herald added a subscriber: arphaman. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project:

[PATCH] D154903: clangd: Provide the resource dir via environment variable

sammccall added a comment. TL;DR: can you try just excluding GCC's resource dir, rather than replacing it with clang's? I think that might solve your problem, and is probably something we should be doing automatically. In D154903#4525329 ,

[PATCH] D155992: [clangd] Use xxh3_64bits for background index file digests

sammccall added a comment. These hashes are used in the filenames of index shards, and in the metadata describing dependencies between shards. So this is an incompatible change to indexing format, you also need to increment `constexpr static uint32_t Version` in Serialization.cpp:460. (I

[PATCH] D155671: [include-cleaner] allow spelling strategies to customize verbatim/system headers

This revision was automatically updated to reflect the committed changes. Closed by commit rGab4e461353bb: [include-cleaner] allow spelling strategies to customize verbatim/system headers (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

sammccall added a comment. In D155540#4524219 , @MaskRay wrote: > `llvm/cmake/modules/HandleLLVMOptions.cmake` passes `-Wl,-z,defs`. > `-DBUILD_SHARED_LIBS=on` builds get checking from the linker option >

[PATCH] D155381: [clangd] Allow indexing of __reserved_names outside system headers

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG290a98c7b008: [clangd] Allow indexing of __reserved_names outside system headers (authored by sammccall). Changed prior to commit:

[PATCH] D153946: [clangd] Add a flag to allow indexing of reserved identifiers

sammccall added a comment. In D153946#4521576 , @nridge wrote: > Yeah, I'm happy to go with D155381 instead. > > In D153946#4503585 , @sammccall > wrote: > >> (Stupid

[PATCH] D155898: [clangd] Fix go-to-type target location

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rGd9d9a2cb2f0d: [clangd] Use index for go-to-type (authored by sammccall). Changed prior to commit:

[PATCH] D155898: [clangd] Fix go-to-type target location

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:347 +void enhanceLocatedSymbolsFromIndex( +llvm::MutableArrayRef Result, +const llvm::DenseMap , usaxena95 wrote: > nit:

[PATCH] D155421: [clangd] Add BlockEnd comments for control flow statements

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rGee032bccc934: [clangd] Add BlockEnd comments for control flow statements (authored by sammccall).

[PATCH] D155421: [clangd] Add BlockEnd comments for control flow statements

sammccall marked 3 inline comments as done. sammccall added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:252 +// This is used to summarize e.g. the condition of a while loop. +std::string summarizeExpr(const Expr *E) { + struct Namer : ConstStmtVisitor {

  1   2   3   4   5   6   7   8   9   10   >