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

kadircet updated this revision to Diff 520648. kadircet marked 3 inline comments as done. kadircet added a comment. Merge with existing tests Perform check earlier Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150187/new/

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

kadircet created this revision. kadircet added a reviewer: VitaNuo. Herald added subscribers: PiotrZSL, 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:

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

kadircet added a comment. (sorry for taking so long to get back to this) In D143260#4270807 , @nridge wrote: > In the case of labels (and angle brackets (D139926 > ) and operators (D136594 >

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

kadircet added a comment. > However, I'm not keen on us playing whack-a-mole with the kinds of checks > from this review. For starters, that's going to have a long-tail that makes > it hard to know if we've ever gotten to the bottom of the issue. But also, > each one of these checks is

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

kadircet created this revision. 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/D149948 Files:

[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

kadircet added a comment. I agree with Ilya's concerns here. We deliberately don't mess with compile flags pushed over LSP. These are "overrides" to whatever information we have from other sources, turning on interpolation at this override layer implies we'll never fallback to other sources

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

kadircet added a comment. Sorry I was trying to give some brief idea about what it might look like in `Implementation Concerns` section above, but let me give some more details; I think we can just change the signature for PreambleParsedCallback

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

kadircet added a comment. In D149733#4315360 , @erichkeane wrote: > I don't recall the case of a Null type being valid for functions (perhaps > Aaron does? Or perhaps its an error condition?). But otherwise, I would > expect `FunctionDecl` to have a

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

kadircet added a comment. FWIW, https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/Decl.cpp#L2987 is the assertion that suggests "null type is fine for functiondecls", but as pointed out pretty much all of the methods assume it's non-null && most of the users also don't check for

[PATCH] D146634: [clang][USR] Prevent crashes when parameter lists have nulls

kadircet abandoned this revision. kadircet added a comment. in favor of D149733 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146634/new/ https://reviews.llvm.org/D146634

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

kadircet created this revision. kadircet added reviewers: aaron.ballman, ilya-biryukov. 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.

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

kadircet added a comment. Thanks for the patch, this is a topic we've discussed a couple times in the past as well as it has far reaching implications. I believe we're actually in a much better situation nowadays. To summarise some concerns (thanks to @sammccall for useful discussion);

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

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:445 -Fix removeAllUnusedIncludes(llvm::ArrayRef UnusedIncludes) { +Fix removeAllUnusedIncludes(llvm::ArrayRef UnusedIncludes, +llvm::StringRef Code) {

[PATCH] D142967: [clangd] Introduce source.organizeImports code action.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1035 } + if (KindAllowed(CodeAction::SOURCE_ORGANIZE_IMPORT)) { +std::lock_guard Lock(FixItsMutex); instead of doing this in here, what about introducing a new

[PATCH] D149165: [clangd] Deduplicate missing-include findings

kadircet added a comment. Thanks @ArturGainullin ! you're right, i've sent out 5e74a3dc2da879d98204f2360e2e33571b93b91b . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

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

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:443 + RemoveAll.Annotations.push_back({RemoveAllUnusedID, + {/*label=*/"", /*needsConfirmation=*/true, +

[PATCH] D149165: [clangd] Deduplicate missing-include findings

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG08f0725a3a54: [clangd] Deduplicate missing-include findings (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D149165: [clangd] Deduplicate missing-include findings

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:419 }); + // Put possibly equal diagnostics together for deduplication. + // The duplicates might be from macro arguments that get expanded

[PATCH] D149165: [clangd] Deduplicate missing-include findings

kadircet updated this revision to Diff 517076. kadircet added a comment. - Reorganize string literals in tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149165/new/ https://reviews.llvm.org/D149165 Files:

[PATCH] D149165: [clangd] Deduplicate missing-include findings

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] D148783: [clangd] Add support TextDocumentEdit.

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:1694 -std::vector ClangdLSPServer::getFixes(llvm::StringRef File, -

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

kadircet added a comment. can you also add test coverage for the new LSP fields? Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:439 + for (auto : RemoveAll.Edits) { +E.annotationId.emplace(); +*E.annotationId = RemoveAllUnusedID; nit:

[PATCH] D148783: [clangd] Add support TextDocumentEdit.

kadircet added a comment. In D148783#4286652 , @hokein wrote: >> can you also have a version of the >> clang-tools-extra/clangd/test/fixits-command.test with documentChanges >> support? it's unlikely to have clients in that configuration but i believe

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

kadircet added a comment. Hi folks, the rationale for a clang-tidy check is enabling include-cleaner findings to be applied at scale and integrations into existing workflows (e.g. a lot of people run cleanups using clang-tidy findings hence there's somewhat existing infra for that). current

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

kadircet added a comment. thanks! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:143 + std::vector Macros; + for (const auto : AST.getMacros().MacroRefs) { +for (const auto : MAndRefs.second) { nit: `const auto &[SID, Refs]`

[PATCH] D148783: [clangd] Add support TextDocumentEdit.

kadircet added a comment. can you also have a version of the clang-tools-extra/clangd/test/fixits-command.test with `documentChanges` support? it's unlikely to have clients in that configuration but i believe the deserialization issue i mentioned above would be discoverable by such a test.

[PATCH] D147044: [clangd] Implement cross reference request for #include lines.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:398 + )cpp", + {include_cleaner::Header{"bar.h"},

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

kadircet added a comment. Hi folks! We have a setup in which clang (more specifically clang-tools) is always built from a version close to HEAD and libcxx is fetched from user's checkout of the codebase (which can lag behind HEAD for ~a month). So the 1 week gap between

[PATCH] D147044: [clangd] Implement cross reference request for #include lines.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.h:87 +std::optional +firstMatchedProvider(const include_cleaner::Includes , + llvm::ArrayRef Providers); VitaNuo wrote: > kadircet wrote: > > can you also

[PATCH] D148552: [include-cleaner] Unify behaviour for static & instance members

This revision was automatically updated to reflect the committed changes. Closed by commit rGcc5fb7a79b58: [include-cleaner] Unify behaviour for static instance members (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D148552: [include-cleaner] Unify behaviour for static & instance members

kadircet updated this revision to Diff 514546. kadircet marked 2 inline comments as done. kadircet added a comment. - Adress review comments - Handle Qualifier being null Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148552/new/

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

kadircet added a comment. before you dive any deeper into the patch, could you give some reasoning about why this is needed/useful? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/ https://reviews.llvm.org/D148489

[PATCH] D148552: [include-cleaner] Unify behaviour for static & instance members

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. Fixes https://github.com/llvm/llvm-project/issues/62185 Repository: rG

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:418 +Fix removeAllUnusedIncludes(llvm::ArrayRef UnusedIncludes) { + Fix RemoveAll; can we also derive these from an `llvm::ArrayRef` ? to make sure there can't be a

[PATCH] D146941: [clangd] Use all inputs to SystemIncludeExtractor in cache key

This revision was automatically updated to reflect the committed changes. Closed by commit rGf099f2fefbab: [clangd] Use all inputs to SystemIncludeExtractor in cache key (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D147808: [clangd] Support defaulted destructors in Define Outline tweak

kadircet added a comment. there's actually a slight difference between an inline defaulted special member function and an out-of-line defaulted one. the latter makes the special member "user-defined" which might cause various headaches (e.g. type is no longer "trivial"). i don't think we

[PATCH] D147802: [clangd] Handle destructors in DefineOutline tweak

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/tweaks/DefineOutline.cpp:186-194 + if (const auto *Destructor = llvm::dyn_cast(FD)) { +if (auto Err =

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

kadircet added a comment. sorry for long silence. > But access specifiers are a completely different thing semantically, that's > the point: The user does not tell the client: "I want everything that is > followed by a single colon in this color"; that would be silly. They say "I > want goto

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

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SystemIncludeExtractor.cpp:340 +// is not installed. +if (Lang == "objective-c++-header") { + Lang = "c++-header"; nridge wrote: > kadircet wrote: > > this feels like too much of a

[PATCH] D148213: [clangd] Use FileEntryRef for canonicalizing filepaths.

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

[PATCH] D148213: [clangd] Use FileEntryRef for canonicalizing filepaths.

kadircet added a comment. we've got one more reference to `getCanonicalPath` in `clang-tools-extra/clangd/IncludeCleaner.cpp:320`, i guess the best way is just calling `getLastRef` on the FileEntry* Comment at: clang-tools-extra/clangd/SourceCode.cpp:518

[PATCH] D148158: [include-cleaner] Handle incomplete template specializations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3d6d2ae6f490: [include-cleaner] Handle incomplete template specializations (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D148158: [include-cleaner] Handle incomplete template specializations

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:163 ElementsAre(Decl::ClassTemplatePartialSpecialization)); + // Incomplete templates don't have a specific specialization associated. +

[PATCH] D148158: [include-cleaner] Handle incomplete template specializations

kadircet updated this revision to Diff 513173. kadircet marked 2 inline comments as done. kadircet added a comment. - Update comments & test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148158/new/ https://reviews.llvm.org/D148158 Files:

[PATCH] D146941: [clangd] Use all inputs to SystemIncludeExtractor in cache key

kadircet updated this revision to Diff 513098. kadircet marked an inline comment as done. kadircet added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146941/new/ https://reviews.llvm.org/D146941 Files:

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

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SystemIncludeExtractor.cpp:340 +// is not installed. +if (Lang == "objective-c++-header") { + Lang = "c++-header"; this feels like too much of a layering violation and might

[PATCH] D147686: [clangd] Fix a nullptr-dereference crash in computeIncludeCleanerFindings.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:450 +void test() { + 1s; +} hokein wrote: > here is the `UserDefinedLiteral` AST node: > > ``` > `-UserDefinedLiteral 0x5556682e4500 'int' >

[PATCH] D146941: [clangd] Use all inputs to SystemIncludeExtractor in cache key

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

[PATCH] D146941: [clangd] Use all inputs to SystemIncludeExtractor in cache key

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/SystemIncludeExtractor.cpp:80 + std::string Driver; + std::string Directory; + // Whether certain includes should be part of query. nridge wrote: >

[PATCH] D148158: [include-cleaner] Handle incomplete template specializations

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-tools-extra. Herald added a subscriber: cfe-commits. Instantiation pattern is null for incomplete template types and using

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

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 rG144562e678d9: [clangd] Treat preamble patch as main file for include-cleaner analysis (authored by

[PATCH] D148112: [include-cleaner] Improve handling for templates

kadircet added a comment. caea93cc4478cca28321cba4fa2871d5e6090299 should fix the issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148112/new/

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

kadircet updated this revision to Diff 512898. kadircet added a comment. - Expose helper to get preamble patch entry Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148143/new/ https://reviews.llvm.org/D148143 Files:

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

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. Since we

[PATCH] D148112: [include-cleaner] Improve handling for templates

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG34f5774920f5: [include-cleaner] Improve handling for templates (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D148112: [include-cleaner] Improve handling for templates

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:109 ADD_FAILURE() << DiagBuf << "\nfrom code:\n" << ReferencingCode; + llvm::sort(TargetDeclKinds); +

[PATCH] D148112: [include-cleaner] Improve handling for templates

kadircet updated this revision to Diff 512851. kadircet marked an inline comment as done. kadircet added a comment. Herald added a subscriber: ChuanqiXu. - Ignore explicit instantiations - Update tests to use decls Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D148112: [include-cleaner] Improve handling for templates

kadircet updated this revision to Diff 512775. kadircet edited the summary of this revision. kadircet added a comment. - Use template instantion pattern helper instead of dealing with partial specializations Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D148112: [include-cleaner] Improve handling for templates

kadircet created this revision. 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. Principal here is: - Making sure each template instantiation implies use

[PATCH] D147686: [clangd] Fix a nullptr-dereference crash in computeIncludeCleanerFindings.

kadircet added a comment. could you give some more details about the crash, so that we actually now where to fix it? e.g. is it token buffers missing the spelled token at that offset somehow ? or is it AST reporting a weird source locations for user-defined literals and we need to do some sort

[PATCH] D147449: [include-cleaner] Only ignore builtins without a header

This revision was automatically updated to reflect the committed changes. Closed by commit rG3402b77db3ee: [include-cleaner] Only ignore builtins without a header (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D147449?vs=510516=510746#toc Repository: rG LLVM

[PATCH] D147449: [include-cleaner] Only ignore builtins without a header

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. Certain standard library functions (e.g. std::move) are also implemented

[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit

This revision was automatically updated to reflect the committed changes. Closed by commit rG26ff268b80c5: [include-cleaner] Report references to operator calls as implicit (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit

kadircet updated this revision to Diff 510474. kadircet marked an inline comment as done. kadircet added a comment. - Update to treat operator calls to members as "explicit" fater offline discussions. This better aligns with what we do for regular member exprs. Repository: rG LLVM Github

[PATCH] D147044: [clangd] Implement cross reference request for #include lines.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:443 + // No match for this provider in the includes list. + return {}; +} `return std::nullopt` Comment at: clang-tools-extra/clangd/IncludeCleaner.h:87

[PATCH] D147044: [clangd] Implement cross reference request for #include lines.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1338 + +auto ReferencedInclude = convertIncludes(SM, Inc); +include_cleaner::walkUsed( can we put the rest into a separate function (I know this function is already quite

[PATCH] D147139: [clangd] Map references from include'd files to directives

This revision was automatically updated to reflect the committed changes. Closed by commit rGf3a815aa776f: [clangd] Map references from included files to directives (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147139/new/

[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit

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. Missing these references can result in false negatives in the used-ness

[PATCH] D147139: [clangd] Map references from include'd files to directives

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. Repository:

[PATCH] D146717: [clangd] Handle the C++2b elifdef and elindef PP structure in CollectMainFileMacros.

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

[PATCH] D146244: [clangd] Show used symbols on #include line hover.

kadircet accepted this revision. kadircet added a comment. thanks the pieces i looked at LG too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146244/new/ https://reviews.llvm.org/D146244 ___ cfe-commits

[PATCH] D146971: [Sema] Populate declarations inside TypeLocs for some invalid types

kadircet added a comment. i've added a reproducer for https://reviews.llvm.org/D146634, can you patch that into here? unfortunately i couldn't get it to crash in any place other than through clangd. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D146634: [clang][USR] Prevent crashes when parameter lists have nulls

kadircet updated this revision to Diff 508934. kadircet added a comment. Herald added a project: clang-tools-extra. - Add test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146634/new/ https://reviews.llvm.org/D146634 Files:

[PATCH] D146244: [clangd] Show used symbols on #include line hover.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1173 +if (Ref.RT != include_cleaner::RefType::Explicit || +UsedSymbolNames.find(getRefName(Ref)) != UsedSymbolNames.end()) + return; creating these symbol

[PATCH] D146727: [clangd] Use expansion location for missing include diagnostics.

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

[PATCH] D146727: [clangd] Use expansion location for missing include diagnostics.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:380 +auto Loc = SM.getFileLoc(Ref.RefLocation); +auto Range = Lexer::makeFileCharRange( +CharSourceRange{SourceRange{Loc}, true}, SM, AST.getLangOpts());

[PATCH] D146727: [clangd] Use expansion location for missing include diagnostics.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:380 +auto Loc = SM.getFileLoc(Ref.RefLocation); +auto Range = Lexer::makeFileCharRange( +CharSourceRange{SourceRange{Loc}, true}, SM, AST.getLangOpts());

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

kadircet added a comment. Sorry I was waiting for D139277 to land, but it seems to be stuck so sent out D146941 . Let's take a look at this again after that one lands CHANGES SINCE LAST ACTION

[PATCH] D146941: [clangd] Use all inputs to SystemIncludeExtractor in cache key

kadircet created this revision. kadircet added a reviewer: nridge. 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. Instead of

[PATCH] D146916: [include-cleaner] Fix crash on unresolved headers

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 rGf5b6e9b6d355: [include-cleaner] Fix crash on unresolved headers

[PATCH] D146916: [include-cleaner] Fix crash on unresolved headers

kadircet created this revision. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Make sure unresolved headers are not analyzed as part of unused includes. Also introduces a testing fixture

[PATCH] D146244: [clangd] Show used symbols on #include line hover.

kadircet added a comment. (pardon the interruption, some drive-by comments :)) Comment at: clang-tools-extra/clangd/Hover.cpp:1174 + return; + +for (const include_cleaner::Header : Providers) { note that we don't care about each reference of

[PATCH] D146732: [include-cleaner] Attribute references to explicit specializations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG03101e141bf7: [include-cleaner] Attribute references to explicit specializations (authored by kadircet). Repository: rG LLVM Github Monorepo

[PATCH] D146732: [include-cleaner] Attribute references to explicit specializations

kadircet updated this revision to Diff 508025. kadircet marked an inline comment as done. kadircet added a comment. - use isa_and_present - indentation for tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146732/new/

[PATCH] D146732: [include-cleaner] Attribute references to explicit specializations

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:82 +if (llvm::isa(ND) || !RD) + return ND; +return RD; hokein wrote: > We seem to miss a testcase for

[PATCH] D146727: [clangd] Use expansion location for missing include diagnostics.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:382 -auto = AST.getTokens(); -auto SpelledForExpanded = -Tokens.spelledForExpanded(Tokens.expandedTokens(Ref.RefLocation)); -if (!SpelledForExpanded) +

[PATCH] D146717: [clangd] Handle the C++2b elifdef and elindef PP structure in CollectMainFileMacros.

kadircet added a comment. thanks, LG apart from the using decls Comment at: clang-tools-extra/clangd/CollectMacros.h:69 const MacroDefinition ) override; + using PPCallbacks::Elifdef; + using PPCallbacks::Elifndef; why do we have these using

[PATCH] D146732: [include-cleaner] Attribute references to explicit specializations

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. Fixes https://github.com/llvm/llvm-project/issues/61652 Repository: rG

[PATCH] D146406: [IncludeCleaner][clangd] Mark umbrella headers as users of private

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG43fcfdb1d6a6: [IncludeCleaner][clangd] Mark umbrella headers as users of private (authored by kadircet). Repository: rG LLVM Github Monorepo

[PATCH] D146406: [IncludeCleaner][clangd] Mark umbrella headers as users of private

kadircet updated this revision to Diff 507701. kadircet added a comment. - address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146406/new/ https://reviews.llvm.org/D146406 Files: clang-tools-extra/clangd/IncludeCleaner.cpp

[PATCH] D146406: [IncludeCleaner][clangd] Mark umbrella headers as users of private

kadircet marked 2 inline comments as done. kadircet added a comment. thanks a lot for the review! Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:100 + // Check if main file is the public interface for a private header. If so + // we shouldn't diagnose

[PATCH] D146634: [clang][USR] Prevent crashes when parameter lists have nulls

kadircet added a comment. I am aware that this null checking at leaves are not considered a sustainable solution and I agree with the sentiment there. But we're seeing an increasing number of crashes in production on invalid code recently. Happy to take a different course if there are

[PATCH] D146634: [clang][USR] Prevent crashes when parameter lists have nulls

kadircet created this revision. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Similar to D146426 Repository: rG LLVM Github

[PATCH] D146417: [clangd] Fix AddUsing in the face of typo-correction

kadircet closed this revision. kadircet added a comment. relanded in 35c2aac6e3957c2e82bf92269039fa02bab0e1d9 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146417/new/

[PATCH] D146490: [Support] On Windows, ensure that UniqueID is really stable

kadircet added a comment. In D146490#4209495 , @aganea wrote: > Fair enough. There are several choices forward: either we mark the issue as > "Will Not Fix" or I can try only scoping this patch to only keep the handle > open for network drives/paths.

[PATCH] D146490: [Support] On Windows, ensure that UniqueID is really stable

kadircet added a comment. > To fix the issue, we keep the file handles open during the lifetime of their > corresponding UniqueID instances. Since handles will live longer now, this > requires particular attention when performing some file actions, such as file > deletions. I am a little

[PATCH] D146417: [clangd] Fix AddUsing in the face of typo-correction

kadircet updated this revision to Diff 506723. kadircet added a comment. Disable msvc compatibility to enable typo-correction on windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146417/new/ https://reviews.llvm.org/D146417 Files:

[PATCH] D145843: [clangd] Add option to always insert headers with <> instead of ""

kadircet added a subscriber: sammccall. kadircet added a comment. In D145843#4207101 , @nridge wrote: > My understanding is that a more elaborate configuration scheme has been > proposed in https://github.com/clangd/clangd/issues/1367, and the feedback

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. as discussed offline, this feels a little fishy and we should probably try and not put nulls into the parameter lists at all (and mark the functiontype as invalid instead), but since i

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