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

arphaman added a comment. You In https://reviews.llvm.org/D38985#901152, @sammccall wrote: > Hi Alex! I'm working on clangd, but am pretty new to the project, so forgive > some naive questions. > > I'm a bit unclear at a high level what the new abstractions in this patch > add, in terms of wir

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

arphaman updated this revision to Diff 120128. arphaman added a comment. Avoid using a separate EditorCommand class. Repository: rL LLVM https://reviews.llvm.org/D38985 Files: include/clang/Tooling/Refactoring/RefactoringActionRule.h include/clang/Tooling/Refactoring/RefactoringActionRul

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

arphaman updated this revision to Diff 120129. arphaman marked an inline comment as done. arphaman added a comment. Fix comment nit. Repository: rL LLVM https://reviews.llvm.org/D38985 Files: include/clang/Tooling/Refactoring/RefactoringActionRule.h include/clang/Tooling/Refactoring/Refa

[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: > ar

[PATCH] D39266: [clang-refactor] Use add_clang_tool CMake template

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. Thanks! Repository: rL LLVM https://reviews.llvm.org/D39266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D36790: [ObjC] Messages to 'self' in class methods that return 'instancetype' should use the pointer to the class as the result type of the message

arphaman updated this revision to Diff 120157. arphaman retitled this revision from "[ObjC] Messages to 'self' in class methods should use class method dispatch to avoid multiple method ambiguities" to "[ObjC] Messages to 'self' in class methods that return 'instancetype' should use the pointer

[PATCH] D36790: [ObjC] Messages to 'self' in class methods that return 'instancetype' should use the pointer to the class as the result type of the message

arphaman added a comment. In https://reviews.llvm.org/D36790#843450, @arphaman wrote: > @rjmccall Do you think that the rules for the return types in overridden > methods that return `instancetype` should be strengthened first? For example, > if we have the following code: > > @interface Unre

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

arphaman added a comment. Hmm, I don't think this solution is ideal, we'd rather have an attribute somewhere for other consumers of availability annotations. Does MyObject have an implicit decl of `new`, or are we referring to `NSObject`s `new`? Ideally we would an attribute on a particular `ne

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

arphaman added a subscriber: ributzka. arphaman added a comment. In https://reviews.llvm.org/D51189#1211763, @erik.pilkington wrote: > In https://reviews.llvm.org/D51189#1211754, @arphaman wrote: > > > Hmm, I don't think this solution is ideal, we'd rather have an attribute > > somewhere for oth

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

arphaman added a comment. That's probably the best solution then, I don't think declaring implicit `new` just for availability attribute is sound. Does this work with `self new` as well? Comment at: clang/lib/AST/DeclObjC.cpp:833 +bool ObjCMethodDecl::definedInNSObject(const

[PATCH] D51507: Allow all supportable attributes to be used with #pragma clang attribute.

arphaman accepted this revision. arphaman added a comment. LGTM, Thank you! Repository: rC Clang https://reviews.llvm.org/D51507 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51649: [Sema] Don't warn about omitting unavailable enum constants in a switch

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/D51649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

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

arphaman updated this revision to Diff 164734. arphaman marked 3 inline comments as done. arphaman added a comment. Remove dead code for filesystem update fileID matching. Repository: rC Clang https://reviews.llvm.org/D50926 Files: include/clang/Basic/SourceManager.h lib/Basic/SourceMana

[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:1626 +if (FileContentCache->OrigEntry == SourceFile) { + if (Callback(FileID::get(I))) +return true; ioeric wrote: > Should we check `FileID::get(I)` is valid? That's not rea

[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#1224292, @ilya-biryukov wrote: > Sorry for the delayed response. It seems we absolutely need this if mirroring > libclang is an absolute requirement. > We seem to have multiple implementation options: > > Which classes to use for rep

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

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

[PATCH] D49523: [clangd] Add support for per-file override compilation command

arphaman added a comment. In https://reviews.llvm.org/D49523#1167553, @malaperle wrote: > Interesting! We also have a need for passing compilation commands in a > context where there is no compile_commands.json, but we were thinking of > putting this in a "didChangeConfiguration" message so tha

[PATCH] D49523: [clangd] Add support for per-file override compilation command

arphaman added a comment. In https://reviews.llvm.org/D49523#1169743, @jkorous wrote: > BTW it looks like we already had kind of support for compilation command > before (extra flags). > > commit 5ec1f7ca32eb85077a22ce81d41aa02a017d4852 > Author: Krasimir Georgiev > Date: Thu Jul 6 08:44:54

[PATCH] D49523: [clangd] Add support for per-file override compilation command

arphaman added a comment. In https://reviews.llvm.org/D49523#1169728, @malaperle wrote: > In https://reviews.llvm.org/D49523#1169000, @arphaman wrote: > > > In https://reviews.llvm.org/D49523#1167553, @malaperle wrote: > > > > > Interesting! We also have a need for passing compilation commands in

[PATCH] D49523: [clangd] Add support for per-file override compilation command

arphaman added a comment. In https://reviews.llvm.org/D49523#1169621, @jkorous wrote: > Alex, I am just wondering if we shouldn't rather create another > implementation of GlobalCompilationDatabase interface (something like > InMemoryGlobalCompilationDatabase), add it to ClangdServer and use it

[PATCH] D48559: [clangd] refactoring for XPC transport layer [NFCI]

arphaman added a comment. In https://reviews.llvm.org/D48559#1169975, @sammccall wrote: > Sorry for the delay here, and I'm sorry I haven't been into the patches in > detail again yet - crazy week. After experiments, I am convinced the > Transport abstraction patch can turn into something nice

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

arphaman created this revision. arphaman added reviewers: malaperle-ericsson, ilya-biryukov, sammccall, jkorous. arphaman added a project: clang-tools-extra. Herald added subscribers: dexonsmith, MaskRay, ioeric. This patch is based on https://reviews.llvm.org/D49523. It extends the 'workspace/di

[PATCH] D49523: [clangd] Add support for per-file override compilation command

arphaman added a comment. In https://reviews.llvm.org/D49523#1171484, @ilya-biryukov wrote: > The extensions itself seems like a reasonable way to provide compile commands > for the individual files. In case didChangeConfiguration does not work for > you for some reason, happy to take a look at

[PATCH] D49736: [Basic] Emit warning flag suggestion only in case there's existing flag *similar* to the unknown one

arphaman added a comment. Thanks for working on this! It looks like we only have one client of `getNearestOption`. Wouldn't it make sense to fold the check into the function itself? Repository: rC Clang https://reviews.llvm.org/D49736 ___ cfe-co

[PATCH] D49736: [Basic] Emit warning flag suggestion only in case there's existing flag *similar* to the unknown one

arphaman added a comment. Sounds good Repository: rC Clang https://reviews.llvm.org/D49736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49523: [clangd] Add support for per-file override compilation command

arphaman added a comment. In https://reviews.llvm.org/D49523#1174627, @ilya-biryukov wrote: > In https://reviews.llvm.org/D49523#1174086, @arphaman wrote: > > > We actually need both mechanisms. I posted the didChangeConfiguration > > extension to https://reviews.llvm.org/D49758. > > > Why do we

[PATCH] D49523: [clangd] Add support for per-file override compilation command

arphaman abandoned this revision. arphaman added a comment. Abandoned in favor of https://reviews.llvm.org/D49758 (will update it today). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49523 ___ cfe-commits mailing list cfe-commits@

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

arphaman added a comment. In https://reviews.llvm.org/D49758#1174629, @ilya-biryukov wrote: > The mode of operation where compile commands come from the client seems > useful, but I wonder if there's any value in mixing it with > `compile_commands.json` and other CDB plugins. > Do you plan to

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

arphaman added inline comments. Comment at: clangd/Protocol.h:429 + // The changes that happened to the compilation database. + llvm::Optional>> + compilationDatabaseChanges; ilya-biryukov wrote: > - Maybe add a comment that the key of the map is a file na

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

arphaman added a comment. In https://reviews.llvm.org/D49758#1178056, @ilya-biryukov wrote: > In https://reviews.llvm.org/D49758#1177747, @arphaman wrote: > > > In https://reviews.llvm.org/D49758#1174629, @ilya-biryukov wrote: > > > > > The mode of operation where compile commands come from the c

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

arphaman updated this revision to Diff 157762. arphaman marked 2 inline comments as done. arphaman added a comment. Updated patch to address review comments: - The compilation database updated are used only when '-in-memory-compile-commands' flag is used. - It's now possible to set the working d

[PATCH] D49548: [clangd] XPC WIP

arphaman requested changes to this revision. arphaman added a comment. FYI, this patch can't be applied because of the broken file paths (some diffs include /clangd prefix, some don't, while the test diffs are missing '/test'). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D495

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

arphaman updated this revision to Diff 158024. arphaman marked 6 inline comments as done. arphaman added a comment. Address review comments https://reviews.llvm.org/D49758 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompil

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

arphaman added inline comments. Comment at: clangd/tool/ClangdMain.cpp:170 +static llvm::cl::opt +InMemoryCompileCommands("in-memory-compile-commands", +llvm::cl::desc("Use an in-memory compile commands"), ilya-biryukov wrote: > T

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

arphaman updated this revision to Diff 158376. arphaman marked 3 inline comments as done. arphaman added a comment. Updated to address review comments. https://reviews.llvm.org/D49758 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/GlobalCompilationDatabase.cpp clangd/

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

arphaman added inline comments. Comment at: test/clangd/did-change-configuration-params.test:43 +# +# ERR: Updating file /clangd-test/foo.c with command [/clangd-test2] clang -c foo.c -Wall -Werror +# Don't reparse the second file: ilya-biryukov wrote: > Matchin

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

This revision was automatically updated to reflect the committed changes. Closed by commit rL338597: [clangd] allow clients to control the compilation database by passing in (authored by arphaman, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

arphaman created this revision. arphaman added reviewers: ilya-biryukov, ioeric, hokein, jkorous. Herald added subscribers: dexonsmith, MaskRay. This patch adds support for diagnostic message capitalization to Clangd. This is enabled when '-capitialize-diagnostic-text' is provided to Clangd. R

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

arphaman updated this revision to Diff 159036. arphaman added a comment. Always capitalize the diagnostic's message. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50154 Files: clangd/Diagnostics.cpp test/clangd/capitalize-diagnostics.test test/clangd/diagnostics.test t

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

arphaman updated this revision to Diff 159059. arphaman marked an inline comment as done. arphaman added a comment. Remove test and update unit test. https://reviews.llvm.org/D50154 Files: clangd/Diagnostics.cpp test/clangd/diagnostics.test test/clangd/did-change-configuration-params.test

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

arphaman added inline comments. Comment at: test/clangd/capitalize-diagnostics.test:1 +# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} ---

[PATCH] D50255: Add test for changing build configuration

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. Thanks! This LGTM. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50255 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D50154: [clangd] capitalize diagnostic messages

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE338919: [clangd] capitalize diagnostic messages (authored by arphaman, committed by ). Changed prior to commit: https://reviews.llvm.org/D50154?vs=159059&id=159080#toc Repository: rCTE Clang Tools

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

arphaman created this revision. arphaman added reviewers: ahatanak, vsapsai. Herald added subscribers: jkorous-apple, kristof.beyls, aemerson. The -Wformat recently started warning for the following code because of the added support for analysis for the '%zi' specifier. NSInteger i = NSInteger

[PATCH] D42561: [PR36008] Avoid -Wsign-compare warning for enum constants in typeof expressions

arphaman updated this revision to Diff 132901. arphaman marked 4 inline comments as done. arphaman added a comment. Herald added a subscriber: jkorous-apple. Address review comments and fix the inverted check case. https://reviews.llvm.org/D42561 Files: lib/Sema/SemaChecking.cpp test/Sema/c

[PATCH] D42561: [PR36008] Avoid -Wsign-compare warning for enum constants in typeof expressions

This revision was automatically updated to reflect the committed changes. Closed by commit rL324514: [PR36008] Avoid -Wsign-compare warning for enum constants in (authored by arphaman, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

arphaman abandoned this revision. arphaman added subscribers: dcoughlin, dexonsmith. arphaman added a comment. Thanks for your input. You're right, this warning is quite correct (even though it seems like too much). I discussed this with @dexonsmith and @dcoughlin and we decided to keep the wa

[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM, I didn't find any issues https://reviews.llvm.org/D36918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D33493: Speed up preamble loading, reduce global completion cache calls

arphaman added a comment. Please post the patch with full context `git diff -U`. https://reviews.llvm.org/D33493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33450: Warn about uses of `@available` that can't suppress the -Wunguarded-availability warnings

This revision was automatically updated to reflect the committed changes. Closed by commit rL303761: Warn about uses of `@available` that can't suppress the (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D33450?vs=99948&id=100094#toc Repository: rL LLVM https://re

[PATCH] D33661: [Sema][ObjC] Don't emit availability diagnostics for categories extending unavailable interfaces

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

[PATCH] D33357: Avoid calling report_fatal_error in the destructor of raw_fd_ostream when saving a module timestamp file

This revision was automatically updated to reflect the committed changes. Closed by commit rL304538: Avoid calling report_fatal_error in the destructor of raw_fd_ostream (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D33357?vs=99904&id=101182#toc Repository: rL LLV

[PATCH] D32178: Delete unstable integration tests

arphaman added a comment. Sorry for the delay, was on vacation. Committing today. https://reviews.llvm.org/D32178 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32178: Delete unstable integration tests

arphaman added a comment. Committed r304541 & r304542. https://reviews.llvm.org/D32178 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

arphaman added inline comments. Comment at: tools/libclang/CIndex.cpp:7287 + if (!HadAvailAttr) if (const EnumConstantDecl *EnumConst = dyn_cast(D)) I think that you can move this `if` before the new `if`, and convert the new `if` to be an `if (Availabil

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

arphaman added inline comments. Comment at: tools/libclang/CIndex.cpp:7322 + + for (int I = 0, E = AvailabilityAttrs.size(); I < E && I < availability_size; + ++I) { rdwampler wrote: > arphaman wrote: > > You can use a ranged for loop here if you use `take

[PATCH] D33913: [index] Index static_assert declarations

arphaman created this revision. static_assert declarations have to be indexed to gather the declaration references in their assert expression. Repository: rL LLVM https://reviews.llvm.org/D33913 Files: lib/Index/IndexDecl.cpp test/Index/Core/index-source.cpp Index: test/Index/Core/ind

[PATCH] D33920: [index] Record C++17 binding declarations

arphaman created this revision. Global binding declarations should be recorded as `variable` symbols. Repository: rL LLVM https://reviews.llvm.org/D33920 Files: lib/Index/IndexDecl.cpp lib/Index/IndexSymbol.cpp test/Index/Core/index-source.cpp Index: test/Index/Core/index-source.cpp

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

arphaman added a comment. It seems that there's a slight bug in the patch: If I print the output of the following code using `c-index-test -test-load-source all`: void bar2(void) __attribute__((availability(macosx, introduced=10.4))) __attribute__((availability(macosx, deprecated=10.5, me

[PATCH] D26065: Improve diagnostics if friend function redefines file-level function.

arphaman added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:646 + // in this case, redefinition will be diagnosed later. + (New->isInlineSpecified() || !New->isOutOfLine() || + !New->getLexicalDeclContext()->isRecord())) { ---

[PATCH] D33493: Speed up preamble loading

arphaman added a comment. Can you use a local map in `TranslateStoredDiagnostics` instead of storing one in the `ASTUnit`, or do you need to keep it around? https://reviews.llvm.org/D33493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D33493: Speed up preamble loading

arphaman added inline comments. Comment at: include/clang/Frontend/ASTUnit.h:192 + /// of that loading + std::map SrcLocCache; + You can use an `llvm::StringMap` instead. Comment at: lib/Frontend/ASTUnit.cpp:1152 + else +SrcLocCache.clea

[PATCH] D26065: Improve diagnostics if friend function redefines file-level function.

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

[PATCH] D33493: Speed up preamble loading

arphaman added inline comments. Comment at: lib/Frontend/ASTUnit.cpp:1152 + else +SrcLocCache.clear(); yvvan wrote: > arphaman wrote: > > Why is `clear` in an `else` here? We always create a new `SourceManager` in > > this function, so the previously cach

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

arphaman added a comment. This looks better, it's almost ready. A couple of small requests: Comment at: tools/libclang/CIndex.cpp:7262 +LHS->getMessage() == RHS->getMessage() && +LHS->getReplacement() == RHS->getReplacement()) + return true; ---

[PATCH] D33493: Speed up preamble loading

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM with one inline request below. I also have a question: what kind of performance benefits do you get for the preamble load times? Comment at: lib/Frontend/ASTUnit.c

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

arphaman added inline comments. Comment at: test/Index/availability.c:20 // CHECK-2: (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7) // CHECK-2: EnumConstantDecl=old_enum:6:3 (Definition) (deprecated) rdwampler wrote: > Can we run `FileCheck` once n

[PATCH] D33976: [clang] Fix format specifiers fixits

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. Thanks for working on this! LGTM with a couple of fixes: Comment at: test/FixIt/fixit-format-darwin.m:3 +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -f

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

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

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

arphaman added a comment. Do you have commit access or shall I commit it on your behalf? https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

This revision was automatically updated to reflect the committed changes. Closed by commit rL305117: [libclang] Merge multiple availability clauses when getting the platform's (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D33478?vs=102050&id=102074#toc Repository:

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

arphaman added a comment. Ronald, I had to revert the commit as the `availability.c` test was failing on Linux (e.g. http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/12532). Can you please look into that? I think you might have to got back to the `CHECK-1` an

[PATCH] D34066: [clang] Cleanup fixit.c

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

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

arphaman added a comment. Thanks, I'll recommit today. https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

arphaman added a comment. Recommitted in r305221. https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32046: [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, correct getNumArguments

arphaman added a comment. Can you somehow test this change? Maybe a unittest for `getStringifiedArgument`that ensures that the assertion is triggered after the number of macro arguments is reached? https://reviews.llvm.org/D32046 ___ cfe-commits m

[PATCH] D34173: [Completion] Code complete the members for a dependent type after a '::'

arphaman created this revision. This is a follow up to r302797 which added support for dependent completions after '.' and '->'. This patch handles the '::' operator. Repository: rL LLVM https://reviews.llvm.org/D34173 Files: lib/Sema/SemaCodeComplete.cpp test/CodeCompletion/member-acce

[PATCH] D34175: [driver][macOS] Pick the system version for the deployment target if the SDK is newer than the system

arphaman created this revision. This patch improves the driver by making sure that it picks the system version for the deployment target when the version of the macOS SDK is newer than the system version. Repository: rL LLVM https://reviews.llvm.org/D34175 Files: lib/Driver/ToolChains/Da

[PATCH] D34185: [Parser][ObjC] Avoid crashing when skipping to EOF while parsing an ObjC interface/implementation

arphaman created this revision. This patch fixes a Clang crash that happens when an Objective-C source code contains an `@interface`/`@implementation` declaration that follows unterminated `@implementation` declaration that contains a method with a message send that doesn't have the ']'. The cr

[PATCH] D34185: [Parser][ObjC] Avoid crashing when skipping to EOF while parsing an ObjC interface/implementation

arphaman added a comment. In https://reviews.llvm.org/D34185#780494, @ahatanak wrote: > This patch fixes the crash and that is fine, but the users might find the > last error ("expected '}'" at the end of the file) confusing. This seems to > happen because Parser::ParseLexedObjCMethodDefs consu

[PATCH] D34175: [driver][macOS] Pick the system version for the deployment target if the SDK is newer than the system

arphaman added inline comments. Comment at: lib/Driver/ToolChains/Darwin.cpp:1133 + HadExtra) || + HadExtra || Major != 10 || Minor >= 100 || Micro >= 100) +return MacOSSDKVersion; inouehrs wrote: > What these magic number

[PATCH] D32046: [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, correct getNumArguments

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM! Please fix the code style issues that I pointed out before committing. Comment at: unittests/Lex/LexerTest.cpp:24 #include "clang/Lex/PreprocessorOptions.h" +#incl

[PATCH] D34175: [driver][macOS] Pick the system version for the deployment target if the SDK is newer than the system

arphaman updated this revision to Diff 102607. arphaman added a comment. Remove redundant driver version correctness checks. Repository: rL LLVM https://reviews.llvm.org/D34175 Files: lib/Driver/ToolChains/Darwin.cpp test/Driver/darwin-sdkroot.c Index: test/Driver/darwin-sdkroot.c

[PATCH] D34185: [Parser][ObjC] Avoid crashing when skipping to EOF while parsing an ObjC interface/implementation

arphaman updated this revision to Diff 102718. arphaman added a comment. Use the 'Eof' token to make sure that the "expected '}'" error is presented not at the end of the file, but at the start of the `@interface`/`@implementation`. Repository: rL LLVM https://reviews.llvm.org/D34185 Files:

[PATCH] D33913: [index] Index static_assert declarations

This revision was automatically updated to reflect the committed changes. Closed by commit rL305504: [index] Index static_assert declarations (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D33913?vs=101458&id=102720#toc Repository: rL LLVM https://reviews.llvm.org

[PATCH] D33920: [index] Record C++17 binding declarations

This revision was automatically updated to reflect the committed changes. Closed by commit rL305508: [index] Record C++17 global binding declarations (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D33920?vs=101478&id=102722#toc Repository: rL LLVM https://reviews.

[PATCH] D34173: [Completion] Code complete the members for a dependent type after a '::'

This revision was automatically updated to reflect the committed changes. Closed by commit rL305511: [Completion] Code complete the members for a dependent type after a '::' (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34173?vs=102410&id=102724#toc Repository: r

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

arphaman created this revision. r300667 added support for editor placeholder to Clang. That commit didn’t take into account that users who use Clang for preprocessing only (-E) will get the “editor placeholder in source file” error when preprocessing their source (PR33394). This commit ensures

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

arphaman updated this revision to Diff 102764. arphaman added a comment. Fair enough. I removed the special checks for `<#>` and `<##>`. Repository: rL LLVM https://reviews.llvm.org/D34256 Files: include/clang/Lex/PreprocessorOptions.h lib/Frontend/CompilerInvocation.cpp lib/Lex/Lexer.

[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets

arphaman created this revision. This patch adds a new warning flag called `-Wunguarded-availability-new`. If `-Wunguarded-availability` is off, this warning only warns about uses of APIs that have been introduced in macOS >= 10.13, iOS >= 11, watchOS >= 4 and tvOS >= 11. This warning is on by d

[PATCH] D33253: [index] Fix typo: inferface -> interface

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

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

This revision was automatically updated to reflect the committed changes. Closed by commit rL305576: [PR33394] Avoid lexing editor placeholders when Clang is used only (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34256?vs=102764&id=102862#toc Repository: rL LLVM

[PATCH] D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations

arphaman added a comment. I find the change to the diagnostic for enum constants a bit off putting, since the warning can refer to the enum itself when an enum constant is used. I'd rather we say something like `'EnumConstant' is deprecated` and keep the note that this patch uses (`'Enum' has b

[PATCH] D33253: [index] Fix typo: inferface -> interface

arphaman added a comment. Btw, you can usually commit typo fixes and other similar changes without pre-commit reviews Repository: rL LLVM https://reviews.llvm.org/D33253 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets

arphaman updated this revision to Diff 102891. arphaman marked 2 inline comments as done. arphaman added a comment. Remove the assert and support the other partial availability warnings. Repository: rL LLVM https://reviews.llvm.org/D34264 Files: include/clang/Basic/DiagnosticGroups.td in

[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets

arphaman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:6944 -diag = !ObjCPropertyAccess ? diag::err_unavailable - : diag::err_property_method_unavailable; -diag_message = diag::err_unavailable_message; erik.pil

[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets

arphaman updated this revision to Diff 102902. arphaman marked an inline comment as done. arphaman added a comment. Swap the checking operators and bring back the accidentally deleted code. Repository: rL LLVM https://reviews.llvm.org/D34264 Files: include/clang/Basic/DiagnosticGroups.td

[PATCH] D34287: Moved code hanlding precompiled preamble out of the ASTUnit.

arphaman added inline comments. Comment at: include/clang/Frontend/PrecompiledPreamble.h:248 +/// doesn't restore the state \p CI had before calling AddImplicitPreamble, only +/// clears relevant settings, so that preamble is disabled in \p CI. +} // namespace clang

[PATCH] D34175: [driver][macOS] Pick the system version for the deployment target if the SDK is newer than the system

This revision was automatically updated to reflect the committed changes. Closed by commit rL305678: [driver][macOS] Pick the system version for the deployment target (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34175?vs=102607&id=103009#toc Repository: rL LLVM

[PATCH] D34185: [Parser][ObjC] Avoid crashing when skipping to EOF while parsing an ObjC interface/implementation

arphaman marked an inline comment as done. arphaman added inline comments. Comment at: lib/Parse/ParseObjc.cpp:220 CheckNestedObjCContexts(AtLoc); + if (isEofOrEom()) +return nullptr; ahatanak wrote: > Do you need this check here (and below)? Not anymore.

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