[PATCH] D67773: [clang-format[PR43144] Add support for SpaceAroundBraces style

2019-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D67773#1694797 , @mitchell-stellar wrote: > This needs to be more thought out. At the very least it needs to cover more > keywords like `do`, `switch`, `try`, and `catch`. There may be more. > Consideration also needs

[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

2019-10-04 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar created this revision. mitchell-stellar added reviewers: MyDeveloperDay, reuk, owenpan. mitchell-stellar added a project: clang-format. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch makes the `SpacesInSquareBrackets` setting also apply to C++ la

[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

2019-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thanks for the patch. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68473/new/ https://reviews.llvm.org/D68473 ___

[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

2019-10-04 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added a comment. Thanks, please commit on my behalf. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68473/new/ https://reviews.llvm.org/D68473 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D67775: [Sema] Split out -Wformat-type-confusion from -Wformat-pedantic

2019-10-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL373774: [Sema] Split out -Wformat-type-confusion from -Wformat-pedantic (authored by epilk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

2019-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, owenpan, mitchell-stellar. MyDeveloperDay added a project: clang-format. Herald added a project: clang. clang-format is incorrectly thinking the parameter parens are part of a cast operation, this is resulting in there

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

2019-10-04 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar requested changes to this revision. mitchell-stellar added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/TokenAnnotator.cpp:1617 +if (Tok.Next->isOneOf(tok::kw_noexcept, tok::kw_volatile, tok::kw_const, +

[PATCH] D68482: [clang] fix a typo from r372531

2019-10-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL373792: [clang] fix a typo from r372531 (authored by yuanfang, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D6

[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

2019-10-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2613 + TT_StructuredBindingLSquare, + TT_LambdaLSquare) && Style.SpacesInSquareBrackets && Right.isNot(tok::r_square)); -

[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

2019-10-05 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL373821: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too (authored by paulhoad, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Change

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

2019-10-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 223369. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Add additional override and final keywords CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68481/new/ https://reviews.llvm.org/D68481 Files: clang/lib/F

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

2019-10-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1617 +if (Tok.Next->isOneOf(tok::kw_noexcept, tok::kw_volatile, tok::kw_const, + tok::kw_throw, tok::l_square, tok::arrow)) + return false; mit

[PATCH] D68539: fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-05 Thread Daniel via Phabricator via cfe-commits
Daniel599 created this revision. Daniel599 added reviewers: llvm-commits, alexfh, alexfh_. Daniel599 added projects: clang-tools-extra, LLVM. Herald added a project: clang. Herald added a subscriber: cfe-commits. Daniel599 edited the summary of this revision. This patch fixes 'Bug 41120' https://b

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-05 Thread Daniel via Phabricator via cfe-commits
Daniel599 updated this revision to Diff 223372. Daniel599 added a comment. code fixes according to code-review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68539/new/ https://reviews.llvm.org/D68539 Files: clang-tools-extra/clang-tidy/

[PATCH] D61256: [clang-format][docs] Fix the Google C++ and Chromium style guide URLs

2019-10-06 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL373844: [clang-format][docs] Fix the Google C++ and Chromium style guide URLs (authored by paulhoad, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D63640: [clang] Improve Serialization/Imporing/Dumping of APValues

2019-10-06 Thread Tyker via Phabricator via cfe-commits
Tyker marked 10 inline comments as done. Tyker added a comment. update done tasks. Comment at: clang/lib/AST/APValue.cpp:599 Out << '[' << Path[I].getAsArrayIndex() << ']'; -ElemTy = Ctx.getAsArrayType(ElemTy)->getElementType(); +ElemTy = cast(ElemTy)->

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

2019-10-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added a reviewer: mitchell-stellar. MyDeveloperDay added a project: clang-format. Herald added a project: clang. Before making a proposed change, ensure ClangFormat.cpp is fully clang-formatted, no functional change just clang-formatting using

[PATCH] D29039: Proposal for clang-format -r option

2019-10-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. When I first saw this issue some time ago I tended to agree with the comment regarding the "its not clang-formats" job to do that... However, over the last couple of months I've seen more and more twitter and stack-overflow comments regarding "how do I check my w

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This should really be a full context diff -U9 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68539/new/ https://reviews.llvm.org/D68539 ___ cfe-commits mailing li

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: mitchell-stellar, klimek, owenpan. MyDeveloperDay added projects: clang-format, clang-tools-extra. Herald added a subscriber: mgorny. Herald added a project: clang. Related somewhat to D29039: [clang-format] Proposal for clang-f

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D68554#1696675 , @klimek wrote: > Why not build a tool that takes the diff output of clang-format and changes > it to what you want? Wouldn't that be a couple of lines of python? Thanks for taking the time to look at t

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-06 Thread Daniel via Phabricator via cfe-commits
Daniel599 updated this revision to Diff 223429. Daniel599 added a comment. added -U99 to diff cmd Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68539/new/ https://reviews.llvm.org/D68539 Files: clang-tools-extra/clang-tidy/readability/Identi

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

2019-10-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @stringham you seemingly have marked this revision so that no one else can edit it, so I cannot add it to the clang-format project or reassign reviewers, could you please change it to be public, it also means others cannot request changes or approve it (if they

[PATCH] D31574: [clang-format] update documentation

2019-10-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31574/new/ https://reviews.llvm.org/D31574 __

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

2019-10-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D68551#1696598 , @Eugene.Zelenko wrote: > If clang-format if clang-formatted now completely, I think will be good idea > to add rule to check this during build, like Polly does. This is a great idea... Do you know if

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I logged the original bug and I like it! I think the warning is better than mutating with a prefix, Thank you. I'll let the code owners approve it, but you have my vote! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

2019-10-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think when polly's script detects a change it's just showing the diff, this might be so much nicer if we could implement D68554: [clang-format] Proposal for clang-format to give compiler style warnings Repository: rC Clang

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Using a suggestion from D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted. as an example of its usefulness, I modified my clang-format CMakeList.txt to do as Polly does and add clang-format check rules, c

[PATCH] D68569: [clang-format] Also look for .{ext}.clang-format file

2019-10-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thanks for the patch, I think I understand what you are trying to do, but I have a few questions. Is the premise here that you need slightly different styles for .cpp than for .h? could you explain why? whilst I can see there is value in having such fine control

[PATCH] D68568: [clang-format] Make '.clang-format' variants finding a loop

2019-10-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thank you for the patch, I think this looks cleaner and generally LGTM, I think its always good to put a little description of the change along with the NFC in the title to let people know that functionally there isn't any change without having to work though the

[PATCH] D68569: [clang-format] Also look for .{ext}.clang-format file

2019-10-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Comment at: clang/lib/Format/Format.cpp:2603 llvm::SmallVector FilesToLookFor; + Nit: almost every file has an extension should this be 3? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

2019-10-07 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar accepted this revision. mitchell-stellar added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68481/new/ https://reviews.llvm.org/D68481 ___ cfe-commits mailin

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

2019-10-07 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar accepted this revision. mitchell-stellar added a comment. This revision is now accepted and ready to land. LGTM. I agree that a system in place that either enforces clang-formatting on commit or after the fact would be ideal. Otherwise, I don't see a need to have to approve the

[PATCH] D29039: [clang-format] Proposal for clang-format -r option

2019-10-07 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added a subscriber: djasper. mitchell-stellar added a comment. I agree with @djasper that this is outside the scope of clang-format. git for Windows gives you a full set of bash utilities to utilize, so doing stuff like this on Windows is much easier now. Repository: rL LLVM

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-07 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added a comment. I don't care for the command line switches that try to emulate compiler flags. I am also of the opinion that external scripts should be used for this functionality. git for Windows gives you a full set of bash utilities to utilize, so doing stuff like this on W

[PATCH] D63960: [C++20] Add consteval-specific semantic for functions

2019-10-07 Thread Tyker via Phabricator via cfe-commits
Tyker updated this revision to Diff 223624. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63960/new/ https://reviews.llvm.org/D63960 Files: clang/include/clang/AST/DeclCXX.h clang/include/clang/Basic/DiagnosticASTKinds.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/inc

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

2019-10-07 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL373921: [clang-format] [NFC] Ensure clang-format is itself clang-formatted. (authored by paulhoad, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to c

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

2019-10-07 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL373922: [clang-format] [PR27004] omits leading space for noexcept when formatting… (authored by paulhoad, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D68554#1697857 , @mitchell-stellar wrote: > I don't care for the command line switches that try to emulate compiler > flags. I am also of the opinion that external scripts should be used for this > functionality. git f

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-08 Thread Daniel via Phabricator via cfe-commits
Daniel599 added a comment. In D68539#1696864 , @MyDeveloperDay wrote: > I logged the original bug and I like it! > > I think the warning is better than mutating with a prefix, Thank you. > > I'll let the code owners approve it, but you have my vote! gla

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > I saw this issue when I submitted mine > (https://bugs.llvm.org/show_bug.cgi?id=43306) which for now seems to be a > harder fix, still looking for a solution there. oh boy!...are you gonna have to look at every variable/macro in scope and compare? Reposito

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

2019-10-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'd like to assess where we are at with this revision, despite our concerns over additional complexity of clang-format, I don't think this is adding too much (I've seen far worse patches) It appears to me that these changes are mainly in mustBreak,canBreak etc...

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-08 Thread Daniel via Phabricator via cfe-commits
Daniel599 added a comment. In D68539#1699288 , @MyDeveloperDay wrote: > > I saw this issue when I submitted mine > > (https://bugs.llvm.org/show_bug.cgi?id=43306) which for now seems to be a > > harder fix, still looking for a solution there. > > oh boy

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

2019-10-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I tried the patch and rebased it locally (as there were conflicts with the current trunk) The change caused other tests to fail (which doesn't completely surprise me) most tests failures look associated with positioning of trailing brace for example @@ -1,4 +

[PATCH] D68682: Clang-tidy fixes should avoid creating new blank lines

2019-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Sorry, I'm going to give a drive-by comment (which you can ignore), mainly by because you mentioned `clang-format`. This seems like a good idea as obviously it solves this problem, however, isn't rather like trying to fix it after the fact? what if (I'm sure the

[PATCH] D68570: Unify the two CRC implementations

2019-10-09 Thread Hans via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1e1e3ba2526e: Unify the two CRC implementations (authored by hansw). Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

2019-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Only because I had problem finding the commit, this is as a consequence of D68072: [clang-format] Reference qualifiers in member templates causing extra indentation. Repository: rC Clang CHANGES SINCE LAST ACTION https://re

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

2019-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thank you for fixing this, I'm sorry my review probably wasn't thorough enough in the first place, this LGTM and thank you for the very thorough description as this really help

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

2019-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:1499 +} else if (Current.isOneOf(tok::identifier, tok::kw_const, + tok::kw_noexcept) && Current.Previous && krasimir wrote: > MyDevel

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

2019-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: mitchell-stellar, STL_MSFT, klimek, krasimir. MyDeveloperDay added projects: clang-format, clang-tools-extra. Herald added a project: clang. MyDeveloperDay edited the summary of this revision. An incorrect assertion is thrown wh

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

2019-10-09 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:847 case tok::kw_while: - assert(!Line.startsWith(tok::hash)); - if (Tok->is(tok::kw_if) && CurrentToken && - CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

2019-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 224075. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. I'm not even sure if the assertion is valid? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68707/new/ https://reviews.llvm.org/D68707 Files: clang/lib

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

2019-10-09 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added a comment. To me, whether or not the assertion was valid is irrelevant. That assertion was intentionally added, and its replacement with an `if()` statement materially changes the inputs and outputs of the function. I'm questioning whether the input of a "while" token wit

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

2019-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:847 case tok::kw_while: - assert(!Line.startsWith(tok::hash)); - if (Tok->is(tok::kw_if) && CurrentToken && - CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier)) -

[PATCH] D68716: [clang] prevent crash for nonnull attribut in constant context (Bug 43601)

2019-10-09 Thread Tyker via Phabricator via cfe-commits
Tyker created this revision. Tyker added a reviewer: rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. bug : https://bugs.llvm.org/show_bug.cgi?id=43601 Repository: rC Clang https://reviews.llvm.org/D68716 Files: clang/lib/AST/ExprConstant.cpp clang/test/SemaCXX

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

2019-10-09 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar accepted this revision. mitchell-stellar added a comment. This revision is now accepted and ready to land. Okay. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68707/new/ https://reviews.llvm.org/D68707 ___ cfe-commit

[PATCH] D68720: Support -fstack-clash-protection for x86

2019-10-09 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision. Herald added subscribers: llvm-commits, cfe-commits, hiraditya, dschuff. Herald added projects: clang, LLVM. Implement protection against the stack clash attack [0]. Probe stack allocation every PAGE_SIZE during frame lowering or dynamic allocation to mak

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

2019-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > Btw, I am the author of the CMakeLists snippet quoted by @MyDeveloperDay. > Before that, it was a shell script that didn't run on Windows. Making it part > of the regression test basically eliminated all discussion about code > formatting, but we had to run l

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. As this behaviour is not something clang-format should do because there are external tool that can do this for us, I thought I'd set myself a challenge of trying to write this with external tools, to prove to myself that it was indeed easy, what I want to achieve

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

2019-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @poelmanc thank you for the very detailed explanation to what was probably my very naive point, I have to say with more information I am inclined to agree with you that perhaps you need to see all the replacements as a whole to make reasonable assumptions. But I

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-09 Thread Daniel via Phabricator via cfe-commits
Daniel599 marked 3 inline comments as done. Daniel599 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:868 + if (CheckNewIdentifier != Idents.end() && + CheckNewIdentifier->second->isKeyword(getLangOpts())) { +

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-09 Thread Daniel via Phabricator via cfe-commits
Daniel599 updated this revision to Diff 224151. Daniel599 added a comment. code-review fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68539/new/ https://reviews.llvm.org/D68539 Files: clang-tools-extra/clang-tidy/readability/IdentifierNamin

[PATCH] D68720: Support -fstack-clash-protection for x86

2019-10-09 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. @efriedma : there's indeed an intersection with the `probe-stack` attribute. The `probe-stack` attribute (a) forces a function call, and (b) this function call only happens **before** the stack gets expanded. (a) is probably a performance issue in several case

[PATCH] D68716: [clang] prevent crash for nonnull attribut in constant context (Bug 43601)

2019-10-09 Thread Tyker via Phabricator via cfe-commits
Tyker updated this revision to Diff 224169. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68716/new/ https://reviews.llvm.org/D68716 Files: clang/lib/AST/ExprConstant.cpp clang/test/SemaCXX/attr-nonnull.cpp Index: clang/test/SemaCXX/attr-nonnull.cpp

[PATCH] D68720: Support -fstack-clash-protection for x86

2019-10-09 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 224196. serge-sans-paille added a comment. Move to `stack-probe` compatibility, using a dedicated name to trigger inline assembly. It looks better to me because 1. it leverage existing mechanics 2. it has a finer grain Repository: rG LLVM Githu

[PATCH] D68720: Support -fstack-clash-protection for x86

2019-10-09 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. @efriedma alos compared to `probe-stack` with a function, this version has the ability to use existing MOV operations to avoid generating probes, which looks like a big plus to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D68720: Support -fstack-clash-protection for x86

2019-10-09 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 224254. serge-sans-paille added a comment. Added documentation + release note entry Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 Files: clang/docs/Releas

[PATCH] D29039: [clang-format] Proposal for clang-format -r option

2019-10-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Comment at: tools/clang-format/ClangFormat.cpp:345 + + Regex regex(FileNames[i]); + I could imagine the path you supplying being just a directory . or ../../include/Format as much as a wild card, once in that directory I

[PATCH] D68720: Support -fstack-clash-protection for x86

2019-10-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a subscriber: eli.friedman. serge-sans-paille added a comment. @efriedma the free probe algorithm requires more testing, and I'd like to take into account memset and memcpy as free probes too. To showcase this algorithm, consider the following LLVM bitcode: define i32

[PATCH] D68716: [clang] prevent crash for nonnull attribut in constant context (Bug 43601)

2019-10-10 Thread Tyker via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG59c6df9b2c52: [clang] prevent crash for nonnull attribut in constant context (Bug 43601) (authored by Tyker). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D6

[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

2019-10-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, mitchell-stellar, owenpan. MyDeveloperDay added a project: clang-format. Herald added a project: clang. I don't think its the main()'s responsibility to be working out how to dump the config, or the format()'s responsi

[PATCH] D68720: Support -fstack-clash-protection for x86

2019-10-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille marked 3 inline comments as done. serge-sans-paille added inline comments. Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:400 +!(STI.isOSWindows() && !STI.isTargetMachO()); +if (InlineStackClashProtector && !InEpilogue) { + const uint64_t

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/tools/clang-format/ClangFormat.cpp:371 + if (!Replaces.empty()) { +IntrusiveRefCntPtr DiagOpts = +new DiagnosticOptions(); klimek wro

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. > I think it's easy enough to do as a kind of driver, either in python or C++, > but then again, at that point putting it into ClangFormat seems fine. One > thing that I find on the one side kinda nice, but on the o

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 224403. MyDeveloperDay added a comment. builds ontop of D68767: [clang-format] NFC - Move functionality into functions to help code structure move replacement warnings into a separate function and move outputXML out i

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D68554#1703596 , @klimek wrote: > I ran through the same line of reasoning in my head when writing my last > comment :) The argument about the readability of error messages by tools is > definitely a good one, so I'd sa

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

2019-10-10 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3b4c8f680712: [clang-format] throws an incorrect assertion in consumeToken() formatting the… (authored by MyDeveloperDay). Changed prior to commit: https://reviews.llvm.org/D68707?vs=224075&id=224415#to

[PATCH] D68720: Support -fstack-clash-protection for x86

2019-10-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 224431. serge-sans-paille edited the summary of this revision. serge-sans-paille added a comment. Added test case, statistics and refactor interactions with existing stack probing mechanism. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D68720: Support -fstack-clash-protection for x86

2019-10-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. Some early stats: on the sqlite amalgamation [0], the free probe reuse allows to skip 123 out of the 474 probes needed during frame lowering. [0] https://www.sqlite.org/download.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D29039: [clang-format] Proposal for clang-format -r option

2019-10-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D29039#1704166 , @zturner wrote: > In Windows you just write a Python script to do this, and this works > everywhere, not just on one platform or the other, so bash isn't even > necessary and Python is easy to write so

[PATCH] D61663: [clang-format] Fix a JavaScript import order bug.

2019-10-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Are you waiting for someone to land this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61663/new/ https://reviews.llvm.org/D61663 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D16066: [clang-format] Add BeforeWhileInDoWhile BraceWrapping option

2019-10-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This change need unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D16066/new/ https://reviews.llvm.org/D16066 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mail

[PATCH] D14927: clang-format: Add SpaceBeforeTemplateParameterList option

2019-10-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This revision would require unit tests to proceed CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14927/new/ https://reviews.llvm.org/D14927 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D56345: [clang-format] Assert that filenames are not empty

2019-10-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thanks for the patch, I'm sorry it's taken so long sometimes items get lost...This looks to still be relevant, I think you'll need to rebase but it LGTM $ clang-format --ass

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

2019-10-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. While reviewing old revisions either not landed or unreviewed I stumbled across this accepted review. Looking at the current code I think its not needed anymore. Perhaps if we merge your test in with the current trunk we can determine if this is still needed C

[PATCH] D63960: [C++20] Add consteval-specific semantic for functions

2019-10-11 Thread Tyker via Phabricator via cfe-commits
Tyker updated this revision to Diff 224588. Tyker added a comment. improve performance in a bad case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63960/new/ https://reviews.llvm.org/D63960 Files: clang/include/clang/AST/DeclCXX.h clang/include/clang/AST/Expr.h clang/include/clan

[PATCH] D68720: Support -fstack-clash-protection for x86

2019-10-11 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 224606. serge-sans-paille added a comment. Ensure the distance between two probes is at max PAGE_SIZE. Use Calls as free probes. Fix alignment for dynamic alloca This passes the llvm-test suite, and thanks to the use of calls, no inserted probe are

[PATCH] D47111: : Implement monotonic_buffer_resource.

2019-10-11 Thread strager via Phabricator via cfe-commits
strager added inline comments. Comment at: include/experimental/memory_resource:427 +static _LIBCPP_CONSTEXPR const size_t __default_buffer_capacity = 1024; +static _LIBCPP_CONSTEXPR const size_t __default_buffer_alignment = 16; + Nit: Why isn't `__defaul

[PATCH] D47111: : Implement monotonic_buffer_resource.

2019-10-11 Thread strager via Phabricator via cfe-commits
strager added inline comments. Comment at: include/experimental/memory_resource:433 +char *__cur_; +size_t __align_; +size_t __allocation_size() { > Eric suggests replacing size_t __align_ with uint8_t __log2_align_. I'm > amenable to thi

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 224742. MyDeveloperDay added a comment. I'm going to commit this as it was approved, but I've added some simple lit tests, which exposed a double free, I fixed that up, and for the lit tests not to fail with an exit code I now return WarningAsError fr

[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 6 inline comments as done. MyDeveloperDay added a comment. I'll likely abandon this review when D68554: [clang-format] Proposal for clang-format to give compiler style warnings lands Comment at: clang/tools/clang-format/

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 224743. MyDeveloperDay added a comment. Incorporate feedback from @owenpan on D68767: [clang-format] NFC - Move functionality into functions to help code structure which this revision is reliant upon. CHANGES SINCE

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1f20bc17d005: [clang-format] Proposal for clang-format to give compiler style warnings (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: bruno, arphaman, klimek, owenpan, mitchell-stellar, dexonsmith. MyDeveloperDay added projects: clang, clang-format, clang-tools-extra. Review comments on D68767: [clang-format] NFC - Move functionality into functions to help c

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-12 Thread Daniel via Phabricator via cfe-commits
Daniel599 marked an inline comment as done. Daniel599 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:868 + if (CheckNewIdentifier != Idents.end() && + CheckNewIdentifier->second->isKeyword(getLangOpts())) { +

[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay abandoned this revision. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Review comments regarding movign getInvalidBOM are addressed in D68914: [clang-format] Remove duplciate code from Invalid BOM detection Rep

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 224776. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Updating with review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D68914 Files: clang/include/clang/Basi

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > I've reverted this for now to green up the bots. > > For the reland, did y'all consider the impact of this on clang-format build > time (it now depends on Frontend and hence on Driver and Sema, and Sema is > slow to build) and binary size (it's now basically im

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-13 Thread Daniel via Phabricator via cfe-commits
Daniel599 updated this revision to Diff 224783. Daniel599 added a comment. Added check for macro definition, please re-review the enum `ShouldFixStatus` as now I use it as a switch-case within the diagnostic message. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D68554#1707537 , @thakis wrote: > > Thanks for doing this, I am struggling to find the MacOS bot log that > > failed, are any available via Buildbot? (I notice the log looks like your > > own machine) > > The mac bots a

<    25   26   27   28   29   30   31   32   33   34   >