[PATCH] D155619: [clangd] Always run preamble indexing on a separate thread

2023-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This has bee

[PATCH] D155173: [clangd] Refine the workflow for diagnostic Fixits.

2023-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1063 + OnlyFix->isPreferred = true; + if (LSPDiags.size() == 1 && LSPDiags.front().range == Sel

[PATCH] D155173: [clangd] Refine the workflow for diagnostic Fixits.

2023-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1014-1016 + llvm::ArrayRef LSPDiags = Params.context.diagnostics; + std::map + ToLSPDiagsIndex; we're already making a copy of `Params.context.diagnostics` when creati

[PATCH] D155173: [clangd] Refine the workflow for diagnostic Fixits.

2023-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:671 + // FIMXE: this is tricky + llvm::StringRef(LSPDiag.Message) + .starts_with_insensitive(Diag.Message)) hokein wrote: > hokein wro

[PATCH] D152900: [clangd] Update symbol collector to use include-cleaner.

2023-07-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks a lot for bearing with me, let's ship it! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:91 +const Inclusion &Inc, ParsedAST &AST, +std::shared_pt

[PATCH] D155215: [clangd] Fix the range for include reference to itself.

2023-07-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/IncludeCleaner.h:87 + +// Returns the range starting at '#' and ending at EOL. Escaped newlines are not +// handled. ---

[PATCH] D155173: [clangd] Refine the workflow for diagnostic Fixits.

2023-07-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:671 + // FIMXE: this is tricky + llvm::StringRef(LSPDiag.Message) + .starts_with_insensitive(Diag.Message)) this is tricky indeed and

[PATCH] D155195: [include-cleaner] Avoid a caching issue when running --edit mode on multiple files.

2023-07-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155195/new/ https://reviews.llvm.org/D155195

[PATCH] D154963: [Format][Tooling] Fix HeaderIncludes::insert not respect the main-file header.

2023-07-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:369-370 + QuotedName, /*CheckMainHeader=*/!MainIncludeFound); + if (Priority == 0) +MainIncludeFound = true; auto CatOffset = CategoryEndOffsets.find(Priority); --

[PATCH] D154963: [Format][Tooling] Fix HeaderIncludes::insert not respect the main-file header.

2023-07-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:338 int Priority = Categories.getIncludePriority( -CurInclude.Name, /*CheckMainHeader=*/FirstIncludeOffset < 0); +CurInclude.Name, /*CheckMainHeader=*/true); Cate

[PATCH] D145302: [clangd] Add library for clangd main function

2023-07-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145302/new/ https://reviews.llvm.org/D145302 _

[PATCH] D154962: [clangd] Use canonical path as resolved path for includes.

