Phabricator Creator Pulling the Plug

2021-08-18 Thread MyDeveloper Day via cfe-commits 
All

I'm a massive fan of Phabricator, and I know there is often lots of
contentious discussion about its relative merits vs github,

But unless I missed this, was there any discussion regarding the recent
"Winding Down" announcement of Phabricator? and what it might mean for us
in LLVM

See:
https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
https://www.phacility.com/phabricator/

Personally I'm excited by the concept of a community driven replacement (
https://we.phorge.it/) .
epriestley did a truly amazing job, it wasn't open to public contributions.
Perhaps more open development could lead to closing some of the github gaps
that were of concern.

MyDeveloperDay
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2021-03-08 Thread MyDeveloper Day via cfe-commits 
+1 we are not going to land this with a failing or removed test


On Tue, 9 Mar 2021 at 07:29, Marek Kurdej via Phabricator <
revi...@reviews.llvm.org> wrote:

> curdeius added a comment.
>
> In D93938#2612952 ,
> @HazardyKnusperkeks wrote:
>
> > In my opinion you should then, either temporarily deactivate the test,
> or fix the bug first. A failing test blocks the pipeline and confuses
> everyone working on the project.
>
> +1
>
> I got confused about this. I know that there was some discussion about
> this failing test but I thought that the plan was to fix it (as it should).
> Also, that's what one expects in a revision called "Fixed AfterEnum
> handling" :).
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D93938/new/
>
> https://reviews.llvm.org/D93938
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

2021-01-30 Thread MyDeveloper Day via cfe-commits 
I have a script that runs clang-format -n on various directories in clang
that are clang format clean, polly is one of them because they have clang
format as a unit test

I use this to ensure I don’t regress behaviour

Maybe we should formalise this with some sort of clang-format-check cmake
rule

Mydeveloperday

On Sat, 30 Jan 2021 at 10:04, Björn Schäpers via Phabricator <
revi...@reviews.llvm.org> wrote:

> HazardyKnusperkeks added a comment.
>
> In D92257#2532071 , @curdeius
> wrote:
>
> > LGTM. Could you please give us a link to the failing test in Polly? May
> be GitHub or buildbot.
>
> No problem:
>
> http://lab.llvm.org:8011/#builders/10/builds/2294
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D92257/new/
>
> https://reviews.llvm.org/D92257
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-17 Thread MyDeveloper Day via cfe-commits 
We could resolve this using a separate TokenAnalyzer pass similar to what
we are doing here  https://reviews.llvm.org/D69764, such a  would let us
look at the last token on the previous line or the next token in the next
time to ensure we don't remove legal cases like

for(;
;
)

Adding a DoubleSemiFixer type class would allow clang-format to resolve
such issue where it see erroneous double ";;" not just were the
replacements have got confused.

I'm happy to look into that if that would resolve your issues?

On Sat, Nov 16, 2019 at 11:03 PM Jonas Toth via Phabricator via cfe-commits
 wrote:

> JonasToth added a comment.
>
> Hmm. I think this is fine, even though its not perfect.
> @aaron.ballman wdyt?
>
>
> Repository:
>   rCTE Clang Tools Extra
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D70144/new/
>
> https://reviews.llvm.org/D70144
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-12 Thread MyDeveloper Day via cfe-commits 
Thank you for handling that

I will look into your suggestions

MyDeveloperDay

On Sat, 12 Oct 2019 at 23:55, Nico Weber via Phabricator <
revi...@reviews.llvm.org> wrote:

> thakis added a comment.
>
> This fails on macOS:
>
>   : 'RUN: at line 2';   grep -E "*code should be clang-formatted*"
> /Users/thakis/src/llvm-project/out/gn/obj/clang/test/Format/Output/dry-run.cpp.tmp.stderr
>   --
>   Exit Code: 2
>
>   Command Output (stderr):
>   --
>   grep: repetition-operator operand invalid
>
> grep -E means extended regex, where you want `.*` to match anything. But
> this probably should use FileCheck, or maybe even -verify since you're
> adding a dep on Frontend anyways.
>
> 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 impossible to
> ever get the diagnostics tables in clang-format gc'd by the linker)?
>
> It might make sense to instead use LLVM's diag stuff (instead of clang's)
> since all the actual diags will be disjoint.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D68554/new/
>
> https://reviews.llvm.org/D68554
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r372939 - [clang-format] Add SortPriority fields to fix -Wmissing-field-initializers after D64695/r372919

