[PATCH] D33029: [clang-format] add option for dangling parenthesis

zroug added a comment. I'd also like to note that this is the code style preferred by most modern code formatters that I know of and use: - rustfmt for rust code - prettier for javascript code - black for python code https://reviews.llvm.org/D33029 __

[PATCH] D41077: [analyser] different.CallArgsOrder checker implementation

anna resigned from this revision. anna added a comment. Perhaps added me incorrectly as reviewer? Repository: rC Clang https://reviews.llvm.org/D41077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk created this revision. reuk added reviewers: Typz, krasimir, cfe-commits. This patch aims to add support for the following rules from the JUCE coding standards: - Always put a space before an open parenthesis that contains text - e.g. foo (123); - Never put a space before an empty pair of

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk added a subscriber: klimek. reuk added a comment. Would someone review this please? I'm not sure who to add for review (sorry), maybe one of the following? @klimek @Typz @krasimir Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk updated this revision to Diff 189942. reuk added a comment. I've rebased onto master, and removed unrelated formatting changes. I've also tried to remove some of the duplicate parens-related expressions. I agree that the heavy nested boolean expressions are a bit painful to read, but I'm n

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk added a comment. Thanks for the review, and for approving this PR. It's very much appreciated! I don't have merge rights - could someone commit this for me please? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55170 _

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

reuk added a comment. Does it make sense to allow `AllowShortIfElseStatementsOnASingleLine` to be enabled when `AllowShortIfStatementsOnASingleLine` is not? Perhaps it would be better to have `AllowShortIfStatementsOnASingleLine` be represented by an enum with values `Never`, `WithoutElse`, `Al

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

reuk added a comment. This looks much better now, thanks for taking another look! I've flagged some formatting/spelling nits, and I think the tests/docs could be a little more explicit about the behaviour of `WithoutElse`. Other than that, looks great! Comment at: clang/docs/

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

reuk added a comment. The code looks good now. There's just a few places left where the formatting looks a bit suspect (sorry for missing those last time). I think once that's fixed this will be good to go. Comment at: clang/unittests/Format/FormatTest.cpp:553 - AllowSimpl

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

reuk accepted this revision. reuk added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59087/new/ https://reviews.llvm.org/D59087 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk added a comment. @MyDeveloperDay I'm not sure you built this branch. Perhaps you applied this patch to an older version of the repo. For me, SpaceBeforeCpp11BracedListOptions is defined at Format.h:1570. This builds fine for me, on macOS 10.14 with Xcode 10.1's clang. I just rebased onto

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk updated this revision to Diff 191015. reuk added a comment. @MyDeveloperDay I'm sorry, you're absolutely correct. I'd got some other JUCE-related changes mixed up in this PR. Should be fixed now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk updated this revision to Diff 191050. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55170 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp lib/Format/TokenAnnotator.h unittests/Format/FormatTest.c

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk added a comment. @klimek I agree that the rule is somewhat arbitrary. However, it's the style rule of an established codebase with many users (I don't have a concrete number, but the project has 1400 stars on github ). I've found this patch useful when c

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk marked an inline comment as done. reuk added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2546-2560 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) || (Style.SpaceBeforeParens != FormatStyle::SBPO_Never && -(Left.isOneOf(tok::

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk updated this revision to Diff 191560. reuk added a comment. Removed unnecessary parens. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55170 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp lib/For

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

This revision was automatically updated to reflect the committed changes. Closed by commit rL357344: [clang-format]: Add NonEmptyParentheses spacing option (authored by reuk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

reuk created this revision. reuk added reviewers: MyDeveloperDay, klimek. reuk added a project: clang-tools-extra. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch aims to support the following rule from the Juce coding standards

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

reuk updated this revision to Diff 193889. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 Files: clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp I

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

reuk updated this revision to Diff 194055. reuk added a comment. Updated with fixes. Thanks for pointing out that the options are ordered alphabetically, I hadn't noticed that! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 Files: clang/docs

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

reuk added a comment. I updated `ClangFormatStyleOptions.rst` by hand, is there a way to do that automatically instead (for future patches)? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 ___ cfe-c

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

reuk updated this revision to Diff 194057. reuk added a comment. Fixed formatting nit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format/Forma

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

reuk closed this revision. reuk added a comment. Closed by https://reviews.llvm.org/rG91f60b44958f, https://reviews.llvm.org/rL357908, https://reviews.llvm.org/rC357908 (sorry, I forgot to update the body of the commit message to close this automatically) CHANGES SINCE LAST ACTION https://re

[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers

reuk added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2009 + // classes case + if (Style.AccessModifierIndentation && Line->Level % 2 == 0) +--Line->Level; klimek wrote: > What if the class starts at level 1? (for example, inside

[PATCH] D60726: Fixed -Wconversion-null warning in GCC.

reuk accepted this revision. reuk added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60726/new/ https://reviews.llvm.org/D60726 ___ cfe-commits mailin

[PATCH] D60726: Fixed -Wconversion-null warning in GCC.

This revision was automatically updated to reflect the committed changes. Closed by commit rL358441: [clang-format] Fix -Wconversion-null warning in GCC (authored by reuk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D69144: [Format] Add format check for throwing negative numbers

jonathoma created this revision. jonathoma added a reviewer: modocache. jonathoma added a project: clang-format. Herald added a project: clang. Herald added a subscriber: cfe-commits. jonathoma edited the summary of this revision. jonathoma added reviewers: sammccall, Quuxplusone. The code `throw

