[PATCH] D94006: [Sema] Fix use-of-uninitialized-value cause by 89b0972aa2f58f927633c63570b36550a17f4e63

2021-01-03 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp created this revision. nullptr.cpp requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D94006 Files: clang/lib/Sema/SemaInit.cpp Index: clang/lib/Sema/SemaInit.cpp

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:130 +/// \endcode +ACA_AcrossComments + }; tinloaf wrote: > MyDeveloperDay wrote: > > Is there a case for aligning over empty lines and comments? > > > > ``` > > int a

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2021-01-03 Thread Ella Ma via Phabricator via cfe-commits
OikawaKirie added a comment. In D92634#2476161 , @danielmarjamaki wrote: >> Besides, the return value should be the exact value computed from the two >> integers, even unknown, rather than undefined. As the developers may >> overflow an integer on purpo

[PATCH] D93979: [clang-tidy] Fix windows tests

2021-01-03 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Looks like this worked :) Thanks again! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93979/new/ https://reviews.llvm.org/D93979 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D93979: [clang-tidy] Fix windows tests

2021-01-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. http://45.33.8.238/win/30678/summary.html It looks like it works, as nothing needed to be built it was a v fast build Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93979/new/ https://reviews.llvm.org/D93979 _

[PATCH] D93979: [clang-tidy] Fix windows tests

