https://github.com/kadircet approved this pull request.
thanks!
https://github.com/llvm/llvm-project/pull/120321
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,31 @@
+//===--- DiagnosticIDs.inc --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
https://github.com/kadircet requested changes to this pull request.
https://github.com/llvm/llvm-project/pull/120321
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kadircet wrote:
we already have a concept of `DiagnosticID`. can we rename this to
`AllDiagnosticKinds.inc` ?
https://github.com/llvm/llvm-project/pull/120321
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
@@ -36,11 +38,16 @@ namespace clangd {
/// `[:partition-name]`. So module names covers
partitions.
class ProjectModules {
public:
+ using CommandProvider =
+ llvm::unique_function;
kadircet wrote:
this looks like a command-mangler, rather than command-p
@@ -191,6 +211,29 @@ export module M;
EXPECT_TRUE(MInfo->canReuse(*Invocation, FS.view(TestDir)));
}
+TEST_F(PrerequisiteModulesTests, ModuleWithArgumentPatch) {
+ MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+ CDB.addExtraClangFlag("-invalid-unknown-flag");
+
+ C
https://github.com/kadircet requested changes to this pull request.
thanks for working on this!
https://github.com/llvm/llvm-project/pull/122606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe
@@ -48,7 +51,8 @@ class ModuleDependencyScanner {
};
/// Scanning the single file specified by \param FilePath.
- std::optional scan(PathRef FilePath);
+ std::optional scan(PathRef FilePath,
+ CommandProvider const &Provider);
--
@@ -9,8 +9,10 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROJECTMODULES_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROJECTMODULES_H
+#include "support/Function.h"
#include "support/Path.h"
#include "support/ThreadsafeFS.h"
+#include "clang/Tooling/CompilationDatabase.h"
-
@@ -191,6 +211,29 @@ export module M;
EXPECT_TRUE(MInfo->canReuse(*Invocation, FS.view(TestDir)));
}
+TEST_F(PrerequisiteModulesTests, ModuleWithArgumentPatch) {
+ MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+ CDB.addExtraClangFlag("-invalid-unknown-flag");
+
+ C
https://github.com/kadircet edited
https://github.com/llvm/llvm-project/pull/122606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -32,6 +32,9 @@ namespace {
/// interfere with each other.
class ModuleDependencyScanner {
public:
+ using CommandProvider =
kadircet wrote:
nit: let's use the one from `ProjectModules` rather than duplicating it here
https://github.com/llvm/llvm-project/p
@@ -39,7 +40,25 @@ class MockDirectoryCompilationDatabase : public
MockCompilationDatabase {
void addFile(llvm::StringRef Path, llvm::StringRef Contents);
std::unique_ptr getProjectModules(PathRef) const override {
-return scanningProjectModules(MockedCDBPtr, TFS);
+
@@ -57,7 +61,7 @@ class ModuleDependencyScanner {
/// a global module dependency scanner to monitor every file. Or we
/// can simply require the build systems (or even the end users)
/// to provide the map.
- void globalScan();
+ void globalScan(CommandProvider const &P
kadircet wrote:
can you update `clang-tools-extra/clangd/TidyFastChecks.inc` in a follow up
change, after this check lands in its final shape?
https://github.com/llvm/llvm-project/pull/122940
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
@@ -181,13 +182,96 @@ class DiagnosticMapping {
class DiagnosticIDs : public RefCountedBase {
public:
/// The level of the diagnostic, after it has been through mapping.
- enum Level {
-Ignored, Note, Remark, Warning, Error, Fatal
+ enum Level : uint8_t { Ignored, Note,
@@ -584,7 +582,9 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID,
SourceLocation Loc,
const auto &SM = Diag.getSourceManager();
// Custom diagnostics always are emitted in system headers.
kadircet wrote:
can you also update the comment here to desc
https://github.com/kadircet requested changes to this pull request.
> but I'd really love to hear from @kadircet as to whether this PR fully
> addresses the issues found before:
> https://github.com/llvm/llvm-project/pull/70976#discussion_r1775918960
I think it mostly does (left some comments
@@ -551,10 +547,12 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID,
SourceLocation Loc,
// as well as disabling all messages which are currently mapped to Warning
// (whether by default or downgraded from Error via e.g. -Wno-error or
#pragma
// diagnostic.)
+ /
https://github.com/kadircet edited
https://github.com/llvm/llvm-project/pull/119712
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kadircet edited
https://github.com/llvm/llvm-project/pull/125988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -449,21 +534,17 @@ llvm::Error
ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
std::unique_ptr
ModulesBuilder::buildPrerequisiteModulesFor(PathRef File,
const ThreadsafeFS &TFS) {
- std::unique_ptr MDB = Impl->getCDB().
@@ -380,30 +381,114 @@ llvm::SmallVector
getAllRequiredModules(ProjectModules &MDB,
return ModuleNames;
}
+class CachingProjectModules : public ProjectModules {
+public:
+ CachingProjectModules(const GlobalCompilationDatabase &CDB) : CDB(CDB) {}
+
+ std::vector getRequire
https://github.com/kadircet requested changes to this pull request.
thanks, I think the biggest concern is about initial performance of this
approach until cache warms up.
also can we have some tests? it should be possible to unittest by injecting a
custom CDB into modules builder.
https://gi
@@ -380,30 +381,114 @@ llvm::SmallVector
getAllRequiredModules(ProjectModules &MDB,
return ModuleNames;
}
+class CachingProjectModules : public ProjectModules {
+public:
+ CachingProjectModules(const GlobalCompilationDatabase &CDB) : CDB(CDB) {}
+
+ std::vector getRequire
https://github.com/kadircet edited
https://github.com/llvm/llvm-project/pull/127109
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kadircet requested changes to this pull request.
can you also add some tests to `SymbolCollectorTests.cpp` to make sure we
capture `overriddenby` relationships relevant to your use cases?
index tests in `XRefsTests.cpp` uses rather dynamic-index hence can have
different char
@@ -1438,6 +1461,15 @@ ReferencesResult findReferences(ParsedAST &AST, Position
Pos, uint32_t Limit,
getOverriddenMethods(CMD, OverriddenMethods);
}
}
+// Special case: Objective-C methods can override a parent class or
+// protoco
@@ -430,6 +430,18 @@ locateASTReferent(SourceLocation CurLoc, const
syntax::Token *TouchedIdentifier,
continue;
}
}
+// Special case: an Objective-C method can override a parent class or
+// protocol.
kadircet wrote:
what about the i
@@ -1283,6 +1295,17 @@ std::vector findImplementations(ParsedAST
&AST, Position Pos,
} else if (const auto *RD = dyn_cast(ND)) {
IDs.insert(getSymbolID(RD));
QueryKind = RelationKind::BaseOf;
+} else if (const auto *OMD = dyn_cast(ND)) {
+ IDs.insert(g
https://github.com/kadircet approved this pull request.
thanks, as long as we're keeping the underlying option I think this makes
sense! we can flip it in production if we see big regressions
https://github.com/llvm/llvm-project/pull/127359
___
cfe-co
https://github.com/kadircet commented:
thanks, as long as we're keeping the underlying option I think this makes
sense! we can flip it in production if we see big regressions
https://github.com/llvm/llvm-project/pull/127359
___
cfe-commits mailing lis
kadircet wrote:
I am a little hesitant on the memory growth implications, as this won't be a
fixed cost we have in static index, but also a significant increase in
dynamic-index memory footprint (which grows proportionally with number of
touched files in an editing session). We're going to pay
@@ -430,6 +439,22 @@ locateASTReferent(SourceLocation CurLoc, const
syntax::Token *TouchedIdentifier,
continue;
}
}
+// Special case: - (void)^method; should jump to all overrides. Note that
an
+// Objective-C method can override a parent class or pr
https://github.com/kadircet requested changes to this pull request.
https://github.com/llvm/llvm-project/pull/127109
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kadircet requested changes to this pull request.
https://github.com/llvm/llvm-project/pull/127109
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -430,6 +430,18 @@ locateASTReferent(SourceLocation CurLoc, const
syntax::Token *TouchedIdentifier,
continue;
}
}
+// Special case: an Objective-C method can override a parent class or
+// protocol.
kadircet wrote:
> I was thinking
kadircet wrote:
> My main motivation is to get DefineOutline
I think if we're solely aiming for improvements to DefineOutline, I'd say the
resource trade-off here isn't worth it.
> I don't see how bracket matching can help us get from the name of the
> function to the actual start of the decl
kadircet wrote:
> In my case I am using my build system (bazel) to generate the
> compile_commands.json file as part of the build. At this point given bazel's
> sandboxing / hermiticity model, the user's checkout PWD is unknown. This file
> is then copied somewhere and used by clangd. Today to
https://github.com/kadircet edited
https://github.com/llvm/llvm-project/pull/125988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -380,30 +381,114 @@ llvm::SmallVector
getAllRequiredModules(ProjectModules &MDB,
return ModuleNames;
}
+class CachingProjectModules : public ProjectModules {
+public:
+ CachingProjectModules(const GlobalCompilationDatabase &CDB) : CDB(CDB) {}
+
+ std::vector getRequire
https://github.com/kadircet approved this pull request.
sorry for the delayed turn around, lgtm!
https://github.com/llvm/llvm-project/pull/125988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cf
https://github.com/kadircet approved this pull request.
https://github.com/llvm/llvm-project/pull/128603
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2625,14 +2636,10 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
// More analysis ...
};
+
// Emit per-function analysis-based warnings that require the whole-TU
- // reasoning. Check if any of them is enabled at all before scanning the AST:
- if (!Di
@@ -1438,6 +1461,15 @@ ReferencesResult findReferences(ParsedAST &AST, Position
Pos, uint32_t Limit,
getOverriddenMethods(CMD, OverriddenMethods);
}
}
+// Special case: Objective-C methods can override a parent class or
+// protoco
https://github.com/kadircet approved this pull request.
https://github.com/llvm/llvm-project/pull/127757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kadircet wrote:
> Could you say more about this? What happened to ClangdModules?
Sorry, apparently I even forgot its proper name, i meant
`clangd::FeatureModule`. Nothing bad happened, we built this infra but didn't
get a chance to polish and use it afterwards.
We wanted to migrate any non-cor
kadircet wrote:
> We could cut the memory usage overhead of the patch in half by only having it
> in one of them. (@kadircet would that change your analysis of the tradeoffs
> at all?)
I think that'd still be too much considering the functionality we'll get in the
end. Others might not share
@@ -411,6 +411,58 @@ TEST(LocateSymbol, FindOverrides) {
sym("foo", Code.range("2"), std::nullopt)));
}
+TEST(LocateSymbol, FindOverridesObjC) {
+ auto Code = Annotations(R"objc(
+@protocol Fooey
+- (void)foo;
+@end
+@interfa
kadircet wrote:
> we've had users for whom the BuiltinHeaders: QueryDriver behaviour worked
> better and want to switch back to it (examples here, here, here, here).
I'd argue that the fix isn't as simple as using builtin headers from a
different toolchain for those users. Because clang itself
https://github.com/kadircet created
https://github.com/llvm/llvm-project/pull/130077
We started seeing some use-after-frees starting with
https://github.com/llvm/llvm-project/pull/125433.
This patch ensures we explicitly block for the async task, if there's one,
before destructing `std::future
kadircet wrote:
Hi folks, we're seeing some regressions on our internal tidy checks after this
patch.
The regression is similar to what @haoNoQ raised in
https://github.com/llvm/llvm-project/pull/128150#issuecomment-2705425193, the
blast radius a little bit wider than checks that build and in
@@ -94,8 +94,6 @@ Improvements to clang-tidy
- :program:`clang-tidy` no longer processes declarations from system headers
by default, greatly improving performance. This behavior is disabled if the
`SystemHeaders` option is enabled.
- Note: this may lead to false negatives
https://github.com/kadircet edited
https://github.com/llvm/llvm-project/pull/132725
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -94,8 +94,6 @@ Improvements to clang-tidy
- :program:`clang-tidy` no longer processes declarations from system headers
by default, greatly improving performance. This behavior is disabled if the
`SystemHeaders` option is enabled.
- Note: this may lead to false negatives
https://github.com/kadircet approved this pull request.
https://github.com/llvm/llvm-project/pull/131591
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kadircet commented:
> Are you able to test this PR and confirm it fixes the regressions you
> encountered?
hi @carlosgalvezp , yes this fixes the particular issues we had, thanks!
https://github.com/llvm/llvm-project/pull/132725
__
801 - 857 of 857 matches
Mail list logo