[PATCH] D69179: [Format] Add format check for coroutine keywords with negative numbers

jonathoma created this revision. Herald added subscribers: cfe-commits, modocache. Herald added a project: clang. jonathoma retitled this revision from "[Format] Add format check for throwing negative numbers" to "[Format] Add format check for coroutine keywords with negative numbers". jonathoma

[PATCH] D69180: [Format] Add format check for coroutine keywords with negative numbers

jonathoma created this revision. jonathoma added reviewers: modocache, sammccall, Quuxplusone. Herald added a project: clang. Herald added a subscriber: cfe-commits. jonathoma planned changes to this revision. As a followup to D69144 , this diff fixes the coroutin

[PATCH] D69180: [Format] Add format check for coroutine keywords with negative numbers

jonathoma updated this revision to Diff 225672. jonathoma added a comment. Rebase properly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69180/new/ https://reviews.llvm.org/D69180 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Form

[PATCH] D69180: [Format] Add format check for coroutine keywords with negative numbers

jonathoma updated this revision to Diff 225671. jonathoma added a comment. Rebase onto master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69180/new/ https://reviews.llvm.org/D69180 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/F

[PATCH] D155145: [X86] Add AVX-VNNI-INT16 instructions.

anna added a comment. We see a crash bisected to this patch about using an illegal instruction. Here's the CPUInfo for the machine: CPU info: current cpu id: 22 total 32(physical cores 16) (assigned logical cores 32) (assigned physical cores 16) (assigned_sockets:2 of 2) (8 cores per cpu,

[PATCH] D155145: [X86] Add AVX-VNNI-INT16 instructions.

anna added a comment. In D155145#4544068 , @pengfei wrote: > In D155145#4543326 , @anna wrote: > >> We see a crash bisected to this patch about using an illegal instruction. >> Here's the CPUInfo for the machine:

[PATCH] D155145: [X86] Add AVX-VNNI-INT16 instructions.

anna added a comment. In D155145#4551621 , @craig.topper wrote: > In D155145#4551526 , @anna wrote: > >> In D155145#4544068 , @pengfei >> wrote: >> >>> In D155145#454332

[PATCH] D155145: [X86] Add AVX-VNNI-INT16 instructions.

anna added a comment. > Can you capture the values of EAX, EBX, ECX, and EDX after the two calls to > getX86CpuIDAndInfoEx that have 0x7 as the first argument? Maybe there's a bug > in CPUID on Sandy Bridge. Sure, on the original code before the patch you suggested right? The two calls are:

[PATCH] D155145: [X86] Add AVX-VNNI-INT16 instructions.

anna added a comment. In D155145#4554786 , @anna wrote: >> Can you capture the values of EAX, EBX, ECX, and EDX after the two calls to >> getX86CpuIDAndInfoEx that have 0x7 as the first argument? Maybe there's a >> bug in CPUID on Sandy Bridge. > > Sure

[PATCH] D155145: [X86] Add AVX-VNNI-INT16 instructions.