2019-09-25 Thread MyDeveloper Day via cfe-commits 
Thank you for fixing this...

On Thu, Sep 26, 2019 at 3:00 AM Fangrui Song via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: maskray
> Date: Wed Sep 25 19:02:17 2019
> New Revision: 372939
>
> URL: http://llvm.org/viewvc/llvm-project?rev=372939=rev
> Log:
> [clang-format] Add SortPriority fields to fix -Wmissing-field-initializers
> after D64695/r372919
>
> Modified:
> cfe/trunk/lib/Format/Format.cpp
> cfe/trunk/unittests/Format/FormatTest.cpp
>
> Modified: cfe/trunk/lib/Format/Format.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=372939=372938=372939=diff
>
> ==
> --- cfe/trunk/lib/Format/Format.cpp (original)
> +++ cfe/trunk/lib/Format/Format.cpp Wed Sep 25 19:02:17 2019
> @@ -734,9 +734,9 @@ FormatStyle getLLVMStyle(FormatStyle::La
>LLVMStyle.ForEachMacros.push_back("Q_FOREACH");
>LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH");
>LLVMStyle.IncludeStyle.IncludeCategories = {
> -  {"^\"(llvm|llvm-c|clang|clang-c)/", 2},
> -  {"^(<|\"(gtest|gmock|isl|json)/)", 3},
> -  {".*", 1}};
> +  {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 0},
> +  {"^(<|\"(gtest|gmock|isl|json)/)", 3, 0},
> +  {".*", 1, 0}};
>LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
>LLVMStyle.IncludeStyle.IncludeBlocks =
> tooling::IncludeStyle::IBS_Preserve;
>LLVMStyle.IndentCaseLabels = false;
> @@ -818,8 +818,10 @@ FormatStyle getGoogleStyle(FormatStyle::
>GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
>GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
>GoogleStyle.DerivePointerAlignment = true;
> -  GoogleStyle.IncludeStyle.IncludeCategories = {
> -  {"^", 2}, {"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};
> +  GoogleStyle.IncludeStyle.IncludeCategories = {{"^", 2, 0},
> +{"^<.*\\.h>", 1, 0},
> +{"^<.*", 2, 0},
> +{".*", 3, 0}};
>GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$";
>GoogleStyle.IncludeStyle.IncludeBlocks =
> tooling::IncludeStyle::IBS_Regroup;
>GoogleStyle.IndentCaseLabels = true;
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=372939=372938=372939=diff
>
> ==
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Sep 25 19:02:17 2019
> @@ -12463,7 +12463,7 @@ TEST_F(FormatTest, ParsesConfiguration)
>
>Style.IncludeStyle.IncludeCategories.clear();
>std::vector ExpectedCategories
> = {
> -  {"abc/.*", 2}, {".*", 1}};
> +  {"abc/.*", 2, 0}, {".*", 1, 0}};
>CHECK_PARSE("IncludeCategories:\n"
>"  - Regex: abc/.*\n"
>"Priority: 2\n"
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r355182 - [clang-format] [NFC] clang-format the Format library

2019-03-09 Thread MyDeveloper Day via cfe-commits 
Yes ideally I'd agree, It also says:

> Avoid committing formatting- or whitespace-only changes outside of code
you plan to make subsequent changes to.

This was getting to be a problem, every time we edited the file (with
auto-clang-format) a large number of other changes would also get made,
presumably because others hadn't been clang-formatting their commits
correctly! we'd have to merge out a lot of changes everytime.

And then its says this..

>Also, try to separate formatting or whitespace changes from functional
changes, either by correcting the format first (ideally) or afterward

which means you want those changes to be separate commits.. so ideally its
better we clang-format the file when there is no other related changes so
future revisions don't need to have a mixture of real changes and
whitespace changes.

I also think given we also encourage people to clang-format out code,  its
ironic clang-format itself is not clang-formatted!


On Fri, Mar 8, 2019 at 8:59 PM Michael Kruse 
wrote:

> Isn't this kind of commit discouraged?, as by
> https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access
>
> > Avoid committing formatting- or whitespace-only changes outside of code
> you plan to make subsequent changes to.
>
> and the discussion after which this has been added:
> https://lists.llvm.org/pipermail/llvm-dev/2018-July/124941.html
>
>
> Michael
>
> Am Fr., 1. März 2019 um 03:09 Uhr schrieb Paul Hoad via cfe-commits
> :
> >
> > Author: paulhoad
> > Date: Fri Mar  1 01:09:54 2019
> > New Revision: 355182
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=355182=rev
> > Log:
> > [clang-format] [NFC] clang-format the Format library
> >
> > Previously revisions commited non-clang-formatted changes to the Format
> library, this means submitting any revision e.g. {D55170} can cause
> additional whitespace changes to potentially be included in a revision.
> >
> > Commit a non functional change using latest build Windows clang-format
> r351376 with no other changes, to remove these differences
> >
> > All FormatTests
> > pass [==] 652 tests from 20 test cases ran.
> >
> > Modified:
> > cfe/trunk/lib/Format/BreakableToken.cpp
> > cfe/trunk/lib/Format/BreakableToken.h
> > cfe/trunk/lib/Format/ContinuationIndenter.cpp
> > cfe/trunk/lib/Format/Format.cpp
> > cfe/trunk/lib/Format/FormatToken.h
> > cfe/trunk/lib/Format/FormatTokenLexer.h
> > cfe/trunk/lib/Format/TokenAnnotator.cpp
> > cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
> > cfe/trunk/lib/Format/UnwrappedLineFormatter.h
> > cfe/trunk/lib/Format/UnwrappedLineParser.cpp
> > cfe/trunk/lib/Format/UnwrappedLineParser.h
> > cfe/trunk/lib/Format/WhitespaceManager.cpp
> >
> > Modified: cfe/trunk/lib/Format/BreakableToken.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=355182=355181=355182=diff
> >
> ==
> > --- cfe/trunk/lib/Format/BreakableToken.cpp (original)
> > +++ cfe/trunk/lib/Format/BreakableToken.cpp Fri Mar  1 01:09:54 2019
> > @@ -62,12 +62,10 @@ static StringRef getLineCommentIndentPre
> >return LongestPrefix;
> >  }
> >
> > -static BreakableToken::Split getCommentSplit(StringRef Text,
> > - unsigned
> ContentStartColumn,
> > - unsigned ColumnLimit,
> > - unsigned TabWidth,
> > - encoding::Encoding
> Encoding,
> > - const FormatStyle ) {
> > +static BreakableToken::Split
> > +getCommentSplit(StringRef Text, unsigned ContentStartColumn,
> > +unsigned ColumnLimit, unsigned TabWidth,
> > +encoding::Encoding Encoding, const FormatStyle ) {
> >LLVM_DEBUG(llvm::dbgs() << "Comment split: \"" << Text
> ><< "\", Column limit: " << ColumnLimit
> ><< ", Content start: " << ContentStartColumn
> << "\n");
> > @@ -191,7 +189,7 @@ bool switchesFormatting(const FormatToke
> >
> >  unsigned
> >  BreakableToken::getLengthAfterCompression(unsigned
> RemainingTokenColumns,
> > -  Split Split) const {
> > +  Split Split) const {
> >// Example: consider the content
> >// lala  lala
> >// - RemainingTokenColumns is the original number of columns, 10;
> > @@ -870,23 +868,20 @@ void BreakableLineCommentSection::reflow
> >  // the next line.
> >  unsigned WhitespaceLength =
> >  Lines[LineIndex].data() - tokenAt(LineIndex).TokenText.data() -
> Offset;
> > -Whitespaces.replaceWhitespaceInToken(*Tokens[LineIndex],
> > - Offset,
> > +Whitespaces.replaceWhitespaceInToken(*Tokens[LineIndex], Offset,
> >
>