[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

kadircet updated this revision to Diff 470558. kadircet marked 5 inline comments as done. kadircet added a comment. - Rename Related to Ambigious in RefKind - Add more tests for implicit constructor calls - Default to Explicit as RefKind when reporting references - Don't report unused overloads, u

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:107 TEST(WalkAST, DeclRef) { - testWalk("int ^x;", "int y = ^x;"); - testWalk("int ^foo();", "int y = ^foo();"); - testWalk("namespace ns { int ^x; }", "int y = ns::^x;");

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

kadircet updated this revision to Diff 470560. kadircet marked an inline comment as done. kadircet added a comment. - Clarify default constructor call example in RefType::Implicit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135859/new/ https://re

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:33 +/// Indicates the relation between the reference and the target. +enum class RefType { + /// Target is explicit from the reference, e.g. function call. hokei

[PATCH] D136723: [include-cleaner] Record main-file macro occurences and includes

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:54 +struct RecordedPP { + // The callback (when installed into clang) tracks macros/includes in this. + std::unique_ptr record(const Preprocessor &PP);

[PATCH] D136071: [include-cleaner] WIP: Add PragmaIncludes which handles include-mapping pragmas.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:35 +// Captures #include mapping information. It analyses IWYU Pragma comments and +// other use-instead-like mechanisms (#pragma include_instead) on included -

[PATCH] D136710: [include-cleaner] Add the missing parts of Symbol and Header clases.

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:49 + Kind kind() const { return static_cast(Storage.index()); } + bool operat

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

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/Symbol.h:93 +Import = 2, + }; + you can use `LLVM_MARK_AS_BITMASK_ENUM` and relevant magic to implement bitwise operators on the type (see llvm-project/llvm/include/llvm/ADT/BitmaskE

[PATCH] D136844: [libclang] Expose completion result kind in `CXCompletionResult`

kadircet added inline comments. Comment at: clang/include/clang/Sema/CodeCompleteConsumer.h:755 - /// Describes the kind of result generated. - enum ResultKind { -/// Refers to a declaration. i don't follow the reason for replacing this struct with `CXComp

[PATCH] D136710: [include-cleaner] Add the missing parts of Symbol and Header clases.

kadircet added a comment. thanks, i agree with your comments around `std::variant`, let's see what kind of patterns will emerge in the future. so, still LG, but please address the triple slash issue and const-ness before landing Comment at: clang-tools-extra/include-cleaner

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

kadircet marked 6 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:33 +/// Indicates the relation between the reference and the target. +enum class RefType { + /// Target is explicit from the reference,

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

kadircet updated this revision to Diff 471512. kadircet added a comment. - Fix typo in Ambiguous - Rebase for Types.h, introduce SymbolReference - Use comments rather than an optional parameter to indicate header-ness of main files in tests - Rather than assuming Root is in main file, check that

[PATCH] D136951: [clangd] Populate ranges and symbol origin for paramname completions

kadircet created this revision. kadircet added reviewers: nridge, 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. Reposito

[PATCH] D136844: [libclang] Expose completion result kind in `CXCompletionResult`

kadircet added inline comments. Comment at: clang/include/clang/Sema/CodeCompleteConsumer.h:755 - /// Describes the kind of result generated. - enum ResultKind { -/// Refers to a declaration. egorzhdan wrote: > kadircet wrote: > > i don't follow the reason

[PATCH] D137056: [clangd] Fix a small inconsistency in system-include-extractor.test

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/test/system-include-extractor.test:14-17 +# The purpose of the three lines below is to test that the -nostdinc etc. +# argume

[PATCH] D137064: [clangd] Fix a semantic-highlighting crash.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:664 bool VisitCXXMemberCallExpr(CXXMemberCallExpr *CE) { -if (isa(CE->getMethodDecl())) { +

[PATCH] D137063: [clangd] Run semantic highligting in clangd check.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/tool/Check.cpp:212 +log("Building semantic highlighting"); +getSemanticHighlightings(*AST); + } can you also vlog the highlights, while limitting the output to ranges that intersect wi

[PATCH] D136951: [clangd] Populate ranges and symbol origin for paramname completions

This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rGda5ded4fc9d8: [clangd] Populate ranges and symbol origin for paramname completions (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D1

[PATCH] D136071: [include-cleaner] Add PragmaIncludes which handles include-mapping pragmas.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LG from my side as well, just some small tweaks for tests. Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:124 + TestAST Processed = build

[PATCH] D137063: [clangd] Run semantic highligting in clangd check.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/tool/Check.cpp:213 +auto Highlights = getSemanticHighlightings(*AST); +if (LineRange) { + for (const auto HL : H

[PATCH] D130363: [clang] Give priority to Class context while parsing declarations

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! let me know if i should land this for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130363/new/ https://reviews.llvm.org/

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:140 for (const Expr *Init : Sem->inits()) { +if (!Init) + continue; we should have this bail out after introducing the scope_exit below to make sure we skip the field

[PATCH] D131724: [pseudo] Eliminate an ambiguity for the empty member declaration.

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

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/InlayHints.cpp:144 }); -if (llvm::isa(Init)) +if (!Init || llvm::isa(Init)) continue; // a "hole" for a subob

[PATCH] D131385: [clangd] Support for standard type hierarchy

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Protocol.cpp:1228 +Result["parents"] = RP.parents; + return std::move(Result); +} usaxena95 wrote: > Nit: Allow RVO. there's an issue with one of

[PATCH] D131385: [clangd] Support for standard type hierarchy

kadircet updated this revision to Diff 452927. 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/D131385/new/ https://reviews.llvm.org/D131385 Files: clang-tools-

[PATCH] D131385: [clangd] Support for standard type hierarchy

kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1713 + fillSuperTypes(*ParentDecl, TUPath, *ParentSym, RPSet); + Item.data.parents->emplace_back(ParentSym->data); + Item.parents->emplace_back

[PATCH] D131385: [clangd] Support for standard type hierarchy

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 rG83411bf06f34: [clangd] Support for standard type hierarchy (authored by kadircet). Repository: rG LLVM

[PATCH] D130363: [clang] Give priority to Class context while parsing declarations

This revision was automatically updated to reflect the committed changes. Closed by commit rG4dd71b3cb947: [clang] Give priority to Class context while parsing declarations (authored by furkanusta, committed by kadircet). Changed prior to commit: https://reviews.llvm.org/D130363?vs=451604&id=45

[PATCH] D131088: [clang] Apply FixIts to members declared via `using` in derived classes

This revision was automatically updated to reflect the committed changes. Closed by commit rG8837ef4d373b: [clang] Apply FixIts to members declared via `using` in derived classes (authored by denis-fatkulin, committed by kadircet). Changed prior to commit: https://reviews.llvm.org/D131088?vs=45

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

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-tools-extra. This brings IncludeCleaner's reference discovery from AST

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

kadircet updated this revision to Diff 453638. kadircet marked 5 inline comments as done. kadircet added a comment. - Adress comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132110/new/ https://reviews.llvm.org/D132110 Files: clang-tools-e

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:55 + bool VisitMemberExpr(MemberExpr *E) { +auto *MD = E->getMemberDecl(); +report(E->getMemberLoc(), MD); sammccall wrote: > sammccall wrote: > > nit: inline?

[PATCH] D133979: [clangd] XRef functions treat visible class definitions as primary.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:291 + // (This exception matches the spirit of our indexing heuristics). + if (/*PreferDef=*/[&] { +if (Def == TD) can we extract all of this into a `const TagDecl* g

[PATCH] D133968: [clangd] Enable standard library index by default.

kadircet added a comment. thanks, LG but seems to be failing on windows :/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133968/new/ https://reviews.llvm.org/D133968 ___ cfe-commits mailing list cfe-comm

[PATCH] D128462: [HLSL] add -I option for dxc mode.

kadircet added a comment. In D128462#3793266 , @beanz wrote: > I think we had no expectation that DXC-mode would be supported by the tooling > APIs at this point. Right, and as I mentioned in my previous comments this is completely fine, at least in th

[PATCH] D133962: [clang(d)] Include/Exclude CLDXC options properly

This revision was automatically updated to reflect the committed changes. Closed by commit rG23ace26e0d1a: [clang(d)] Include/Exclude CLDXC options properly (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133962/new/ https://re

[PATCH] D133962: [clang(d)] Include/Exclude CLDXC options properly

kadircet added a comment. asked for a backport for 15.0.1 in https://github.com/llvm/llvm-project/issues/57779 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133962/new/ https://reviews.llvm.org/D133962

[PATCH] D133757: [clangd] Turn QueryDriverDatabase into a CompileCommandsAdjuster

kadircet added a comment. In D133757#3792930 , @nridge wrote: > Thanks for the feedback! Yeah I'd be wary of introducing a corner case where > the user passes `--query-driver`, and there is in fact a driver available to > query in `PATH`, but we end up

[PATCH] D134137: [clangd] Return earlier when snippet is empty

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1017 +TEST(CompletionTests, EmptySnippetDoesNotCrash) { +// See https://github.com/clangd/clangd/issues/1216 tom-anders wrote: > nridge wrote: > > Does this

[PATCH] D134379: [clangd] IncludeCleaner: handle using namespace

kadircet added a comment. In D134379#3807770 , @ArcsinX wrote: > Anyway if this is the only concern, we can handle namespace declaration as a > special case inside `ReferencedLocationCrawler::add()`. something like this: > > diff > -for (const De

[PATCH] D132918: [clang] Fix a crash in constant evaluation

kadircet closed this revision. kadircet added a comment. Landed as abd2b1a9d0421f99d3d132dc99af55ae52f3ac3e Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132918/new/ https://rev

[PATCH] D135161: [clang][Lex] Fix a crash on malformed string literals

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/D135161 Files: clang

[PATCH] D119130: [clangd] NFC: Move stdlib headers handling to Clang

kadircet added a comment. yeah sorry for the mess here. i agree that we should move StandardLibrary to its own library. i'll try to take a stab at this tomorrow, if no one does it before then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119130/n

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

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. As pointed out in https://rev

[PATCH] D119130: [clangd] NFC: Move stdlib headers handling to Clang

kadircet added a comment. sent out https://reviews.llvm.org/D135245 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119130/new/ https://reviews.llvm.org/D119130 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D135161: [clang][Lex] Fix a crash on malformed string literals

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG36a200208fac: [clang][Lex] Fix a crash on malformed string literals (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D135161: [clang][Lex] Fix a crash on malformed string literals

kadircet added a comment. this is causing some crashes in buildbots, and i can't repro. reverting until i figure this out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135161/new/ https://reviews.llvm.org/D135161

[PATCH] D135254: [clang][Sema] Fix crash on invalid base destructor

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. LookupSpecialMember might fail, so changes the cast to cast_or_null. Inside Sema, skip

[PATCH] D135257: [clangd][Tweak] Make sure enclosing function doesnt have invalid children

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: r

[PATCH] D135257: [clangd][Tweak] Make sure enclosing function doesnt have invalid children

kadircet added a comment. This is triggering crashes in our production setup, but I couldn't come up with a reduced test case. crash is triggered in the loop: for (const auto *S : EnclosingFunction->getBody()->children()) { if (SM.isBeforeInTranslationUnit(S->getSourceRange().getEnd()

[PATCH] D135254: [clang][Sema] Fix crash on invalid base destructor

kadircet updated this revision to Diff 465365. 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/D135254/new/ https://reviews.llvm.org/D135254 Files: clang/lib/Sem

[PATCH] D135254: [clang][Sema] Fix crash on invalid base destructor

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGadab08ecf2bd: [clang][Sema] Fix crash on invalid base destructor (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D134618: [Syntax] Fix macro-arg handling in TokenBuffer::spelledForExpanded

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:82 + // When First and Last are part of the *same macro arg* of a macro written + // in the main file, we

[PATCH] D135257: [clangd][Tweak] Make sure enclosing function doesnt have invalid children

kadircet added a comment. In D135257#3836511 , @ilya-biryukov wrote: > Having `nullptr` inside `children()` seems really weird. Should we fix those > instead to never produce `nullptr`? Or is this something that is expected (I > can come up with a few

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/CMakeLists.txt:163 clangToolingInclusions + clangToolingInclusionsSTL clangToolingSyntax sammccall wrote: > StandardLibrary or Stdlib? > > STL

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

kadircet updated this revision to Diff 465671. kadircet marked 2 inline comments as done. kadircet added a comment. - Rename STL to Stdlib Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135245/new/ https://reviews.llvm.org/D135245 Files: clang-to

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd1f13c54f172: [clang][Tooling] Move STL recognizer to its own library (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D1

[PATCH] D135314: [clangd] Avoid scanning up to end of file on each comment!

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. wow, that's an interesting finding. thanks! Comment at: clang-tools-extra/clangd/Headers.cpp:25 -const char IWYUPragmaKeep[] = "// IWYU pragma: keep"; -const char IWYUP

[PATCH] D135161: [clang][Lex] Fix a crash on malformed string literals

kadircet added a comment. i'll reland the fix without the test case, as it's clearly fixing one of the codepaths that'll lead to a crash. it's only the test case that's crashing, because i don't think there are certain test cases that exposed literal parser to invalid/incomplete input and i am

[PATCH] D135356: [Format] Fix crash when hitting eof while lexing JS template string

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang/lib/Format/FormatTokenLexer.cpp:767 if (Offset[0] == '\\') { ++Offset; // Skip the escaped character. } else if (Offset + 1 < L

[PATCH] D135257: [clangd][Tweak] Make sure enclosing function doesnt have invalid children

kadircet added a comment. I wasn't claiming that there are legitimate reasons for having null children, I completely agree with your verdict on this one. The only one I can think of is cases like this, i.e. you don't want to perform some analysis if function body has invalid code in it. but i t

[PATCH] D135257: [clangd][Tweak] Make sure enclosing function doesnt have invalid children

This revision was automatically updated to reflect the committed changes. Closed by commit rGe09aa0d192e0: [clangd][Tweak] Make sure enclosing function doesnt have invalid children (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

kadircet added a comment. i also would rather not have the workaround solely for an editor (we usually try to address these on editor/plugin side). i am also worried about the understanding of that inferred line afterwards (e.g. what if editor thinks that line doesn't have a trailing `\n` and s

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

kadircet created this revision. kadircet added a reviewer: hokein. 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. RefTypes are distinct categories for eac

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

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. Creates a one to many mapping,

[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 Fi

[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

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 LAS

[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 +LO(*CI.getLangOpt

[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 `cla

[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 &LangOpts, could you rather move this to `AST.h`, with some com

[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] 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] 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 comple

[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 confi

[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) + "::"); tom-ander

[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. + Symbol::IncludeDirecti

[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) + "::"); tom-ander

[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] 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] 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 `Symb

[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/ https://reviews.llvm.org/D13

[PATCH] D141467: [clang][Driver][CUDA] Get rid of unused LibPath

kadircet created this revision. kadircet added a reviewer: tra. Herald added subscribers: mattd, yaxunl. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. LibPath discovered during InstallationDet

[PATCH] D141495: [clangd] Suppress clang-tidy warnings for code spelled in system macros

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

[PATCH] D121593: [clangd][WIP] Provide clang-include-cleaner

kadircet added a comment. Hi @Abpostelnicu, as @nridge pointed out. we're actively working on the library pieces of include-cleaner and applications of the library aren't the focus yet. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121593/new/ htt

[PATCH] D137252: [include-cleaner] Testing helpers for ast walking

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. Address comments around verbos

[PATCH] D137320: [include-cleaner] Initial version for the "Location=>Header" step

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:54 Header(tooling::stdlib::Header H) : Storage(H) {} + Header(llvm::StringRef VerbatimSpelling) : Storage(VerbatimSpelling.str()) {} can you

[PATCH] D137319: [include-cleaner] Add export IWYU pragma support.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:82 + llvm::DenseMap> + IWYUExportBy; what about a smallvector, instead of a denseset here? Comment at: clang-tools-extr

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

kadircet updated this revision to Diff 472925. kadircet marked 5 inline comments as done. kadircet added a comment. - Fix typo - Change comment - Use Types.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135859/new/ https://reviews.llvm.org/D135859

[PATCH] D137320: [include-cleaner] Initial version for the "Location=>Header" step

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:54 + /// A verbatim header spelling, a string quoted with <> or "" that can be + /// #included directly + Header(StringRef VerbatimSpelling) : Storage(VerbatimS

[PATCH] D137252: [include-cleaner] Testing helpers for ast walking

kadircet marked an inline comment as done. kadircet added a comment. In D137252#3907665 , @hokein wrote: > Just share my experience when reading the code, overall I have a feeling that > we're regressing the code readability of the tests (left some comme

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

kadircet added inline comments. Comment at: clang/lib/Support/RISCVVIntrinsicUtils.cpp:35 +// Concat BasicType, LMUL and Proto as key +static std::unordered_map LegalTypes; +static std::set IllegalTypes; these were previously owned by `RVVEmitter`, hence we would

[PATCH] D137338: Fix dupe word typos

kadircet added a comment. LGTM for changes under `clang-tools-extra/clangd` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137338/new/ https://reviews.llvm.org/D137338 ___ cfe-commits mailing list cfe-com

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

This revision was automatically updated to reflect the committed changes. Closed by commit rGd19ba74dee0b: [Includecleaner] Introduce RefType to ast walking (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D135859?vs=472925&id=474001#toc Repository: rG LLVM Github Mo

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:25 namespace clang::include_cleaner { namespace { +using DeclCallback = tschuett wrote: > There is a cuter way to use anonymous namespaces: > https://llvm.org/docs/Cod

[PATCH] D137644: [include-cleaner] pass through recorded macro refs in walkUsed

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/Analysis.h:47 /// (e.g. IWYU pragmas). -void walkUsed(llvm::ArrayRef ASTRoots,

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

kadircet added a comment. 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 clearly introducing data races. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D137650: [clangd] Implement hover for string literals

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:816 + if (llvm::isa(E)) { +HoverInfo::PrintedType PT; +PT.Type = E->getType().getAsString(PP); can you extract this into a function, similar to the cases below?

[PATCH] D137650: [clangd] Implement hover for string literals

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

<    21   22   23   24   25   26   27   28   29   30   >