[PATCH] D50154: [clangd] capitalize diagnostic messages

arphaman added a comment. In https://reviews.llvm.org/D50154#1191002, @dblaikie wrote: > What's the motivation for clangd to differ from clang here? (& if the first > letter is going to be capitalized, should there be a period at the end? But > also the phrasing of most/all diagnostic text isn'

[PATCH] D50415: [clangd] extend the publishDiagnostics response to send back fixits to the client directly as well (if requested)

arphaman created this revision. arphaman added reviewers: jkorous, sammccall, ilya-biryukov. Herald added subscribers: dexonsmith, MaskRay, ioeric. This change extends the 'textDocument/publishDiagnostics' notification sent from Clangd to the client. The extension can be enabled using the '-fixi

[PATCH] D50415: [clangd] extend the publishDiagnostics response to send back fixits to the client directly as well (if requested)

arphaman updated this revision to Diff 159760. arphaman added a comment. - Client now can request the fixes using a 'clientCapabilities/textDocument/publishDiagnostics' extension. - Use 'Fixes' instead of 'Fixits' in code. https://reviews.llvm.org/D50415 Files: clangd/ClangdLSPServer.cpp c

[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

arphaman added a comment. This seems sensible to me. Comment at: include/clang/Basic/Diagnostic.h:764 + /// off extra processing that might be wasteful in this case. +bool areDiagnosticsSuppressedAfterFatalError() const { +return FatalErrorOccurred && SuppressAfterFata

[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

arphaman added a comment. On a second look I think that there is value separating the concepts of 'producing diagnostics' after hitting the fatal error (which is SuppressAfterFatalError currently does), and trying to build a more complete AST. For example, - An editor client might not want to

[PATCH] D50415: [clangd] extend the publishDiagnostics response to send back fixits to the client directly as well (if requested)

arphaman updated this revision to Diff 160007. arphaman marked an inline comment as done. arphaman added a comment. remove parameter. https://reviews.llvm.org/D50415 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/Diagnostics.h clangd/Protocol.cpp clangd/Protocol.h

[PATCH] D50415: [clangd] extend the publishDiagnostics response to send back fixits to the client directly as well (if requested)

This revision was automatically updated to reflect the committed changes. arphaman marked an inline comment as done. Closed by commit rCTE339454: [clangd] extend the publishDiagnostics response to send back fixits to the (authored by arphaman, committed by ). Changed prior to commit: https://re

[PATCH] D50571: [clangd] add an extension field to LSP to transfer the diagnostic's category

arphaman created this revision. arphaman added reviewers: jkorous, ilya-biryukov, sammccall. Herald added subscribers: dexonsmith, MaskRay, ioeric. This patch adds a 'category' extension field to the LSP diagnostic that's sent by Clangd. This extension is always on by default. Repository: rCT

[PATCH] D50571: [clangd] add an extension field to LSP to transfer the diagnostic's category

arphaman updated this revision to Diff 160464. arphaman marked 2 inline comments as done. arphaman added a comment. Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50571 Files: clangd/ClangdLSPServer.cpp clangd/Diagnostics.cpp clangd/Diagnostics.h

[PATCH] D50641: [clangd][test] Fix exit messages in tests

arphaman added a comment. Thanks for fixing this! You're right we should try to fix it properly to avoid such mistakes in the future. Checking for stderr from Clangd might work, but I don't think it's the most optimal solution. What do you think about Clangd exiting with failure on malformed J

[PATCH] D50740: [SourceManager] isPointWithin: avoid using isBeforeInTranslationUnit, compare buffer offsets directly for lexical correctness

arphaman created this revision. arphaman added reviewers: jkorous, klimek, ioeric, vsapsai, ilya-biryukov. Herald added a subscriber: dexonsmith. The current implementation of `isPointWithin` uses `isBeforeInTranslationUnit` to check if a location is within a range. The problem is that `isPointWi

[PATCH] D50571: [clangd] add an extension field to LSP to transfer the diagnostic's category

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE339738: [clangd] add an extension field to LSP to transfer the diagnostic's category (authored by arphaman, committed by ). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50571 Files:

[PATCH] D50785: [clangd][tests] Add exit(EXIT_FAILURE) in case of JSON parsing failure in TestMode

arphaman added a comment. I think propagating the 'test' yes/no value is not the best way to describe the intention of this change. Our intention is to exit from the LSP server on JSON error. One way to describe this intention better is to propagate a boolean 'exitOnJSONError' value. Furthermore

[PATCH] D50801: [rename] Use isPointWithin when looking for a declaration at location

arphaman created this revision. arphaman added reviewers: jkorous, hokein, ioeric. Herald added a subscriber: dexonsmith. This patch is a followup to https://reviews.llvm.org/D50740 . This patch fixes the issue where clang-refactor local-rename was unable to find a declaration in a header file i

[PATCH] D50814: [clangd] transfer the fixits with the notes instead of adding them to the main diagnostic if request by the client

arphaman created this revision. arphaman added reviewers: ilya-biryukov, jkorous, sammccall. Herald added subscribers: cfe-commits, dexonsmith, MaskRay, ioeric, javed.absar. This patch extends Clangd to allow the clients to specify the preference for how it wants to consume the fixes on the notes

[PATCH] D50740: [SourceManager] isPointWithin: avoid using isBeforeInTranslationUnit, compare buffer offsets directly for lexical correctness

arphaman added a comment. In https://reviews.llvm.org/D50740#1202248, @jkorous wrote: > Hi Alex, nice work! > > I am just wondering if it would be beneficial to assert Loc and End are in > the same file. It might help to catch bugs. I don't see the value in that unless I'm misunderstanding som

[PATCH] D50740: [SourceManager] isPointWithin: avoid using isBeforeInTranslationUnit, compare buffer offsets directly for lexical correctness

arphaman marked 2 inline comments as done. arphaman added inline comments. Comment at: lib/Basic/SourceManager.cpp:2035 + "Passed invalid source location!"); + assert(Start.isFileID() && End.isFileID() && Loc.isFileID() && + "Passed non-file source location!"); -

[PATCH] D50740: [SourceManager] isPointWithin: avoid using isBeforeInTranslationUnit, compare buffer offsets directly for lexical correctness

arphaman updated this revision to Diff 161132. arphaman marked an inline comment as done. arphaman added a comment. - Use lambda - Work with spelling locs Repository: rC Clang https://reviews.llvm.org/D50740 Files: include/clang/Basic/SourceManager.h lib/Basic/SourceManager.cpp unittes

[PATCH] D49462: [ObjC] Error out when using forward-declared protocol in a @protocol expression

arphaman updated this revision to Diff 161142. arphaman marked 3 inline comments as done. arphaman added a comment. address review comments. Repository: rC Clang https://reviews.llvm.org/D49462 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td li

[PATCH] D49462: [ObjC] Error out when using forward-declared protocol in a @protocol expression

arphaman added a comment. Sorry for the delay. In https://reviews.llvm.org/D49462#1166032, @rjmccall wrote: > Hmm. I think this is a reasonable change to make to the language. Have you > investigated to see if this causes source-compatibility problems? Yes, I tested this change on internal

[PATCH] D49462: [ObjC] Error out when using forward-declared protocol in a @protocol expression

arphaman added inline comments. Comment at: test/CodeGenObjC/forward-declare-protocol-gnu.m:6 -Protocol *getProtocol(void) -{ - return @protocol(P); -} +@interface I +@end rjmccall wrote: > arphaman wrote: > > rjmccall wrote: > > > Does this real

[PATCH] D49462: [ObjC] Error out when using forward-declared protocol in a @protocol expression

arphaman added inline comments. Comment at: lib/CodeGen/CGObjCMac.cpp:6788 + "emitting protocol metadata without definition"); + PD = PD->getDefinition(); rjmccall wrote: > What happens in the `@implementation` case (the one that we're not diagnosing

[PATCH] D50740: [SourceManager] isPointWithin: avoid using isBeforeInTranslationUnit, compare buffer offsets directly for lexical correctness

arphaman added a comment. Hmm, after more analysis I realized that this is not the right solution for the rename problem. It would be correct to map one file:line:column location to a set of `SourceLocation`s, and to use the old `isPointWithin` on a set of such locations. I opened https://revie

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

arphaman created this revision. arphaman added reviewers: jkorous, ioeric. Herald added a subscriber: dexonsmith. This patch extracts the code that searches for a file id from `translateFile` to `findFileIDsForFile` to allow the users to map from one file entry to multiple FileIDs. Will be used

[PATCH] D50814: [clangd] transfer the fixits with the notes instead of adding them to the main diagnostic if request by the client

arphaman added a comment. Our client has a presentation layer with a Clang-based hierarchical model for diagnostics, so we need the original notes. This is a hard requirement in that sense. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50814 ___

[PATCH] D49462: [ObjC] Error out when using forward-declared protocol in a @protocol expression

This revision was automatically updated to reflect the committed changes. Closed by commit rL340102: [ObjC] Error out when using forward-declared protocol in a @protocol (authored by arphaman, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm

[PATCH] D50571: [clangd] add an extension field to LSP to transfer the diagnostic's category

arphaman added a comment. In https://reviews.llvm.org/D50571#1205650, @joaotavora wrote: > > LGTM. Let's watch out for possible breakages in any of the clients, though. > > I've checked VSCode and it seems to be fine with the added fields. > > This isn't in the spec and broke the LSP client eglo

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

arphaman added inline comments. Comment at: include/clang/Basic/SourceManager.h:1539 + /// \returns true if the callback returned true, false otherwise. + bool findFileIDsForFile(const FileEntry *SourceFile, + llvm::function_ref Callback) const; ---

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

arphaman updated this revision to Diff 161504. arphaman marked 2 inline comments as done. arphaman added a comment. Address review comments. Repository: rC Clang https://reviews.llvm.org/D50926 Files: include/clang/Basic/SourceManager.h lib/Basic/SourceManager.cpp unittests/Basic/Sourc

[PATCH] D50993: [clangd] Increase stack size of the new threads on macOS

arphaman added inline comments. Comment at: clangd/Threading.cpp:76 + + if (::pthread_attr_setstacksize(&Attr, 8 * 1024 * 1024) != 0) +return; please use clang::DesiredStackSize instead. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50993

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

arphaman updated this revision to Diff 161847. arphaman marked an inline comment as done. arphaman added a comment. Address review comments Repository: rC Clang https://reviews.llvm.org/D50926 Files: include/clang/Basic/SourceManager.h lib/Basic/SourceManager.cpp unittests/Basic/Source

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

arphaman added inline comments. Comment at: lib/Basic/SourceManager.cpp:1705 // If we haven't found what we want yet, try again, but this time stat() // each of the files in case the files have changed since we originally ioeric wrote: > Do we also need t

[PATCH] D50814: [clangd] transfer the fixits with the notes instead of adding them to the main diagnostic if request by the client

arphaman added a comment. In https://reviews.llvm.org/D50814#1207219, @ilya-biryukov wrote: > Just to make sure I fully understand the use-case: could you elaborate a > little more? Do you need to get exactly the same set of notes that clang > provides? Yep. We are replacing libclang in a cli

[PATCH] D51077: [clangd] send diagnostic categories only when 'categorySupport' capability was given by the client

arphaman created this revision. arphaman added a reviewer: ilya-biryukov. Herald added subscribers: kadircet, dexonsmith, jkorous, MaskRay, ioeric. After https://reviews.llvm.org/D50571 Clangd started sending categories with each diagnostic, but that broke the eglot client. This patch puts the c

[PATCH] D51077: [clangd] send diagnostic categories only when 'categorySupport' capability was given by the client

This revision was automatically updated to reflect the committed changes. Closed by commit rL340449: [clangd] send diagnostic categories only when 'categorySupport' (authored by arphaman, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/

[PATCH] D50571: [clangd] add an extension field to LSP to transfer the diagnostic's category

arphaman added a comment. In https://reviews.llvm.org/D50571#1208635, @joaotavora wrote: > In https://reviews.llvm.org/D50571#1206020, @arphaman wrote: > > > In https://reviews.llvm.org/D50571#1205650, @joaotavora wrote: > > > > > > LGTM. Let's watch out for possible breakages in any of the clien

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

arphaman updated this revision to Diff 162106. arphaman added a comment. Address review comments. https://reviews.llvm.org/D50926 Files: include/clang/Basic/SourceManager.h lib/Basic/SourceManager.cpp unittests/Basic/SourceManagerTest.cpp Index: unittests/Basic/SourceManagerTest.cpp

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

arphaman marked an inline comment as done. arphaman added inline comments. Comment at: lib/Basic/SourceManager.cpp:1705 // If we haven't found what we want yet, try again, but this time stat() // each of the files in case the files have changed since we originally ---

[PATCH] D47280: [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM. Repository: rC Clang https://reviews.llvm.org/D47280 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D47229: Make atomic non-member functions as nonnull

arphaman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:3497 +else if (Form == Copy || Form == Xchg) { + if (!IsC11 && !IsN) +// The value pointer is always dereferenced, a nullptr is undefined. Nit: might make more sen

[PATCH] D47341: [Sema] Disable creating new delayed typos while correcting existing.

arphaman added inline comments. Comment at: clang/test/SemaCXX/typo-correction-delayed.cpp:146 + // expected-error@+1 {{use of undeclared identifier 'variableX'}} int x = variableX.getX(); } Loosing typo correction for 'getX' is fine, however, I think we sti

[PATCH] D46918: [Coverage] Discard the last uncompleted deferred region in a decl

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D46918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D47225: Add nonnull; use it for atomics

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM (FYI, Please use a weekly frequency for Pings). Repository: rCXX libc++ https://reviews.llvm.org/D47225 ___ cfe-commits mailing list

[PATCH] D37856: [refactor] add support for refactoring options

arphaman updated this revision to Diff 117124. arphaman marked 10 inline comments as done. arphaman added a comment. Address review comments Repository: rL LLVM https://reviews.llvm.org/D37856 Files: include/clang/Tooling/Refactoring/RefactoringActionRule.h include/clang/Tooling/Refactor

[PATCH] D37856: [refactor] add support for refactoring options

arphaman added inline comments. Comment at: include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:78 + std::vector> + getRefactoringOptions() const final override { +return {Opt}; ioeric wrote: > Why return a vector instead of a single valu

[PATCH] D37976: [docs][refactor] add refactoring engine design documentation

This revision was automatically updated to reflect the committed changes. arphaman marked 2 inline comments as done. Closed by commit rL314509: [docs][refactor] Add refactoring engine design documentation (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D37976?vs=115638

[PATCH] D38402: [clang-refactor] Apply source replacements

arphaman created this revision. This patch actually brings clang-refactor to a usable state as it can now apply the refactoring changes to the source files. The `-selection` option is now also fully supported. Repository: rL LLVM https://reviews.llvm.org/D38402 Files: include/clang/Fronte

[PATCH] D37681: [refactor] Simplify the interface and remove some template magic

arphaman added a comment. I will commit this today. @klimek, let me know if there any issues in post-commit review. Repository: rL LLVM https://reviews.llvm.org/D37681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D37681: [refactor] Simplify the interface and remove some template magic

This revision was automatically updated to reflect the committed changes. Closed by commit rL314704: [refactor] Simplify the refactoring interface (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D37681?vs=115211&id=117403#toc Repository: rL LLVM https://reviews.llv

[PATCH] D38618: Do not add a colon chunk to the code completion of class inheritance access modifiers

arphaman added inline comments. Comment at: include/clang/Sema/Scope.h:129 +/// We are between inheritance colon and the real class/struct definition scope +ClassInheritanceScope = 0x40, }; Could you please rebase this patch? There's another scope

[PATCH] D38538: Avoid printing some redundant name qualifiers in completion

arphaman added inline comments. Comment at: test/CodeCompletion/call.cpp:22 // CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>) - // CHECK-CC1: f(N::Y y, <#int ZZ#>) + // CHECK-CC1: f(Y y, <#int ZZ#>) // CHECK-CC1-NEXT: f(int i, <#int j#>, int k) --

[PATCH] D38402: [clang-refactor] Apply source replacements

arphaman added inline comments. Comment at: test/Refactor/tool-apply-replacements.cpp:6 + +class RenameMe { // CHECK: class { +}; ioeric wrote: > Why is the result `class {`? Sorry, the test was broken. Fixed! Repository: rL LLVM https://reviews.llvm.org/D38

[PATCH] D38402: [clang-refactor] Apply source replacements

arphaman updated this revision to Diff 118042. arphaman marked 5 inline comments as done. arphaman added a comment. Address review comments Repository: rL LLVM https://reviews.llvm.org/D38402 Files: include/clang/Frontend/CommandLineSourceLoc.h test/Refactor/tool-apply-replacements.cpp

[PATCH] D37856: [refactor] add support for refactoring options

This revision was automatically updated to reflect the committed changes. arphaman marked 3 inline comments as done. Closed by commit rL315087: [refactor] add support for refactoring options (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D37856?vs=117124&id=118044#toc

[PATCH] D37856: [refactor] add support for refactoring options

arphaman added inline comments. Comment at: include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:73 +template +class OptionRequirement : public RefactoringOptionsRequirement { +public: ioeric wrote: > nit: `OptionRequirement` sounds more genera

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

arphaman added a comment. Thanks for working on this! Could you please post the patch with full context (`git diff -U99`)? Comment at: test/Index/USR/array-type.cpp:1 +// RUN: c-index-test core -print-source-symbols -- %s | grep "function(Gen,TS)/C++" | grep foo | cut -s -

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

arphaman added inline comments. Comment at: lib/Index/USRGeneration.cpp:820 +if (const ArrayType *const AT = dyn_cast(T)) { + VisitType(AT->getElementType()); + Out << "["; We should probably follow the other types and just set `T = AT->getElementT

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

arphaman added a comment. Thanks, it looks much better! A couple more comments: Comment at: lib/Index/USRGeneration.cpp:819 } +if (const ArrayType *const AT = dyn_cast(T)) { + Out << "{"; You can use `const auto *` here. Comment

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Index/USRGeneration.cpp:819 } +if (const auto *const AT = dyn_cast(T)) { + Out << "{"; Nit: I don't think you really n

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

arphaman added inline comments. Comment at: lib/Index/USRGeneration.cpp:820 +if (const auto *const AT = dyn_cast(T)) { + Out << "{"; + switch (AT->getSizeModifier()) { You might also want to use the character literals for one char strings for effic

[PATCH] D38708: [AST] Flag the typo-corrected nodes for better tooling

arphaman created this revision. This patch adds a new boolean field to the `DeclRefExpr`, `MemberExpr`, `CXXCtorInitializer`, `ObjCIvarRefExpr`, `ObjCPropertyRefExpr` nodes which is set to true when these nodes have been produced during typo-correction. This is useful for Clang-based tooling as

[PATCH] D38707: PR13575: Fix USR mangling for functions taking function pointers as arguments.

arphaman added inline comments. Comment at: lib/Index/USRGeneration.cpp:757 VisitType(FT->getReturnType()); - for (const auto &I : FT->param_types()) + Out << '('; + for (const auto &I : FT->param_types()) { I believe you can drop the '(' an

[PATCH] D38707: PR13575: Fix USR mangling for functions taking function pointers as arguments.

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D38707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D38567: [config] Warn when POSIX_C_SOURCE breaks threading support on Darwin

arphaman added inline comments. Comment at: include/__threading_support:26 +#if (defined(__MAC_OS_X_VERSION_MIN_REQUIRED) && (__MAC_OS_X_VERSION_MIN_REQUIRED < 101300)) \ +|| (defined(__IPHONE_OS_VERSION_MIN_REQUIRED) && (__IPHONE_OS_VERSION_MIN_REQUIRED < 11)) \

[PATCH] D38538: Avoid printing some redundant name qualifiers in completion

arphaman added inline comments. Comment at: test/CodeCompletion/qualifiers-as-written.cpp:29 + // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar c#>, <#ns::baz d#> + // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz c#> +} -

[PATCH] D38402: [clang-refactor] Apply source replacements

arphaman updated this revision to Diff 118471. arphaman marked 6 inline comments as done. arphaman added a comment. Address review comments Repository: rL LLVM https://reviews.llvm.org/D38402 Files: include/clang/Frontend/CommandLineSourceLoc.h test/Refactor/tool-apply-replacements.cpp

[PATCH] D38755: Fixed crash during indexing default template template param

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. Thanks, LGTM. https://reviews.llvm.org/D38755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D38772: [refactor] allow the use of refactoring diagnostics

arphaman created this revision. Herald added a subscriber: mgorny. This patch allows the refactoring library to use its own set of refactoring-specific diagnostics to reports things like initiation errors. Repository: rL LLVM https://reviews.llvm.org/D38772 Files: include/clang/Basic/AllD

[PATCH] D37856: [refactor] add support for refactoring options

arphaman added a comment. In https://reviews.llvm.org/D37856#894638, @hokein wrote: > Sorry for the delay. I saw you have reverted this commit somehow. A post > commit. I had some issues with ppc64/s390x bots for some reason, so I had to revert. I'm still trying to investigate what went wrong

[PATCH] D38772: [refactor] allow the use of refactoring diagnostics

arphaman added inline comments. Comment at: lib/Basic/DiagnosticIDs.cpp:46 unsigned WarnShowInSystemHeader : 1; - unsigned Category : 5; + unsigned Category : 6; hokein wrote: > just curious: is this change needed? I get a build warning without this change

[PATCH] D38772: [refactor] allow the use of refactoring diagnostics

arphaman updated this revision to Diff 118701. arphaman marked 2 inline comments as done. arphaman added a comment. - rename the common consumer class. Repository: rL LLVM https://reviews.llvm.org/D38772 Files: include/clang/Basic/AllDiagnostics.h include/clang/Basic/CMakeLists.txt inc

[PATCH] D38835: [refactor] selection: new CodeRangeASTSelection represents a set of selected consecu

arphaman created this revision. Repository: rL LLVM https://reviews.llvm.org/D38835 Files: include/clang/Tooling/Refactoring/ASTSelection.h lib/Tooling/Refactoring/ASTSelection.cpp unittests/Tooling/ASTSelectionTest.cpp Index: unittests/Tooling/ASTSelectionTest.cpp =

[PATCH] D38863: Typos in tutorial

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. You generally don't need a pre-commit review for such a change. https://reviews.llvm.org/D38863 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D38402: [clang-refactor] Apply source replacements

This revision was automatically updated to reflect the committed changes. Closed by commit rL315738: [clang-refactor] Apply source replacements (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D38402?vs=118471&id=118959#toc Repository: rL LLVM https://reviews.llvm.o

[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

arphaman added inline comments. Comment at: include/clang/Tooling/CommonOptionsParser.h:116 + bool HasError; + std::string ErrorMessage; std::unique_ptr Compilations; Would it be better to have an `llvm::Error' instead of the two fields? C

[PATCH] D38772: [refactor] allow the use of refactoring diagnostics

This revision was automatically updated to reflect the committed changes. Closed by commit rL315924: [refactor] allow the use of refactoring diagnostics (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D38772?vs=118701&id=119185#toc Repository: rL LLVM https://revie

[PATCH] D38835: [refactor] selection: new CodeRangeASTSelection represents a set of selected consecutive statements

arphaman added inline comments. Comment at: include/clang/Tooling/Refactoring/ASTSelection.h:74 +/// An AST selection value that corresponds to a selection of a set of +/// statements that belong to one body of code (like one function). +/// hokein wrote: > I see

[PATCH] D38835: [refactor] selection: new CodeRangeASTSelection represents a set of selected consecutive statements

arphaman updated this revision to Diff 119197. arphaman marked 6 inline comments as done. arphaman added a comment. Address review comments Repository: rL LLVM https://reviews.llvm.org/D38835 Files: include/clang/Tooling/Refactoring/ASTSelection.h lib/Tooling/Refactoring/ASTSelection.cpp

[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

arphaman created this revision. Herald added a subscriber: mgorny. This patch adds an initial, skeleton outline of the "extract function" refactoring. The extracted function doesn't capture variables / rewrite code yet, it just basically does a simple copy-paste. The following initiation rules a

[PATCH] D38985: [refactor] Add support for editor commands that connect IDEs/editors to the refactoring actions

arphaman created this revision. Herald added subscribers: ilya-biryukov, mgorny. This patch adds support for editor commands that allow refactoring to be used in editor clients like libclang or clangd. An editor command can be bound to an refactoring action rule. Once it is bound, it's available

[PATCH] D39027: [docs][refactor] Add a new tutorial that talks about how one can implement refactoring actions

arphaman created this revision. This patch adds a new tutorial to Clang's talks that walks through an example of a refactoring action and how it can be implemented in Clang. Repository: rL LLVM https://reviews.llvm.org/D39027 Files: docs/RefactoringActionTutorial.rst docs/RefactoringEng

[PATCH] D39050: Add index-while-building support to Clang

arphaman added a comment. I think this patch should be split into a number of smaller patches to help the review process. Things like `tools/IndexStore`, `DirectoryWatcher` and other components that are not directly needed right now should definitely be in their own patches. It would be nice to

[PATCH] D39055: [refactor] Add an editor client that is used in clangd

arphaman created this revision. Herald added subscribers: mgorny, klimek. This patch adds an editor client that can do things like: - find a list of available refactoring editor commands in a particular range. - perform a particular refactoring editor command in a particular range. This editor c

[PATCH] D39057: [clangd][WIP] Integrate the refactoring actions into clangd

arphaman created this revision. arphaman added a project: clang-tools-extra. Herald added a subscriber: mgorny. This WIP patch provides a sample implementation of an integration of Clang's refactoring actions from libToolingRefactor into clangd. In terms of protocol support, the patch adds: - S

[PATCH] D39057: [clangd][WIP] Integrate the refactoring actions into clangd

arphaman added a comment. I think that the clangd editor plugin will have to be modified as well, but I haven't looked into that yet. Repository: rL LLVM https://reviews.llvm.org/D39057 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D38985: [refactor] Add support for editor commands that connect IDEs/editors to the refactoring actions

arphaman added inline comments. Comment at: include/clang/Tooling/Refactoring/RefactoringActionRule.h:58 + /// Returns the editor command that's was bound to this rule. + virtual const EditorCommand *getEditorCommand() { return nullptr; } }; ioeric wrote: > I'

[PATCH] D38835: [refactor] selection: new CodeRangeASTSelection represents a set of selected consecutive statements

arphaman added inline comments. Comment at: unittests/Tooling/ASTSelectionTest.cpp:688 + Source, {2, 2}, None, + [](SourceRange SelectionRange, Optional Node) { +EXPECT_TRUE(Node); hokein wrote: > arphaman wrote: > > hokein wrote: > > > I'm a bi

[PATCH] D38835: [refactor] selection: new CodeRangeASTSelection represents a set of selected consecutive statements

This revision was automatically updated to reflect the committed changes. Closed by commit rL316104: [refactor] selection: new CodeRangeASTSelection represents a set of selected (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D38835?vs=119197&id=119514#toc Repository:

[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

arphaman added inline comments. Comment at: include/clang/Tooling/StandaloneExecution.h:1 +//===--- Execution.h - Standalone clang action execution. -*- C++ ---*-===// +// StandaloneExecution.h? https://reviews.llvm.org/D34272 __

[PATCH] D39092: [clang-refactor] Add "-Inplace" option to the commandline tool.

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D39092 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

arphaman updated this revision to Diff 119708. arphaman marked 4 inline comments as done. arphaman added a comment. Address review comments. Repository: rL LLVM https://reviews.llvm.org/D38982 Files: include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h include/clang/Tool

[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

arphaman added a comment. In https://reviews.llvm.org/D38982#900744, @ioeric wrote: > Code looks good in general. I see there are a bunch of major features > missing; it might make sense to print a warning or document the major missing > features in the high-level API. I added a warning in th

[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

arphaman updated this revision to Diff 119711. arphaman added a comment. Fix diff Repository: rL LLVM https://reviews.llvm.org/D38982 Files: include/clang/Basic/DiagnosticRefactoringKinds.td include/clang/Tooling/Refactoring/ASTSelection.h include/clang/Tooling/Refactoring/RefactoringA

[PATCH] D38618: Do not add a colon chunk to the code completion of class inheritance access modifiers

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. Overall LGTM, just a couple of comments: Comment at: include/clang/Sema/Scope.h:131 + +/// We are between inheritance colon and the real class/struct definition scop

[PATCH] D38538: Avoid printing some redundant name qualifiers in completion

arphaman accepted this revision. arphaman added inline comments. This revision is now accepted and ready to land. Comment at: test/CodeCompletion/qualifiers-as-written.cpp:29 + // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar c#>, <#ns::baz d#> + // C

[PATCH] D36790: [ObjC] Messages to 'self' in class methods should use class method dispatch to avoid multiple method ambiguities

arphaman added a comment. @doug.gregor Ping Repository: rL LLVM https://reviews.llvm.org/D36790 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments

arphaman marked an inline comment as done. arphaman added a comment. In https://reviews.llvm.org/D37341#869042, @vsapsai wrote: > Does your fix work for deeper nesting too (e.g. template in template in > template)? Looks like it should, just want to confirm. Yes. > Are there other places wher

[PATCH] D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments

arphaman updated this revision to Diff 119967. arphaman added a comment. Use just the `isInvalidDecl` check. Repository: rL LLVM https://reviews.llvm.org/D37341 Files: lib/Sema/SemaDecl.cpp test/SemaTemplate/deduction-crash.cpp test/SemaTemplate/explicit-specialization-member.cpp Ind

[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

This revision was automatically updated to reflect the committed changes. arphaman marked an inline comment as done. Closed by commit rL316465: [refactor] Initial outline of implementation of "extract function" refactoring (authored by arphaman). Changed prior to commit: https://reviews.llvm.or

[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

arphaman added inline comments. Comment at: include/clang/Basic/DiagnosticRefactoringKinds.td:24 + "not overlap with the AST nodes of interest">; + +def err_refactor_code_outside_of_function : Error<"the selected code is not a " ioeric wrote: > nit: was this new

  1   2   3   4   5   6   7   8   9   10   >