[PATCH] D139921: [include-cleaner] Ranking of providers based on hints

kadircet updated this revision to Diff 487721. kadircet marked 5 inline comments as done. kadircet added a comment. - Address all comments but the ones on tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139921/new/

[PATCH] D139921: [include-cleaner] Ranking of providers based on hints

kadircet marked 26 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:84 +/// Represents properties of a symbol provider. +enum class Hint : uint8_t { sammccall wrote: > along with

[PATCH] D141218: [clangd] Include the correct header for typeid()

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks for catching this! Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:1531 +TEST(IncludeFixerTest, Typeid) { + Annotations Test(R"cpp(

[PATCH] D139458: [clangd] Full support for #import insertions

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/D139458/new/ https://reviews.llvm.org/D139458

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:674 +else if (const auto *ND = dyn_cast(Context)) { + if (ND->isInlineNamespace()) +Scopes.AccessibleScopes.push_back(printQualifiedName(*ND) + "::");

[PATCH] D139458: [clangd] Full support for #import insertions

kadircet requested changes to this revision. kadircet added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:572 + // case where a header file contains ObjC decls but no #imports. +

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:674 +else if (const auto *ND = dyn_cast(Context)) { + if (ND->isInlineNamespace()) +Scopes.AccessibleScopes.push_back(printQualifiedName(*ND) + "::");

[PATCH] D140745: WIP: generate Config {Fragment structure, json schema, docs, YAML parser} from schema spec

kadircet added a comment. In D140745#4021783 , @sammccall wrote: > worthwhile overall? > --- > > This isn't trivial: does it solve a big enough problem to be worth the > complexity? (Benefit would be improved maintenance of existing

[PATCH] D139446: [clangd] Add flag to control #import include insertions

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:271 +desc("If header insertion is enabled, add #import directives when " + "accepting code

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:674 +else if (const auto *ND = dyn_cast(Context)) { + if (ND->isInlineNamespace()) +Scopes.AccessibleScopes.push_back(printQualifiedName(*ND) + "::"); since we

[PATCH] D139458: [clangd] Full support for #import insertions

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/AST.h:122 +/// Returns the Include Directive which should be used for include insertions +/// for the given main file.

[PATCH] D139458: [clangd] Full support for #import insertions

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ASTSignals.h:39 + + static Symbol::IncludeDirective preferredIncludeDirective( + llvm::StringRef Filename, const LangOptions , could you rather move this to `AST.h`, with some comments

[PATCH] D140000: [clangd] Compute the unused includes in the check mode.

kadircet added a comment. what's the motivating reason behind the change? we'll soon change the implementation to use include-cleaner instead, and can run the tool directly to verify the findings (rather than clangd). hence i feel like this will just turn into not-so-useful extra output for

[PATCH] D140486: [clangd] Fix crashing race in ClangdServer shutdown with stdlib indexing

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/ClangdServer.cpp:86 +auto Task = [ +// Captured by value +

[PATCH] D140380: [include-cleaner] Respect IWYU pragmas during analyze

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb76cc30e1585: [include-cleaner] Respect IWYU pragmas during analyze (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D140380: [include-cleaner] Respect IWYU pragmas during analyze

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp:184 #include "b.h" +#include "keep.h" // IWYU pragma: keep hokein wrote: > add the `export` case as well. i don't think we should be testing the behaviour

[PATCH] D140380: [include-cleaner] Respect IWYU pragmas during analyze

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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D140380

[PATCH] D140191: [CodeComplete] Offer completions for headers with extension .hxx in include directives

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang/lib/Sema/SemaCodeComplete.cpp:10020 // Only files that really look like headers. (Except in special dirs). // Header

[PATCH] D139998: [clangd] Remove ReferenceFinder::Reference::Target

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/XRefs.cpp:871 for (const NamedDecl *ND : Targets) { const Decl *CD = ND->getCanonicalDecl(); +

[PATCH] D134130: [clangd] Add doxygen parsing for Hover [1/3]

kadircet added a comment. In D134130#3992215 , @tom-anders wrote: > Thanks for the detailed feedback! Unfortunately, I’m sick right now, so I > probably won’t be able to give a detailed answer until after Christmas. Sorry to hear that. I hope you get

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:469 // Also grab prefixes for each option, these are not fully exposed. -const char *const *Prefixes[DriverID::LastOption] = {nullptr}; -#define PREFIX(NAME, VALUE) static const

[PATCH] D134130: [clangd] Add doxygen parsing for Hover [1/3]

kadircet added a comment. Hi! Sorry for letting these series of patches sit around without any comments. We were having some discussions internally to both understand the value proposition and its implications on the infrastructure. So it'd help a lot if you can provide some more information

[PATCH] D139458: [clangd] Full support for #import insertions

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ASTSignals.cpp:28 + // Source files: Use #import if the ObjC language flag is enabled + if (isHeaderFile(AST.tuPath(), AST.getLangOpts())) { +if (AST.getLangOpts().ObjC) { can we rewrite

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470 const char *const *Prefixes[DriverID::LastOption] = {nullptr}; -#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE; +#define PREFIX(NAME, VALUE) static constexpr

[PATCH] D139921: [include-cleaner] Ranking of providers based on hints

kadircet created this revision. kadircet added reviewers: hokein, sammccall. Herald added a subscriber: mgrang. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Introduce signals to rank

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470 const char *const *Prefixes[DriverID::LastOption] = {nullptr}; -#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE; +#define PREFIX(NAME, VALUE) static constexpr

[PATCH] D139835: [include-cleaner] Include the reference type when printing the SymbolReference.

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/lib/Types.cpp:49 // isn't completely useless (and distinguishes SymbolReference from Symbol). - return OS <<

[PATCH] D138779: [include-cleaner] Filter out references that not spelled in the main file.

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/unittests/AnalysisTest.cpp:292 + R"cpp( +#define PLUS_ONE(X) X() + 1 +int a =

[PATCH] D128677: [clang][Tooling] Add support for generating #import edits

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang/unittests/Tooling/HeaderIncludesTest.cpp:77 +TEST_F(HeaderIncludesTest, DeleteImportAndSameInclude) { + std::string Code = "#include

[PATCH] D128457: [clangd] Add new IncludeType to IncludeHeaderWithReferences

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang/unittests/Tooling/HeaderAnalysisTest.cpp:69 + #include "foo.h" +#import "NSFoo.h" + indentation

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG29a8eec1f9eb: [include-cleaner] Make use of locateSymbol in WalkUsed and HTMLReport (authored by kadircet). Repository: rG LLVM Github Monorepo

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

kadircet updated this revision to Diff 480454. kadircet marked an inline comment as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138200/new/ https://reviews.llvm.org/D138200 Files:

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/test/multiple-providers.cpp:1 +// RUN: clang-include-cleaner --print=changes %s -- -I %S/Inputs | FileCheck \ +// RUN: --allow-empty %s

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

kadircet updated this revision to Diff 480420. kadircet added a comment. - Fix RUN: line in lit test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138200/new/ https://reviews.llvm.org/D138200 Files:

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

kadircet updated this revision to Diff 480419. kadircet marked an inline comment as done. kadircet added a comment. - Drop the cache - Use `-print=changes` in lit test - Update unittest helper to limit decls to main file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

kadircet marked 5 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/test/html.cpp:7 int n = foo(); +// CHECK: Symbol{{.*}}foo +// CHECK-NEXT: int foo() sammccall wrote: > I'd like to keep a very simple smoke

[PATCH] D127844: [clangd] Pull suppression logic into common path, apply for driver diagnostics

kadircet added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127844/new/ https://reviews.llvm.org/D127844 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D138779: [include-cleaner] Filter out references that not spelled in the main file.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:36 /// Find and report all references to symbols in a region of code. +/// It will filter out references that are not spelled in the main file. ///

[PATCH] D139277: [clangd] Use all query-driver arguments in cache key

kadircet added a comment. thanks a lot for taking a look at this! Comment at: clang-tools-extra/clangd/SystemIncludeExtractor.cpp:301 llvm::StringRef Lang; +llvm::SmallVector AdditionalDriverArgs; + can we introduce a struct instead? ``` struct

[PATCH] D128457: [clangd] Add new IncludeType to IncludeHeaderWithReferences

kadircet added a comment. it feels like rebasing went wrong. changes from 2 unrelated patches seem to be part of this one now. can you make sure this patch only contains the delta for symbolcollector/clangd pieces? Comment at: clang-tools-extra/clangd/Headers.h:50 + /// The

[PATCH] D128677: [clang][Tooling] Add support for generating #import edits

kadircet added a comment. thanks, mostly LG. some small changes. Comment at: clang-tools-extra/clangd/IncludeFixer.cpp:261 if (Symbol.empty()) +F.Message = llvm::formatv("{0} {1}", nit: `llvm::StringLiteral DirectiveSpelling = Directive ==

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:44 + + // Cache for decl to header mappings, as the same decl might be referenced in + // multiple locations and finding providers for each location is expensive.

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

kadircet updated this revision to Diff 480340. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138200/new/ https://reviews.llvm.org/D138200 Files:

[PATCH] D139013: [include-cleaner] clang-include-cleaner can print/apply edits

kadircet accepted this revision. kadircet added a comment. thanks! Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:77 + bool Satisfied = false; + for (const Header : Providers) { + if (H.kind() == Header::Physical &&

[PATCH] D139087: [include-cleaner] Handle base class member access from derived class.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:62 bool VisitMemberExpr(MemberExpr *E) { -report(E->getMemberLoc(), E->getFoundDecl().getDecl()); +// Instead of the FieldDecl for MemberExpr, we report the Decl of +//

[PATCH] D128677: [clangd] Add support for generating #import edits

kadircet added a comment. we should have tests in `clang/unittests/Tooling/HeaderIncludesTest.cpp` and the commit itself should be tagged as `[clang][Tooling]` rather than `[clangd]`. Comment at: clang-tools-extra/clangd/Headers.h:250 + llvm::Optional insert(llvm::StringRef

[PATCH] D139013: [include-cleaner] clang-include-cleaner can print/apply edits

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/include/clang-include-cleaner/Analysis.h:65 +llvm::ArrayRef MacroRefs, +

[PATCH] D139093: [include-cleaner] Use RAV instead of ASTMatchers in LocateSymbolTest

This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG4764ee6055c7: [include-cleaner] Use RAV instead of ASTMatchers in LocateSymbolTest (authored by kadircet). Changed prior to commit:

[PATCH] D139093: [include-cleaner] Use RAV instead of ASTMatchers in LocateSymbolTest

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp:60 + return true; +Out = ND; +return false; sammccall wrote: > you might want to

[PATCH] D139093: [include-cleaner] Use RAV instead of ASTMatchers in LocateSymbolTest

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang-tools-extra. ASTMatchers are pulling in

[PATCH] D135953: [IncludeCleaner] Introduce decl to location mapping

This revision was automatically updated to reflect the committed changes. kadircet marked 6 inline comments as done. Closed by commit rGf82f5b0507a2: [include-cleaner] Introduce symbol to location mapping (authored by kadircet). Changed prior to commit:

[PATCH] D139018: [include-cleaner] Record whether includes are spelled with quotes

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/D139018/new/ https://reviews.llvm.org/D139018

[PATCH] D128457: [clangd] Add new IncludeType to IncludeHeaderWithReferences

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Headers.cpp:237 for (const auto : Includes) -Headers.push_back(Include.IncludeHeader); +if (Include.SupportedDirectives & clang::clangd::Symbol::Include) +

[PATCH] D138821: [include-cleaner] Remove filtering from UsingDecl visit.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:86 report(UD->getLocation(), TD, IsUsed ? RefType::Explicit : RefType::Ambiguous); } hokein wrote: > we should report all references as

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

kadircet updated this revision to Diff 478614. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138200/new/ https://reviews.llvm.org/D138200 Files: clang-tools-extra/include-cleaner/lib/Analysis.cpp

[PATCH] D135953: [IncludeCleaner] Introduce decl to location mapping

kadircet updated this revision to Diff 478611. kadircet marked 6 inline comments as done. kadircet added a comment. - Leaving out all the pieces around signals as discussed. - Update tests to use a LocateExample helper, have a test for macros. - Get rid of residues in other test files.

[PATCH] D135508: [clangd] Heuristic to avoid desync if editors are confused about newline-at-eof

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. as discussed, this is a rather contained fix that are likely to help with other editors that might get confused with new-lines at EOF. so LGTM Repository: rG LLVM Github Monorepo

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

kadircet added a comment. thanks LG, i'd like to hear how we're planning to let downstream users customise the list of fast checks. otherwise they have to run with `Loose` at all times. the easiest i can think of is, generating their own `fastchecks.inc` fragment and #include that in addition

[PATCH] D138779: [include-cleaner] Filter out references that not spelled in the main file.

kadircet added a comment. Maybe I am missing some context here, but why are we trying to filter out references outside of the main file? ATM there's no concept of main file in neither walkUsed, it just receives some ast nodes for analyzing references to declarations inside these nodes, and the

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

kadircet added a comment. In D137205#3951230 , @Febbe wrote: > In D137205#3950722 , @kadircet > wrote: > >> another thing that i noticed is usage of east consts in the implementation >> files. no one seem to

[PATCH] D138678: [include-cleaner] Capture private headers in PragmaIncludes.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:237 + auto *FE = SM.getFileEntryForID(SM.getFileID(Range.getBegin())); + if (!FE) { +return false; nit: you can drop the braces here. LLVM convention

[PATCH] D138709: Reland "[Lex] Fix suggested spelling of /usr/bin/../include/foo"

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang/lib/Lex/HeaderSearch.cpp:1936 + path::remove_dots(FilePath, /*remove_dot_dot=*/true); + path::native(FilePath, path::Style::posix); + File =

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

kadircet added a comment. another thing that i noticed is usage of east consts in the implementation files. no one seem to have brought it up so far (at least none that i can see). in theory there are no explicit guidelines about it in LLVM coding style, but rest of the codebase uses west const

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

kadircet added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.cpp:215 "-bugprone-use-after-move", + // Using an CFG and might crash on invalid code: +

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

kadircet added a comment. Sorry for noticing this so late. Yes the initial reason we left `sysroot` and `std/builtin-inc` related flags were exactly that, in theory they could vary but in practice they were applied to all or none of the project. Regarding this change itself, surely preserving

[PATCH] D138678: [include-cleaner] Capture private headers in PragmaIncludes.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:97 + /// the IWYU private pragmas with public mapping. + llvm::DenseSet IWYUPrivate; + i'd actually merge this with the previous map, and

[PATCH] D138677: [Lex] Fix suggested spelling of /usr/bin/../include/foo

kadircet added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:1932 + + llvm::SmallString<32> FilePath(File.begin(), File.end()); + path::remove_dots(FilePath, /*remove_dot_dot=*/true); nit: you can change the method signature to take in a

[PATCH] D138648: [include-cleaner] Make Symbol (and Macro) hashable.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:147 + using Outer = clang::include_cleaner::Symbol; + constexpr

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

kadircet added a comment. i can't think of a proper way to test this out either. unless we somehow let slow-tidy-check list to be configurable, so probably fine to make sure it works locally and hope that new people introducing tidy checks do complain. Comment at:

[PATCH] D138491: [clangd] Add script to maintain list of fast clang-tidy checks

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/TidyFastChecks.inc:2 +// This file is generated, do not edit it directly! +// This describes +#ifndef FAST can

[PATCH] D138491: [clangd] Add script to maintain list of fast clang-tidy checks

kadircet added inline comments. Comment at: clang-tools-extra/clangd/TidyFastChecks.py:31 +default='clang-tools-extra/clangd/TidyFastChecks.inc') +parser.add_argument('--file', help='clangd binary to invoke', +default='clang/lib/Sema/Sema.cpp') `file to

[PATCH] D138429: [clang][RISCV][NFC] Prevent data race in RVVType::computeType

kadircet added inline comments. Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:289 + + static uint64_t computeRVVTypeHashValue(BasicType BT, int Log2LMUL, + PrototypeDescriptor Proto); khchen wrote: >

[PATCH] D138429: [clang][RISCV][NFC] Prevent data race in RVVType::computeType

kadircet added a comment. thanks! +1 from my side as well for thread-safety purposes (and I hope having 2 separate caches for tablegen and sema doesn't have unexpected effects). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138429/new/

[PATCH] D124730: [RISCV][NFC] Refactor RISC-V vector intrinsic utils.

kadircet added a comment. In D124730#3916786 , @kadircet wrote: > Hi @kito-cheng, can you please let us know if you're working on a fix here > (and whether it seems to be close)? otherwise i am planning to revert this > and consequent changes, as it's

[PATCH] D138287: [clang][RISCV] Drop caching from RVVType as it introduces data races

kadircet created this revision. kadircet added reviewers: kito-cheng, ilya-biryukov. Herald added subscribers: sunshaoce, VincentWu, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01,

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:47 + } + void buildAST() { AST = std::make_unique(Inputs); } +

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

kadircet added a comment. thanks, mostly LG a bunch of suggestions for tests Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:74 + /// Returns true if the given file is a self-contained file. + bool isSelfContained(const FileEntry *File)

[PATCH] D128457: [clangd] Add new IncludeType to IncludeHeaderWithReferences

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:1241 + !HeaderInfo.hasFileBeenImported(FE) && + (FID != SM.getMainFileID() || !fileContainsImports(FID, SM))) return false; can you put a comment here saying `any

[PATCH] D136082: [clangd] Extend --check to time clang-tidy checks, so we can block slow ones

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Herald added a reviewer: njames93. thanks, lgtm! Comment at: clang-tools-extra/clangd/tool/Check.cpp:66 +llvm::cl::desc("Print the overhead of checks matching this

[PATCH] D135953: [IncludeCleaner] Introduce decl to location mapping

kadircet updated this revision to Diff 476091. kadircet added a comment. - Rebase - Move to new file - Handle macros - Change from boolean output to enum for signalling Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135953/new/

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

kadircet created this revision. kadircet added reviewers: hokein, sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Depens on D135953

[PATCH] D138047: Fix use of dangling stack allocated string in IncludeFixer

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/D138047/new/ https://reviews.llvm.org/D138047

[PATCH] D138134: [include-cleaner] Defer decl->stdlib conversion into decl->location conversion

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe5739eb4f813: [include-cleaner] Defer decl-stdlib conversion into decl-location conversion (authored by kadircet). Repository: rG LLVM Github

[PATCH] D138134: [include-cleaner] Defer decl->stdlib conversion into decl->location conversion

kadircet created this revision. kadircet added reviewers: hokein, sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. We preserve decls for stdlib symbols after this patch in symbol.

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:294 +} +using namespace ns1::ns2; +using namespace ns1::ns3; v1nh1shungry wrote: > kadircet wrote: > > sorry i am having trouble understanding

[PATCH] D137919: [clangd] use fine-grained code action kinds

kadircet added a comment. as discussed in https://github.com/clangd/clangd/issues/1326#issuecomment-1313502572, we've decided to not move forward with the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137919/new/

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

kadircet added a comment. thanks for pinging @tom-anders , i've added some concerns about the behaviour in general. sorry if these are discussed somewhere else but i've missed. Comment at: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:294 +} +using

[PATCH] D138047: Fix use of dangling stack allocated string in IncludeFixer

kadircet added a comment. yeah unfortunately testing this is hard, but bug are obvious so it's fine to land without a test. but i think we should rather fix the broken call site at clang-tools-extra/clangd/ParsedAST.cpp and move `BuildDir` to outer scope where it'll outlive IncludeFixer

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:100 + /// Contains all non self-contained files during the parsing. + llvm::DenseSet NonSelfContainedFiles; s/during/detected during/

[PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGae59131d3ef3: [clang][Tooling] Make the filename behaviour consistent (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

kadircet updated this revision to Diff 475362. kadircet added a comment. - Drop documentation - Call remove_dots on the common path Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137962/new/ https://reviews.llvm.org/D137962 Files:

[PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. Dotdots were removed only when converting a relative path to absolute

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

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/D137697/new/ https://reviews.llvm.org/D137697

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/HeaderAnalysis.h:28 +/// dont-include-me pattern heuristically. +bool isSelfContainedHeader(FileID FID, const SourceManager , + HeaderSearch ); oops

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

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/D137697/new/ https://reviews.llvm.org/D137697

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

kadircet added inline comments. Comment at: clang/lib/Tooling/Inclusions/Header.cpp:64 + // particular preprocessor state, usually set up by another header. + return !isDontIncludeMeHeader(SM.getBufferData(SM.translateFile(FE))); +} sammccall wrote: > kadircet

[PATCH] D137825: [clang-include-cleaner] make SymbolLocation a real class, move FindHeaders

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/D137825/new/ https://reviews.llvm.org/D137825

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/Header.h:21 +/// +/// A header is considered self-contained if either it has a proper header guard +/// or it doesn't has dont-include-me-similar pattern. kadircet wrote: > let's

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

kadircet added a comment. i guess this patch needs a rebase for other pieces as well? but LG in general Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:47 + const PragmaIncludes ) { + const FileEntry *FE =

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/Header.h:1 +//===--- Header.h *- C++-*-===// +// rather than calling this `Header` can we call this `HeaderAnalysis`?

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