2021-01-03 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG59810c51e761: [clang-tidy] Fix windows tests (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93979/new/ https://reviews.llvm.org/D9397

[clang-tools-extra] 59810c5 - [clang-tidy] Fix windows tests

2021-01-03 Thread Nathan James via cfe-commits
Author: Nathan James Date: 2021-01-04T00:39:34Z New Revision: 59810c51e761f241f23f45a120d5c3518983d2d8 URL: https://github.com/llvm/llvm-project/commit/59810c51e761f241f23f45a120d5c3518983d2d8 DIFF: https://github.com/llvm/llvm-project/commit/59810c51e761f241f23f45a120d5c3518983d2d8.diff LOG:

[PATCH] D93901: [NFC] Renaming PackStack to AlignPackStack

2021-01-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:65-67 + // The #pragma align/pack affected a record in an included file, so Clang + // should warn when that pragma was written in a file that included the + // included file. --

[PATCH] D93979: [clang-tidy] Fix windows tests

2021-01-03 Thread Nico Weber via Phabricator via cfe-commits
thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Hey, thanks! I don't have a win machine at hand either, but let's land this and see what the win bot on http://45.33.8.238/ thinks about this. It should take at most 15 min to cycle. Reposi

[PATCH] D93999: [clang] Fix message text for `-Wpointer-sign` to account for plain char

2021-01-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: aaron.ballman, rsmith. Herald added subscribers: jfb, jvesely. hubert.reinterpretcast requested review of this revision. Herald added a project: clang. The `-Wpointer-sign` warning text is inappropriate for descr

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit added a comment. The test still is there; Git is just showing the diff strangely. I didn't modify that test at all. The corner case bug doesn't affect `FormatTest.ShortEnums` because that test effectively has `AfterEnum: false`. Should I add cases for `AfterEnum: true` in that test too?

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:1347 Style.AllowShortEnumsOnASingleLine = false; + verifyFormat("enum {\n" + " A,\n" + " B,\n" + " C\n" + "} ShortEnum1, ShortEnu

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit updated this revision to Diff 314301. atirit added a comment. Squashed commits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93938/new/ https://reviews.llvm.org/D93938 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Form

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit added a comment. I don't understand; should every commit I've made be squashed into one and then submitted here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93938/new/ https://reviews.llvm.org/D93938 _

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf marked an inline comment as done. tinloaf added inline comments. Comment at: clang/include/clang/Format/Format.h:130 +/// \endcode +ACA_AcrossComments + }; MyDeveloperDay wrote: > Is there a case for aligning over empty lines and comments? > > `

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:421 // matching token, the sequence ends here. - if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine) + if (((Changes[i].NewlinesBefore > 1) && (!AcrossEmpty)) || + (!F

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. I'm sorry, but now you are missing the files from the review, I think you diffing against your own commits and not commit that are in the repo. This makes the review

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:130 +/// \endcode +ACA_AcrossComments + }; Is there a case for aligning over empty lines and comments? ``` int a = 5; /* comment */ int oneTwoThree = 123; `

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit added a comment. `AfterEnum: true` currently overrides `AllowShortEnumsOnASingleLine: true`. If this is epxected behaviour then I'll modify the test to accomodate that, but otherwise, there's a separate issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit updated this revision to Diff 314299. atirit added a comment. Removed multiple enum names from new test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93938/new/ https://reviews.llvm.org/D93938 Files: clang/unittests/Format/FormatTest.cpp

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. we don't commit with failing tests so lets understand why it fails. Can you add the tests without multiple names for the enum Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93938/new/ https://reviews.llvm.org/D93938

[PATCH] D72281: [Matrix] Add matrix type to Clang.

2021-01-03 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7855 + return; +RowsExpr = Columns.get(); + } else { RKSimon wrote: > @fhahn Should this be ColsExpr? > ``` > ColsExpr = Columns.get(); > ``` > Noticed when looking at > https://llvm.

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit added a comment. The first test fails due to the aforementioned corner case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93938/new/ https://reviews.llvm.org/D93938 ___ cfe-commits mailing list c

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit updated this revision to Diff 314296. atirit added a comment. Added unit test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93938/new/ https://reviews.llvm.org/D93938 Files: clang/unittests/Format/FormatTest.cpp Index: clang/unittests/F

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf marked an inline comment as done. tinloaf added inline comments. Comment at: clang/include/clang/Format/Format.h:110 + + /// Styles for alignment of consecutive assignments + enum AlignConsecutiveAssignmentsStyle { MyDeveloperDay wrote: > you need to pr

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit added a comment. In D93938#2476519 , @MyDeveloperDay wrote: > You need to have these conversations by adding new unit tests that prove your > point, I highly doubt I'll personally be willing to accept any revision > without more unit tests than t

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 314293. tinloaf marked 3 inline comments as done. tinloaf added a comment. Address MyDeveloperDay's review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 Fil

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:12581 +"\n" +"/* block comment */\n" +"\n" can you add an example with a block comment that spans multiple lines e.g. ``` int a = 5 /* *

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. by and large, this looks pretty good to me.. Comment at: clang/docs/ClangFormatStyleOptions.rst:198 -**AlignConsecutiveAssignments** (``bool``) +**AlignConsecutiveAssignments** (``AlignConsecutiveAssignmentsStyle``) If ``true``, aligns conse

[PATCH] D93817: Update transformations to use poison for insertelement/shufflevector's placeholder value

2021-01-03 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 314289. aqjune added a comment. Rebase I'll continue splitting after working on lifetime patches Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93817/new/ https://reviews.llvm.org/D93817 Files: clang/test/Code

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

2021-01-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In D92936#2476356 , @vitalybuka wrote: > Something is not initialized > http://lab.llvm.org:8011/#/builders/74/builds/1834/steps/9/logs/stdio Confirmed; @nullptr.cpp what do you want to do about this? I hypothesize that maybe

[PATCH] D93986: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 314284. tinloaf added a comment. Fix formatting issue resulting from ancient clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 Files: clang/docs/ClangForm

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:1346 verifyFormat("enum { A, B, C } ShortEnum1, ShortEnum2;", Style); Style.AllowShortEnumsOnASingleLine = false; + verifyFormat("enum {\n" EXPECT_FALSE(Style.BraceWrap

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. You need to have these conversations by adding new unit tests that prove your point, I highly doubt I'll personally be willing to accept any revision without more unit tests than this one line change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D93988: [ASTMatchers] Make tests explicit about mode-dependence

2021-01-03 Thread Stephen Kelly via Phabricator via cfe-commits
steveire created this revision. steveire added a reviewer: aaron.ballman. steveire requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D93988 Files: clang/unittests/ASTMatchers/

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93938#2476432 , @atirit wrote: > I think accepting a revision that includes only a fix for `AfterEnum` being > ignored (not the corner case) and the new unit test would be the best way to > go about this, since the

[PATCH] D93986: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added a comment. Two remarks, first: I made the change only for AlignConsecutiveAssignments, but I think it should be consistent for AlignConsecutiveMacros, AlignConsecutiveBitFields and AlignConsecutiveDeclarations, as well. If this change is accepted, I will make corresponding changes

[PATCH] D93986: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf created this revision. tinloaf added reviewers: MyDeveloperDay, djasper. tinloaf requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently, empty lines and comments break alignment of assignments on consecutive lines. This makes th

[PATCH] D93985: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 314277. tinloaf added a comment. [format] Add the possibility to align assignments spanning empty lines or comments Currently, empty lines and comments break alignment of assignments on consecutive lines. This makes the AlignConsecutiveAssignments option an

[PATCH] D93985: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf updated this revision to Diff 314276. tinloaf added a comment. Fix bogus include of iostream Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93985/new/ https://reviews.llvm.org/D93985 Files: clang/lib/Format/WhitespaceManager.cpp Index:

[PATCH] D93985: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf added a comment. One remark: I made the change only for AlignConsecutiveAssignments, but I think it should be consistent for AlignConsecutiveMacros, AlignConsecutiveBitFields and AlignConsecutiveDeclarations, as well. If this change is accepted, I will make corresponding changes to the

[PATCH] D93985: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits
tinloaf created this revision. tinloaf added reviewers: MyDeveloperDay, djasper. tinloaf requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently, empty lines and comments break alignment of assignments on consecutive lines. This makes th

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit added a comment. I think accepting a revision that includes only a fix for `AfterEnum` being ignored (not the corner case) and the new unit test would be the best way to go about this, since they're separate bugs. Then I can fix the corner case (and compatibility with the new unit test)

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit added a comment. OK, I'm getting a few more failed unit tests and I'm really not sure what correct formatting behaviour is anymore, so I'll just ask. Assuming the following settings: BreakBeforeBraces: Custom BraceWrapping: { AfterEnum: true } AllowShortEnumsOnASingleLine: tr