[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:93 + for (int I = PatN; I <= WordN; I++) +Best = std::max(Best, Scores[PatN][I][Match].Score); if (isAwful(Best)) sammccall wrote: > this looks like a behavior change - why? This is a

[PATCH] D44559: [Sema] Wrong width of result of mul operation

aaron.ballman added a comment. In https://reviews.llvm.org/D44559#1044639, @rjmccall wrote: > In https://reviews.llvm.org/D44559#1044186, @avt77 wrote: > > > >> In https://reviews.llvm.org/D44559#1040799, @rjmccall wrote: > > >> > > >>> I think we're correct not to warn here and that GCC/ICC

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

MaskRay updated this revision to Diff 139314. MaskRay edited the summary of this revision. MaskRay added a comment. Update summary Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 Files: clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h Index: clangd/FuzzyMatch.h

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

MaskRay updated this revision to Diff 139313. MaskRay added a comment. Update summary Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 Files: clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h Index: clangd/FuzzyMatch.h

[PATCH] D44559: [Sema] Wrong width of result of mul operation

rjmccall added a comment. In https://reviews.llvm.org/D44559#1044186, @avt77 wrote: > >> In https://reviews.llvm.org/D44559#1040799, @rjmccall wrote: > >> > >>> I think we're correct not to warn here and that GCC/ICC are being noisy. > >>> The existence of a temporary promotion to a wider

[PATCH] D44724: [Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin

This revision was automatically updated to reflect the committed changes. Closed by commit rL328114: [Fuchsia] Dont install libc++, libc++abi or libunwind on Darwin (authored by phosek, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

r328114 - [Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin

Author: phosek Date: Wed Mar 21 09:48:26 2018 New Revision: 328114 URL: http://llvm.org/viewvc/llvm-project?rev=328114=rev Log: [Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin The Clang driver doesn't currently know how to use the libraries that are shipped as part of the

[PATCH] D44272: [clangd] Support incremental document syncing

ilya-biryukov added inline comments. Comment at: clangd/DraftStore.h:36 /// Replace contents of the draft for \p File with \p Contents. - void updateDraft(PathRef File, StringRef Contents); + void addDraft(PathRef File, StringRef Contents); + simark wrote:

[PATCH] D44724: [Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin

phosek updated this revision to Diff 139311. https://reviews.llvm.org/D44724 Files: clang/cmake/caches/Fuchsia-stage2.cmake Index: clang/cmake/caches/Fuchsia-stage2.cmake === --- clang/cmake/caches/Fuchsia-stage2.cmake +++

[PATCH] D44272: [clangd] Support incremental document syncing

simark updated this revision to Diff 139310. simark marked an inline comment as done. simark added a comment. Address review comments, except LSP version check Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44272 Files: clangd/ClangdLSPServer.cpp clangd/DraftStore.cpp

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

vsapsai added a comment. In https://reviews.llvm.org/D44589#1044350, @aaron.ballman wrote: > This generally looks reasonable to me, but @rsmith should weigh in before you > commit because `MultiSourceLocation` is novel. Thanks for the review, Aaron. I tried not to do anything stupid with

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54 + } else if (const auto *const Call = + Result.Nodes.getNodeAs("ptr_fun_call")) { +diag(Call->getLocStart(), Message) << "'std::ptr_fun'"; + } else if

[clang-tools-extra] r328108 - [Fix] fix type deduction on ARM and MSVC

Author: jonastoth Date: Wed Mar 21 08:50:15 2018 New Revision: 328108 URL: http://llvm.org/viewvc/llvm-project?rev=328108=rev Log: [Fix] fix type deduction on ARM and MSVC Modified: clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp Modified:

[PATCH] D43967: [ASTImporter] Add test helper Fixture

a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. This looks good to me. Thank you! @xazax.hun Gabor, could you please take a look? Repository: rC Clang https://reviews.llvm.org/D43967

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

alexfh added inline comments. Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54 + } else if (const auto *const Call = + Result.Nodes.getNodeAs("ptr_fun_call")) { +diag(Call->getLocStart(), Message) << "'std::ptr_fun'"; + } else if (const

[PATCH] D44272: [clangd] Support incremental document syncing

simark marked 34 inline comments as done. simark added inline comments. Comment at: clangd/DraftStore.h:36 /// Replace contents of the draft for \p File with \p Contents. - void updateDraft(PathRef File, StringRef Contents); + void addDraft(PathRef File, StringRef

[PATCH] D44729: [ASTMatchers] Add hasSubExpr() matcher.

courbet added a comment. Thanks for the pointer. I missed the trampoline through GetSourceExpressionMatcher. Repository: rC Clang https://reviews.llvm.org/D44729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: r327959 - [ms] Parse #pragma optimize and ignore it behind its own flag

>From the bot changes, it seems that -Wunknown-pragma doesn't disable this new warning. Shouldn't it do that? On Tue, Mar 20, 2018, 9:55 AM Hans Wennborg via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: hans > Date: Tue Mar 20 01:53:11 2018 > New Revision: 327959 > > URL:

[clang-tools-extra] r328107 - [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

Author: jonastoth Date: Wed Mar 21 08:34:15 2018 New Revision: 328107 URL: http://llvm.org/viewvc/llvm-project?rev=328107=rev Log: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test The original check did break the green buildbot in the sanitizer build. It took a while to

[PATCH] D44426: Fix llvm + clang build with Intel compiler

mibintc added inline comments. Comment at: include/llvm-c/Target.h:25 -#if defined(_MSC_VER) && !defined(inline) +#if defined(_MSC_VER) && !defined(inline) && !defined(__INTEL_COMPILER) #define inline __inline yvvan wrote: > mibintc wrote: > > I really think

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

JonasToth added inline comments. Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:119 + if (!SwitchHasDefault && SwitchCaseCount == 0) { +diag(Switch->getLocStart(), "degenerated switch without labels"); +return; aaron.ballman wrote: >

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

JonasToth updated this revision to Diff 139304. JonasToth marked 3 inline comments as done. JonasToth added a comment. - add fixme for degenerated switch Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40737 Files: clang-tidy/hicpp/CMakeLists.txt

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:119 + if (!SwitchHasDefault && SwitchCaseCount == 0) { +diag(Switch->getLocStart(),

[PATCH] D38798: [OpenMP] Support for implicit "declare target" functions - Sema patch

grokos abandoned this revision. grokos added a comment. @ABataev came up with a much simpler solution to the implementation of `declare target`: https://reviews.llvm.org/rL327636 I am abandoning this obsolete revision. Repository: rL LLVM https://reviews.llvm.org/D38798

[PATCH] D43026: [OpenMP] CodeGen for the "declare target" directive - variables, functions, ctors/dtors

grokos abandoned this revision. grokos added a comment. @ABataev came up with a much simpler solution to the implementation of `declare target`: https://reviews.llvm.org/rL327636 I am abandoning this obsolete revision. Repository: rC Clang https://reviews.llvm.org/D43026

[PATCH] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic

djasper accepted this revision. djasper added a comment. Looks good. Repository: rC Clang https://reviews.llvm.org/D44632 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44692: [clang-format] Don't insert space between r_paren and 'new' in ObjC decl

djasper accepted this revision. djasper added a comment. Looks good. Repository: rC Clang https://reviews.llvm.org/D44692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r328100 - Make positionToOffset return llvm::Expected

Author: simark Date: Wed Mar 21 07:36:46 2018 New Revision: 328100 URL: http://llvm.org/viewvc/llvm-project?rev=328100=rev Log: Make positionToOffset return llvm::Expected Summary: To implement incremental document syncing, we want to verify that the ranges provided by the front-end are valid.

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG modulo other reviewers' open comments. https://reviews.llvm.org/D44231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [clang-tools-extra] r327833 - [clang-tidy] New check bugprone-unused-return-value

On Mon, Mar 19, 2018 at 10:43 PM wrote: > This should be fixed by r327911. > Thanks! > > > Douglas Yung > > > > *From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On > Behalf Of *Galina Kistanova via cfe-commits > *Sent:* Monday, March 19, 2018 13:13 >

[PATCH] D44673: Make positionToOffset return llvm::Expected

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE328100: Make positionToOffset return llvm::Expectedsize_t (authored by simark, committed by ). Changed prior to commit: https://reviews.llvm.org/D44673?vs=139292=139297#toc Repository: rL LLVM

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

This revision was automatically updated to reflect the committed changes. Closed by commit rL328101: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std… (authored by alexfh, committed by ). Herald added subscribers: llvm-commits, klimek. Changed prior to commit:

[PATCH] D44673: Make positionToOffset return llvm::Expected

This revision was automatically updated to reflect the committed changes. Closed by commit rL328100: Make positionToOffset return llvm::Expectedsize_t (authored by simark, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44673 Files:

[clang-tools-extra] r328101 - [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

Author: alexfh Date: Wed Mar 21 07:39:24 2018 New Revision: 328101 URL: http://llvm.org/viewvc/llvm-project?rev=328101=rev Log: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique Summary: For a c++11 code, the clang-tidy rule "modernize-make-unique" should

[PATCH] D44694: [clang-tidy] Use :doc: for check links in Release Notes

alexfh accepted this revision. alexfh added a comment. LG. Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44673: Make positionToOffset return llvm::Expected

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:199 +return End.takeError(); + + return formatCode(Code, File, {tooling::Range(*Begin, *End - *Begin)}); simark wrote: > ilya-biryukov wrote: > > NIT: unnecessary empty line > In

[PATCH] D43967: [ASTImporter] Add test helper Fixture

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:1169 + MatchVerifier Verifier; + ASSERT_TRUE(Verifier.match(From, cxxRecordDecl(has(fieldDecl(); + ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl(); a.sidorin

[PATCH] D43967: [ASTImporter] Add test helper Fixture

martong updated this revision to Diff 139295. martong marked 7 inline comments as done. martong added a comment. Add minor syntax changes and rename Repository: rC Clang https://reviews.llvm.org/D43967 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/DeclMatcher.h Index:

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

JonasToth added inline comments. Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:119 + if (!SwitchHasDefault && SwitchCaseCount == 0) { +diag(Switch->getLocStart(), "degenerated switch without labels"); +return; aaron.ballman wrote: > I think

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

JonasToth updated this revision to Diff 139293. JonasToth marked 4 inline comments as done. JonasToth added a comment. - adress review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40737 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark marked 6 inline comments as done. simark added inline comments. Comment at: clangd/ClangdServer.cpp:199 +return End.takeError(); + + return formatCode(Code, File, {tooling::Range(*Begin, *End - *Begin)}); ilya-biryukov wrote: > NIT: unnecessary empty

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark updated this revision to Diff 139292. simark marked 3 inline comments as done. simark added a comment. Remove spaces, add fixmes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44673 Files: clangd/ClangdServer.cpp clangd/SourceCode.cpp clangd/SourceCode.h

[PATCH] D44732: [clangd] Set CLANGD_EDITOR environment variable in vscode extension.

hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: ioeric, jkorous-apple, ilya-biryukov, klimek. This would allow us to log the editor using custom-built clangd. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44732 Files:

r328092 - clang-interpreter example cmake fix

Author: lukecheeseman Date: Wed Mar 21 05:05:19 2018 New Revision: 328092 URL: http://llvm.org/viewvc/llvm-project?rev=328092=rev Log: clang-interpreter example cmake fix Add in a space when appending the export to the linker options. Without the space the export is appended onto whatever the

[PATCH] D44673: Make positionToOffset return llvm::Expected

ilya-biryukov added inline comments. Comment at: clangd/SourceCode.h:41 /// Turn an offset in Code into a [line, column] pair. Position offsetToPosition(llvm::StringRef Code, size_t Offset); We should be consistent for all functions inside this fail. They

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

aaron.ballman added a comment. This generally looks reasonable to me, but @rsmith should weigh in before you commit because `MultiSourceLocation` is novel. https://reviews.llvm.org/D44589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44729: [ASTMatchers] Add hasSubExpr() matcher.

aaron.ballman added a comment. I think `hasSourceExpression()` already does what you're looking for. void f() { int i = 1.0f; } clang-query> match implicitCastExpr(hasSourceExpression(floatLiteral())) Match #1: C:\Users\aballman.GRAMMATECH\Desktop\test.c:2:11: note: "root" binds here

[PATCH] D43957: Fixing issue where a space was not added before a global namespace variable when SpacesInParentheses is set

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Please generally upload diffs with the full file as context so that Phabricator can properly expand the context where necessary. This looks good, though. Repository: rC Clang

[PATCH] D43967: [ASTImporter] Add test helper Fixture

a.sidorin added a comment. Hi Gabor! This looks much better. Comment at: unittests/AST/ASTImporterTest.cpp:1000 + R"( + template + struct X {}; In C++, names started with underscore are reserved for implementation so it's undesirable to use

[PATCH] D44555: clang-interpreter example cmake fix

LukeCheeseman closed this revision. LukeCheeseman added a comment. closed by https://reviews.llvm.org/rC328092 Repository: rC Clang https://reviews.llvm.org/D44555 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic

jolesiak added inline comments. Comment at: lib/Format/Format.cpp:1450 // Keep this array sorted, since we are binary searching over it. static constexpr llvm::StringLiteral FoundationIdentifiers[] = { "CGFloat", krasimir wrote: > jolesiak

[PATCH] D44673: Make positionToOffset return llvm::Expected

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. See the empty line NITS, though. Comment at: clangd/ClangdServer.cpp:199 +return End.takeError(); + + return formatCode(Code, File,

[PATCH] D41808: Rename clang link from clang-X.Y to clang-X

dim accepted this revision. dim added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D41808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

r328087 - [ASTMatchers] Remove extra qualifier for consistency (LibASTMatchersReference.html)

Author: courbet Date: Wed Mar 21 03:54:29 2018 New Revision: 328087 URL: http://llvm.org/viewvc/llvm-project?rev=328087=rev Log: [ASTMatchers] Remove extra qualifier for consistency (LibASTMatchersReference.html) + Regenerate doc. Modified: cfe/trunk/docs/LibASTMatchersReference.html

[PATCH] D44729: [ASTMatchers] Add hasSubExpr() matcher.

courbet created this revision. courbet added reviewers: aaron.ballman, alexfh. Herald added a subscriber: klimek. This is needed to implement more checks in https://reviews.llvm.org/D38455. Repository: rC Clang https://reviews.llvm.org/D44729 Files: docs/LibASTMatchersReference.html

r328086 - [ASTMatchers] Regenerate doc.

Author: courbet Date: Wed Mar 21 03:48:00 2018 New Revision: 328086 URL: http://llvm.org/viewvc/llvm-project?rev=328086=rev Log: [ASTMatchers] Regenerate doc. Modified: cfe/trunk/docs/LibASTMatchersReference.html Modified: cfe/trunk/docs/LibASTMatchersReference.html URL:

[PATCH] D44559: [Sema] Wrong width of result of mul operation

avt77 updated this revision to Diff 139271. avt77 added a comment. I removed the dependence on arch and updated the tests. https://reviews.llvm.org/D44559 Files: lib/Sema/SemaChecking.cpp test/Sema/conversion.c test/SemaCXX/conversion.cpp Index: test/SemaCXX/conversion.cpp

[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

courbet added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:26 + binaryOperator( + anyOf(hasOperatorName("+="), hasOperatorName("-=")), + hasLHS(hasType(isInteger())), aaron.ballman wrote: >

Re: [clang-tools-extra] r328069 - Revert "[lit] Adding config initialization to lit tests in clang-tools-extra"

On Wed, Mar 21, 2018 at 5:28 AM, Petr Hosek via cfe-commits wrote: > Author: phosek > Date: Tue Mar 20 19:28:22 2018 > New Revision: 328069 > > URL: http://llvm.org/viewvc/llvm-project?rev=328069=rev > Log: > Revert "[lit] Adding config initialization to lit tests in

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

yvvan added inline comments. Comment at: include/clang/Sema/CodeCompleteConsumer.h:565 + /// \brief For this completion result correction is required. + Optional Corr = None; + ilya-biryukov wrote: > Having a string replacement without an actual range to

[PATCH] D44426: Fix llvm + clang build with Intel compiler

yvvan added a comment. In https://reviews.llvm.org/D44426#1042162, @mibintc wrote: > I added some inline comments. You are using the Intel 18.0 compiler on > Windows - what version of Visual Studio is in the environment? Yes, I'm using 18.0 Comment at:

[PATCH] D44559: [Sema] Wrong width of result of mul operation

avt77 added a comment. >> In https://reviews.llvm.org/D44559#1040799, @rjmccall wrote: >> >>> I think we're correct not to warn here and that GCC/ICC are being noisy. >>> The existence of a temporary promotion to a wider type doesn't justify >>> warning on arithmetic between two operands that

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

sammccall added a comment. (Sorry if I sound gruff - I'm sure there are improvements to be had here. But since the code is a bit dense (my fault) I have trouble inferring them from the deltas. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

sammccall added a comment. Can you elaborate on what this patch is improving, and how? There are some stylistic changes, and also what look like subtle logic changes, and some rearrangement of the algorithm - to what end? Canonical way to run all tests is `ninja check-clang-tools`, the way you

[PATCH] D44189: [RISCV] Verify the input value of -march=

kito-cheng marked 4 inline comments as done. kito-cheng added a comment. Hi Alex: Thanks for your input, check for repeated letter was missed in my last patch :) Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:34 + +// The canonical order specified in ISA manual. +

[PATCH] D44189: [RISCV] Verify the input value of -march=

kito-cheng updated this revision to Diff 139256. kito-cheng added a comment. Update revision according Alex's review. Changes: - Add testcase for uppercase of -march string. - Add testcase for repeated letter in -march. - Add more comment. - Add several TODO item for diagnostic message

[PATCH] D44724: [Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. Lgtm with some more comments Repository: rC Clang https://reviews.llvm.org/D44724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

<    1   2