2023-07-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/Headers.cpp:57-61 + auto CanonicalPath = + File ? getCanonicalPath(File->getFileEntry().getLastRef(), +

[PATCH] D154950: [include-cleaner] Fix the `fixIncludes` API not respect main-file header.

2023-07-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp:303 - EXPECT_EQ(fixIncludes(Results, Code, format::getLLVMStyle()), R"cpp( + EXPECT_EQ(fi

[PATCH] D152900: [clangd] Update symbol collector to use include-cleaner.

2023-07-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:85 auto Task = [FIndex(FIndex), Path(Path.str()), Version(Version.str()), - ASTCtx(std::move(ASTCtx)), - CanonIncludes(CanonIncludes)]() mutable { +

[PATCH] D154349: [include-cleaner] Add a signal to down-rank exporting headers

2023-07-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5933d265b72a: [include-cleaner] Add a signal to down-rank exporting headers (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154349/new/

[PATCH] D154477: [include-cleaner] Ignore the layering-violation errors for the standalone tool

2023-07-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp:99 + bool BeginInvocation(CompilerInstance &CI) override { +// Disable the clang-module-bas

[PATCH] D153340: [include-cleaner] Add an IgnoreHeaders flag to the command-line tool.

2023-07-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:138 + /// Returns the path (without surrounding quotes/brackets) for the header.

[PATCH] D154473: [clang][Tooling] Add mapping for make_error_code

2023-07-05 Thread Kadir Cetinkaya 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 rG80c679220038: [clang][Tooling] Add mapping for make_error_code (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D154473?v

[PATCH] D154473: [clang][Tooling] Add mapping for make_error_code

2023-07-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D154473 Files: clang

[PATCH] D154443: [clangd] Downgrade deprecated warnings to hints

2023-07-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa59b24be47ed: [clangd] Downgrade deprecated warnings to hints (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154443/new/ https://revi

[PATCH] D154443: [clangd] Downgrade deprecated warnings to hints

2023-07-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This tries t

[PATCH] D153340: [include-cleaner] Add an IgnoreHeaders flag to the command-line tool.

2023-07-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:67 /// Determine which headers should be inserted or removed from the main file. /// This exposes conclusions but not reasons: use lower-level walkUsed for t

[PATCH] D154335: [clang][tooling] Fix early termination when there are nested expansions

2023-07-03 Thread Kadir Cetinkaya 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 rG9841daf27076: [clang][tooling] Fix early termination when there are nested expansions (authored by kadircet). Repository: rG LLVM Github Monorepo

[PATCH] D154349: [include-cleaner] Add a signal to down-rank exporting headers

2023-07-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Currently exporter can have same relevance signals as the origin header whe

[PATCH] D154335: [clang][tooling] Fix early termination when there are nested expansions

2023-07-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added projects: clang, clang-tools-extra. Herald added a subscriber: cfe-commits. This also does some cleanups,

[PATCH] D153882: [clangd] Always allow diagnostics from stale preambles

2023-06-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rGbd89f9ec293e: [clangd] Always allow diagnostics from stale preambles (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D153882?vs=53499

[PATCH] D153882: [clangd] Always allow diagnostics from stale preambles

2023-06-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigYAML.cpp:137 Dict.handle("ClangTidy", [&](Node &N) { parse(F.ClangTidy, N); }); -Dict.handle("AllowStalePreamble", [&](Node &N) { - F.AllowStalePrea

[PATCH] D150978: [clang][Sema] Add CodeCompletionContext::CCC_ObjCClassForwardDecl

2023-06-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150978/new/ https://reviews.llvm.org/D150978 _

[PATCH] D153882: [clangd] Always allow diagnostics from stale preambles

2023-06-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 534995. kadircet added a comment. - Squash base commit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153882/new/ https://reviews.llvm.org/D153882 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp clang-

[PATCH] D153882: [clangd] Always allow diagnostics from stale preambles

2023-06-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: arphaman, javed.absar. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. We'

[PATCH] D152686: [clangd] Enforce strict unused includes by default

2023-06-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D152686#4449481 , @cjdb wrote: > This patch seems to have broken clangd for a project of mine. Most of my > headers are now flagged as not being directly used even though they're the > root header for the symbol. replied on

[PATCH] D152720: [clangd][ObjC] Support ObjC class rename from implementation decls

2023-06-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:144 +const NamedDecl *pickInterestingTarget(const NamedDecl *D) { + // We only support renaming the cl

[PATCH] D152900: [clangd] Update symbol collector to use include-cleaner.

2023-06-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:846 +void SymbolCollector::setSymbolProviders( +const Symbol &S, const llvm::SmallVector Headers) { + if (Opts.CollectIncludePath && kadircet wrote: > what about

[PATCH] D147034: [clangd] Replace the hacky include-cleaner macro-reference implementation.

2023-06-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:360 +for (const auto &Ref : Refs) { + SourceLocation Loc = SM.getLocForStartOfFile(SM.getMainFileID()

[PATCH] D152720: [clangd][ObjC] Support ObjC class rename from implementation decls

2023-06-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:164 AST.getHeuristicResolver())) { Result.insert(canonicalRenameDecl(D)); } before calling `canonicalRenameDecl` here we can do a `D = pickInteres

[PATCH] D145302: [clangd] Add library for clangd main function

2023-06-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks LG and seems to be working in a couple build configurations I tried. but there might still be breakages in different configs, so please be on the watchout after landing this for breakages in https://lab.llvm.org/. FWIW something like: # Needed by LLVM's CMake

[PATCH] D153259: [clangd] Store offsets in MacroOccurrence

2023-06-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/CollectMacros.cpp:37 Out.Names.insert(Name); - auto Range = halfOpenToRange( - SM, CharSourceRange::getCharRan

[PATCH] D153340: [include-cleaner] Add an IgnoreHeaders flag to the command-line tool.

2023-06-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp:54 +"ignore-headers", +cl::desc("A comma-separated list of headers to ignore."), +cl::init(""), `A comma-separated list of regexes to match against s

[PATCH] D152900: [clangd] Update symbol collector to use include-cleaner.

2023-06-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:628 CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts()); - std::unique_ptr IWYUHandler = - collectIWYUHeaderMaps(&CanonIncludes); can you also add a FIXME here s

[PATCH] D150978: [clang][Sema] Add CodeCompletionContext::CCC_ObjCClassForwardDecl

2023-06-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:407 +// reference the declaration. +if (ContextKind == CodeCompletionContext::CCC_ObjCClassForwardDecl) + ShouldInsert = false; rather than doing this here, can you

[PATCH] D153330: [include-cleaner] Ignore the ParmVarDecl itself in WalkAST.cpp

2023-06-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:219 +// as they don't contribute to the main-file #include. +if (isa(VD)) + return true

[PATCH] D152720: [clangd][ObjC] Support ObjC class rename from implementation decls

2023-06-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:715 + +void VisitObjCImplementationDecl(const ObjCImplementationDecl *OIMD) { + if (const auto *CI = OIMD->getClassInterface()) can you also add tests for these into `Fin

[PATCH] D152900: [clangd] Update symbol collector to use include-cleaner.

2023-06-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:393-395 +const auto *FileEntry = SM.getFileEntryForID(FID); +for (const auto *Export : PI.getExporters(FileEntry, SM.getFileManager())) + return toURI(Export->tryGetRealPat

[PATCH] D152274: [clang] Don't create import decls without -fmodules

2023-06-16 Thread Kadir Cetinkaya 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 rG4d0cfa6d09e2: [clang] Don't create import decls without -fmodules (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D15227

[PATCH] D152801: [include-cleaner] Don't apply PreferredHeader hings for standard headers.

2023-06-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152801/new/ https://reviews.llvm.org/D152801

[PATCH] D152686: [clangd] Enforce strict unused includes by default

2023-06-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rGea20f339d91f: [clangd] Enforce strict unused includes by default (authored by kadircet). Changed prior to

[PATCH] D152685: [clangd] Decouple IncludeCleaner implementation from Config

2023-06-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG031ffc3e0645: [clangd] Decouple IncludeCleaner implementation from Config (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D152685?vs=

[PATCH] D152687: [clangd] Turn on missing-include diags by default

2023-06-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. By disabling include-cleaner analysis for Diagnost

[PATCH] D152686: [clangd] Enforce strict unused includes by default

2023-06-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: ChuanqiXu, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Depen

[PATCH] D152685: [clangd] Decouple IncludeCleaner implementation from Config

2023-06-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This should hel

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-06-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks a lot for bearing with me, LGTM! let me know if i should land this for you. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:165 bool CollectInactiveRegio

[PATCH] D149437: [clangd] Emit ChangeAnnotation label and description for include-cleaner diagnostics.

2023-06-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks, mostly LG. I think we need to be a little careful when generating insertions for all the missing includes though. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:169 -std::vector generateMissingIncludeDiagnostics( +using MissingInclu

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks, mostly LG. some nits around comments and request for a knob :) Comment at: clang-tools-extra/clangd/ClangdServer.cpp:80 + +auto Task = [FIndex(FIndex), Path(Path.str()), Version(Version.str()), + ASTCtx(std::move(ASTCtx)), -

[PATCH] D152345: [include-cleaner] Report all specializations if the primary template is introduced by a using-decl.

2023-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:176 + void reportSpecializations(SourceLocation Loc, NamedDecl *ND) { +if (const auto *TD = dyn

[PATCH] D143260: [clangd] Add semantic token for labels

2023-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. alright then, this patch LGTM. going forward let's try to introduce new kinds (or handling for different semantic constructs) in a more holistic manner. concerns with new language structs

[PATCH] D152274: [clang] Don't create import decls without -fmodules

2023-06-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 528863. kadircet added a comment. - Test layering check violation diagnostics Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152274/new/ https://reviews.llvm.org/D152274 Files: clang/lib/Sema/SemaModule.cpp

[PATCH] D152274: [clang] Don't create import decls without -fmodules

2023-06-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. @dblaikie this also updates `getSourceDescriptor-crash.cpp`, which was making use of generation of implicit import decl. I assumed that test is trying to make sure `ImportDecl` is represented properly, and incidentally relied on implicit generation of it. LMK if that i

[PATCH] D152274: [clang] Don't create import decls without -fmodules

2023-06-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: sammccall, dblaikie. Herald added a subscriber: ChuanqiXu. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When modules are disabled, there's no

[PATCH] D152265: [clang][test] Use a physical copy of FS

2023-06-06 Thread Kadir Cetinkaya 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 rG476e7c49ecb7: [clang][test] Use a physical copy of FS (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D152265: [clang][test] Use a physical copy of FS

2023-06-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 528834. kadircet added a comment. - Fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152265/new/ https://reviews.llvm.org/D152265 Files: clang/unittests/Serialization/ModuleCacheTest.cpp clang/unitt

[PATCH] D152265: [clang][test] Use a physical copy of FS

2023-06-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Make use of a physical copy, rather than real FS in unittests that change working-di

[PATCH] D150185: [include-cleaner] Allow multiple strategies for spelling includes.

2023-05-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks a lot! outline seems good. mostly some comments on implementation details. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:92 + + virtual std::string operator()(llvm::StringRef HeaderPhysicalPath) const

[PATCH] D148793: [WIP][clang-tidy] Implement an include-cleaner check.

2023-05-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:42 + +struct MissingIncludeInfo { + SourceLocation SymRefLocation; let's put this struct into anon namespace Comment at: clang-tools-extra/cl

[PATCH] D151294: [clangd] Remove inline Specifier for DefineOutline Tweak

2023-05-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfe7afcf70c93: [clangd] Remove inline Specifier for DefineOutline Tweak (authored by bgluzman, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D151294: [clangd] Remove inline Specifier for DefineOutline Tweak

2023-05-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LGTM! Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:136 +// Removes matching instances of given token preceeding the function defition. +

[PATCH] D151321: [clangd] Dont run raw-lexer for OOB source locations

2023-05-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa2f7352f3e80: [clangd] Dont run raw-lexer for OOB source locations (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151321/new/ https:/

[PATCH] D148793: [WIP][clang-tidy] Implement an include-cleaner check.

2023-05-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:61 +void IncludeCleanerCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "Suppress", ""); +} after storing them in the class state

[PATCH] D151321: [clangd] Dont run raw-lexer for OOB source locations

2023-05-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. We can get stal

[PATCH] D151303: [clangd] Fix add-using tweak on declrefs with template arguments

2023-05-24 Thread Kadir Cetinkaya 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 rG51df1a9ac96f: [clangd] Fix add-using tweak on declrefs with template arguments (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D151303: [clangd] Fix add-using tweak on declrefs with template arguments

2023-05-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: VitaNuo. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository:

[PATCH] D149948: [include-cleaner] Treat references to nested types implicit

2023-05-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rGcce7b816a1ee: [include-cleaner] Treat references to nested types implicit (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D149948?vs=

[PATCH] D151185: [clangd] Store paths as requested in PreambleStatCache

2023-05-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG35ce741ef3e3: [clangd] Store paths as requested in PreambleStatCache (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151185/new/ https

[PATCH] D151185: [clangd] Store paths as requested in PreambleStatCache

2023-05-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Underlying FS c

[PATCH] D151073: [clang] Fix label (de-)serialization in ASM statements.

2023-05-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang/test/PCH/asm-label.cpp:5 +#define HEADER_H +#pragma once +void MyMethod() { you can drop `#pragma once` know, as it won't be parsed again anyways Comment at: clang/test/PCH/asm-label.cpp:6 +#pra

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

2023-05-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D150997#4357589 , @nikic wrote: > It looks like you forgot to `git add` the new file maybe? > > This was added in https://reviews.llvm.org/D133200, maybe @kadircet can > comment on what this was intended for. It was already

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

2023-05-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D150997#4357589 , @nikic wrote: > It looks like you forgot to `git add` the new file maybe? > > This was added in https://reviews.llvm.org/D133200, maybe @kadircet can > comment on what this was intended for. It was already

[PATCH] D147808: [clangd] Support non-trivial defaulted special member functions in Define Outline tweak

2023-05-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks a lot, and sorry for the delay! Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:333 +[](const syntax::Token &Tok) { return Tok.kind(

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:90 +if (Tasks) { + std::unique_lock Lock(*Barrier, std::try_to_lock); + Tasks->runAsync("task:" + Path + Version, std::move(Task)); DmitryPolukhin wrote: > kadirce

[PATCH] D150548: [clangd] Deduplicate diagnostic codeaction fixes when the codeaction fixes multiple diagnostics

2023-05-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1012-1013 + + void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, can you

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:90 +if (Tasks) { + std::unique_lock Lock(*Barrier, std::try_to_lock); + Tasks->runAsync("task:" + Path + Version, std::move(Task)); what's the point of this semaph

[PATCH] D150668: Add doc link to missing include diagnostics.

2023-05-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150668/new/ https://reviews.llvm.org/D150668

[PATCH] D149276: [Clang] Fix parsing of `(auto(x))`.

2023-05-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. hi! this seem to have regressed compilation for some valid C++ code: struct Bar { const char *name(); }; struct Baz { static Bar *method(); }; struct Foo { Foo(const char *); }; template void bar() { Foo _(T::method()->name()); } void foo

[PATCH] D150582: [clangd] Fix test failure when it's built with compiler flags unknown by clang

2023-05-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. This broke buildbots because `compile_commands.json` being `empty` has other consequences. Fixing forward in 9ffef0f24de0fc05b946e662a7b233a32ad058e3

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

2023-05-15 Thread Kadir Cetinkaya 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 rG0d1be98a67e2: [clang][USR] Prevent crashes on incomplete FunctionDecls (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

2023-05-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D149733#4342300 , @aaron.ballman wrote: > But at the same time, it's become more obvious (at least to me) that clangd > has features that don't work with all of the invariants in Clang and I don't > know that we ever really

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:694 Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded(); -return Result; +CapturedCtx.emplace(CapturedInfo.takeLife()); +return std::make_pair(Result, CapturedCtx

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

2023-05-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > I think we probably should have a broader discussion before moving forward > here. It's not that this isn't incremental progress fixing an issue, but it's > more that this same justification works to add the workaround 200 more times > without ever addressing the und

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. i think something went wrong with the diff, you don't seem to update PreambleCallbacks to trigger indexing on a different thread at all (and also there are certain lifetime issues). is this the final version of the patch or did I bump into a WIP version unknowingly ?

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

2023-05-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. gentle ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149733/new/ https://reviews.llvm.org/D149733 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D149912: [clangd] downgrade missing-includes diagnostic to Information level

2023-05-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149912/new/ https://reviews.llvm.org/D149912 __

[PATCH] D148793: [WIP][clang-tidy] Implement an include-cleaner check.

2023-05-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks, mostly LG at the high level i think one thing we're missing is the ability to suppress analysis for certain headers, similar to what we have in clangd config options. can you add a check option for that? Comment at: clang-tools-extra/clang-ti

[PATCH] D150254: [tidy] Fix possible use-after-free in IdentifierNamingCheck

2023-05-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG2b240cc377b5: [tidy] Expose getID to tidy checks (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150254/new/ https://reviews.llvm.org/

[PATCH] D150257: [clangd] Initialize clang-tidy modules only once

2023-05-10 Thread Kadir Cetinkaya 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 rG62a090f958ce: [clangd] Initialize clang-tidy modules only once (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D150254: [tidy] Fix possible use-after-free in IdentifierNamingCheck

2023-05-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D150254#4331738 , @njames93 wrote: > In D150254#4331640 , @kadircet > wrote: > >> see >> https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/ClangTidy.cpp#L63

[PATCH] D150257: [clangd] Initialize clang-tidy modules only once

2023-05-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 520948. kadircet marked 4 inline comments as done. kadircet added a comment. - Move ReplayPreambleTests to its own file and move registry to global scope. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150257/ne

[PATCH] D150254: [tidy] Fix possible use-after-free in IdentifierNamingCheck

2023-05-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 520936. kadircet added a comment. - Expose getID to tidy-checks and use it instead of storing checkname Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150254/new/ https://reviews.llvm.org/D150254 Files: clan

[PATCH] D150257: [clangd] Initialize clang-tidy modules only once

2023-05-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: carlosgalvezp, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.

[PATCH] D150254: [tidy] Fix possible use-after-free in IdentifierNamingCheck

2023-05-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. see https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/ClangTidy.cpp#L638 for such a pattern, clangd also initializes checks with a similar approach. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D150254: [tidy] Fix possible use-after-free in IdentifierNamingCheck

2023-05-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: hokein, PiotrZSL. Herald added a subscriber: carlosgalvezp. Herald added a reviewer: njames93. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe

[PATCH] D150187: [tidy][IdentifierNaming] Fix crashes on non-identifiers

2023-05-09 Thread Kadir Cetinkaya 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 rGb36a2e7828be: [tidy][IdentifierNaming] Fix crashes on non-identifiers (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE L

<    1   2   3   4   5   6   7   8   9   10   >