anna added a comment. thank you @craig.topper and @pengfei . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155145/new/ https://reviews.llvm.org/D155145 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked an inline comment as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 + if (NumInstChecked++ > MaxInstCheckedForThrow || + isGuaranteedToTransferExecutionToSuccessor(&I)) +return true; --

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 251901. anna added a comment. Noticed while adding couple more tests, there were 2 bugs: 1 the isGuaranteedToTransferExecutionToSuccessor check should be inverted 2. make_range should be until the return instruction - so we do not want std::prev on the returnI

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked an inline comment as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 + if (NumInstChecked++ > MaxInstCheckedForThrow || + isGuaranteedToTransferExecutionToSuccessor(&I)) +return true; --

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked an inline comment as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 + if (NumInstChecked++ > MaxInstCheckedForThrow || + isGuaranteedToTransferExecutionToSuccessor(&I)) +return true; --

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna added a comment. ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 252868. anna added a comment. NFC w.r.t prev diff. Use VMap.lookup instead of a lambda which does the same. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 Files: clang/

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked 3 inline comments as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1159 + + auto MayContainThrowingOrExitingCall = [&](Instruction *RVal, + Instruction *RInst) {

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 253704. anna added a comment. addressed review comments. Added two test cases: deref value being different, inlined callee body better optimized compared to callee. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked 3 inline comments as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1175 + continue; +// Sanity check that the cloned return instruction exists and is a return +// instruction itself. anna wrote

[PATCH] D76140: [InlineFunction] update attributes during inlining

This revision was automatically updated to reflect the committed changes. Closed by commit rG28518d9ae39f: [InlineFunction] Handle return attributes on call within inlined body (authored by anna). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/n

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna added a comment. I got a failure in one of the binaryFormats: lib/BinaryFormat/CMakeFiles/LLVMBinaryFormat.dir/MsgPackReader.cpp Attributes 'zeroext and signext' are incompatible! %rev.i.i.i.i.i.i.i.i = tail call signext zeroext i16 @llvm.bswap.i16(i16 %ret.0.copyload.i.i.i.i) #6 in

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna planned changes to this revision. anna added a comment. see above comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 ___ cfe-commits mailing list cfe

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna reopened this revision. anna added a comment. This revision is now accepted and ready to land. In D76140#1953201 , @anna wrote: > I got a failure in one of the binaryFormats: > lib/BinaryFormat/CMakeFiles/LLVMBinaryFormat.dir/MsgPackReader.cpp > >

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked an inline comment as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1175 + continue; +// Sanity check that the cloned return instruction exists and is a return +// instruction itself. anna wrote

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna requested review of this revision. anna added a comment. fixed buildbot failure. see above comments and added testcase `test8`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 ___

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 254213. anna added a comment. This revision is now accepted and ready to land. whitelist valid return attributes and only add those. Added testcase for signext. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 254222. anna added a comment. fixed missing code left out during rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 Files: llvm/lib/Transforms/Utils/InlineFunction.

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna added a comment. In D76140#1957416 , @reames wrote: > LGTM again, with minor change. will update it. > p.s. Sorry for missing the functional issue the first time. All of the test > changes should have made the issue obvious, but despite reading t

[PATCH] D76140: [InlineFunction] update attributes during inlining

This revision was automatically updated to reflect the committed changes. Closed by commit rGbf7a16a76871: [InlineFunction] Update valid return attributes at callsite within callee body (authored by anna). Changed prior to commit: https://reviews.llvm.org/D76140?vs=254222&id=254573#toc Reposit

[PATCH] D69180: [Format] Add format check for coroutine keywords with negative numbers

jonathoma added a comment. @modocache Feel free to commit from my end! Thanks again :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69180/new/ https://reviews.llvm.org/D69180 ___ cfe-commits mailing l

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 251447. anna added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. fixed clang tests. rot-intrinsics.c testcase has 5 different RUNs with 3 prefixes. Depending on target-triple, the attribute is added to the caller, so I've dis

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked an inline comment as done. anna added inline comments. Herald added a reviewer: aartbik. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 +return true; +return false; + }; jdoerfert wrote: > `mayThrow` is not sufficient. As w

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 251615. anna added a comment. use isGuaranteedToTransferExecutionToSuccessor instead of `MayThrow` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 Files: clang/test/Code