Re: [PATCH] Add clang-format config to contrib folder

[Pedro Alves]

On 11/20/2015 11:33 AM, Martin Liška wrote:

> Hi Pedro.

Hi Martin.

> Fully agree with you, there's suggested patch.
> Hope I can install the patch for trunk?

I'd call it obvious.  :-)

Thanks,
Pedro Alves

Re: [PATCH] Add clang-format config to contrib folder

[Martin Liška]

On 11/19/2015 06:43 PM, Pedro Alves wrote:
> On 11/19/2015 12:54 PM, Martin Liška wrote:
>>  ContinuationIndentWidth: 2
>> -ForEachMacros: 
>> ['_FOR_EACH','_FOR_EACH_1','FOR_EACH_AGGR_INIT_EXPR_ARG','FOR_EACH_ALIAS','FOR_EACH_ALLOCNO','FOR_EACH_ALLOCNO_OBJECT','FOR_EACH_ARTIFICIAL_DEF','FOR_EACH_ARTIFICIAL_USE','FOR_EACH_BB_FN','FOR_EACH_BB_REVERSE_FN','FOR_EACH_BIT_IN_MINMAX_SET','FOR_EACH_CALL_EXPR_ARG','FOR_EACH_CLONE','FOR_EACH_CONST_CALL_EXPR_ARG','FOR_EACH_CONSTRUCTOR_ELT','FOR_EACH_CONSTRUCTOR_VALUE','FOR_EACH_COPY','FOR_EACH_DEF','FOR_EACH_DEFINED_FUNCTION','FOR_EACH_DEFINED_SYMBOL','FOR_EACH_DEFINED_VARIABLE','FOR_EACH_DEP','FOR_EACH_EDGE','FOR_EACH_EXPR','FOR_EACH_EXPR_1','FOR_EACH_FUNCTION','FOR_EACH_FUNCTION_WITH_GIMPLE_BODY','FOR_EACH_HASH_TABLE_ELEMENT','FOR_EACH_IMM_USE_FAST','FOR_EACH_IMM_USE_ON_STMT','FOR_EACH_IMM_USE_STMT','FOR_EACH_INSN','FOR_EACH_INSN_1','FOR_EACH_INSN_DEF','FOR_EACH_INSN_EQ_USE','FOR_EACH_INSN_INFO_DEF','FOR_EACH_INSN_INFO_EQ_USE','FOR_EACH_INSN_INFO_MW','FOR_EACH_INSN_INFO_USE','FOR_EACH_INSN_USE','FOR_EACH_LOCAL_DECL','FOR_EACH_LOOP','FOR_EACH_LOOP_FN','FOR!
>  _EACH_OBJE
> CT','FOR_EACH_OBJECT_CONFLICT','FOR_EACH_PHI_ARG','FOR_EACH_PHI_OR_STMT_DEF','FOR_EACH_PHI_OR_STMT_USE','FOR_EACH_PREF','FOR_EACH_SCALAR','FOR_EACH_SSA_DEF_OPERAND','FOR_EACH_SSA_TREE_OPERAND','FOR_EACH_SSA_USE_OPERAND','FOR_EACH_STATIC_INITIALIZER','FOR_EACH_SUBRTX','FOR_EACH_SUBRTX_PTR','FOR_EACH_SUBRTX_VAR','FOR_EACH_SUCC','FOR_EACH_SUCC_1','FOR_EACH_SYMBOL','FOR_EACH_VARIABLE','FOR_EACH_VEC_ELT','FOR_EACH_VEC_ELT_FROM','FOR_EACH_VEC_ELT_REVERSE','FOR_EACH_VEC_SAFE_ELT','FOR_EACH_VEC_SAFE_ELT_REVERSE','_FOR_EACH_X','_FOR_EACH_X_1','FOREACH_FUNCTION_ARGS','FOREACH_FUNCTION_ARGS_PTR']
>> +ForEachMacros: 
>> ['FOR_ALL_BB_FN','FOR_ALL_EH_REGION','FOR_ALL_EH_REGION_AT','FOR_ALL_EH_REGION_FN','FOR_ALL_INHERITED_FIELDS','FOR_ALL_PREDICATES','FOR_BB_BETWEEN','FOR_BB_INSNS','FOR_BB_INSNS_REVERSE','FOR_BB_INSNS_REVERSE_SAFE','FOR_BB_INSNS_SAFE','FOR_BODY','FOR_COND','FOR_EACH_AGGR_INIT_EXPR_ARG','FOR_EACH_ALIAS','FOR_EACH_ALLOCNO','FOR_EACH_ALLOCNO_OBJECT','FOR_EACH_ARTIFICIAL_DEF','FOR_EACH_ARTIFICIAL_USE','FOR_EACH_BB_FN','FOR_EACH_BB_REVERSE_FN','FOR_EACH_BIT_IN_MINMAX_SET','FOR_EACH_CALL_EXPR_ARG','FOR_EACH_CLONE','FOR_EACH_CONST_CALL_EXPR_ARG','FOR_EACH_CONSTRUCTOR_ELT','FOR_EACH_CONSTRUCTOR_VALUE','FOR_EACH_COPY','FOR_EACH_DEF','FOR_EACH_DEFINED_FUNCTION','FOR_EACH_DEFINED_SYMBOL','FOR_EACH_DEFINED_VARIABLE','FOR_EACH_DEP','FOR_EACH_EDGE','FOR_EACH_EXPR','FOR_EACH_EXPR_1','FOR_EACH_FUNCTION','FOREACH_FUNCTION_ARGS','FOREACH_FUNCTION_ARGS_PTR','FOR_EACH_FUNCTION_WITH_GIMPLE_BODY','FOR_EACH_HASH_TABLE_ELEMENT','FOR_EACH_IMM_USE_FAST','FOR_EACH_IMM_USE_ON_STMT','FO!
>  R_EACH_IMM
> _USE_STMT','FOR_EACH_INSN','FOR_EACH_INSN_1','FOR_EACH_INSN_DEF','FOR_EACH_INSN_EQ_USE','FOR_EACH_INSN_INFO_DEF','FOR_EACH_INSN_INFO_EQ_USE','FOR_EACH_INSN_INFO_MW','FOR_EACH_INSN_INFO_USE','FOR_EACH_INSN_USE','FOR_EACH_LOCAL_DECL','FOR_EACH_LOOP','FOR_EACH_LOOP_FN','FOR_EACH_OBJECT','FOR_EACH_OBJECT_CONFLICT','FOR_EACH_PHI_ARG','FOR_EACH_PHI_OR_STMT_DEF','FOR_EACH_PHI_OR_STMT_USE','FOR_EACH_PREF','FOR_EACH_SCALAR','FOR_EACH_SSA_DEF_OPERAND','FOR_EACH_SSA_TREE_OPERAND','FOR_EACH_SSA_USE_OPERAND','FOR_EACH_STATIC_INITIALIZER','FOR_EACH_SUBRTX','FOR_EACH_SUBRTX_PTR','FOR_EACH_SUBRTX_VAR','FOR_EACH_SUCC','FOR_EACH_SUCC_1','FOR_EACH_SYMBOL','FOR_EACH_VARIABLE','FOR_EACH_VEC_ELT','FOR_EACH_VEC_ELT_FROM','FOR_EACH_VEC_ELT_REVERSE','FOR_EACH_VEC_SAFE_ELT','FOR_EACH_VEC_SAFE_ELT_REVERSE','FOR_EXPR','FOR_INIT_STMT','FOR_SCOPE']
>>  IndentCaseLabels: false
> 
> I don't know the tool's syntax here, but it's usually much better to
> keep entries like these one per line (or at least a few only).  Then
> changes to the list are _much_ easier to review and maintain, like:
> 
> ...
>  ForEachMacros: [
>  'FOR_ALL_BB_FN',
>  'FOR_ALL_EH_REGION',
> -'FOR_ALL_EH_REGION_AT',
> -'FOR_ALL_EH_REGION',
> +'FOR_ALL_EH_WHATNOT,
> +'FOR_ALL_EH_WHATNOT_AT,
>  'FOR_ALL_INHERITED_FIELDS',
>  'FOR_ALL_PREDICATES',
>  'FOR_BB_BETWEEN',
>  'FOR_BB_INSNS',
> ...
> 
> vs a single-line hunk that's basically unintelligible.
> 
> Thanks,
> Pedro Alves
> 

Hi Pedro.

Fully agree with you, there's suggested patch.
Hope I can install the patch for trunk?

Thanks,
Martin
From eac730138b51db897f579a961aabaee805338bdd Mon Sep 17 00:00:00 2001
From: marxin 
Date: Fri, 20 Nov 2015 12:29:49 +0100
Subject: [PATCH] clang-format: split content of a list to multiple lines

contrib/ChangeLog:

2015-11-20  Martin Liska  

	* clang-format: Split content of a list to multiple
	lines.
---
 contrib/clang-format | 85 +++-
 1 file changed, 84 insertions(+), 1 deletion(-)

diff --git a/contrib/clang-format b/contrib/clang-format
index 76a9d87..d734001 100644
--- a/contrib/clang-format
+++ b/contrib/clang-format
@@ -43,7 +43,90 @@ BreakBeforeTernaryOperators: true
 ColumnLimit: 80
 

Re: [PATCH] Add clang-format config to contrib folder

[Jeff Law]

On 11/20/2015 04:33 AM, Martin Liška wrote:

On 11/19/2015 06:43 PM, Pedro Alves wrote:

On 11/19/2015 12:54 PM, Martin Liška wrote:

  ContinuationIndentWidth: 2
-ForEachMacros: 
['_FOR_EACH','_FOR_EACH_1','FOR_EACH_AGGR_INIT_EXPR_ARG','FOR_EACH_ALIAS','FOR_EACH_ALLOCNO','FOR_EACH_ALLOCNO_OBJECT','FOR_EACH_ARTIFICIAL_DEF','FOR_EACH_ARTIFICIAL_USE','FOR_EACH_BB_FN','FOR_EACH_BB_REVERSE_FN','FOR_EACH_BIT_IN_MINMAX_SET','FOR_EACH_CALL_EXPR_ARG','FOR_EACH_CLONE','FOR_EACH_CONST_CALL_EXPR_ARG','FOR_EACH_CONSTRUCTOR_ELT','FOR_EACH_CONSTRUCTOR_VALUE','FOR_EACH_COPY','FOR_EACH_DEF','FOR_EACH_DEFINED_FUNCTION','FOR_EACH_DEFINED_SYMBOL','FOR_EACH_DEFINED_VARIABLE','FOR_EACH_DEP','FOR_EACH_EDGE','FOR_EACH_EXPR','FOR_EACH_EXPR_1','FOR_EACH_FUNCTION','FOR_EACH_FUNCTION_WITH_GIMPLE_BODY','FOR_EACH_HASH_TABLE_ELEMENT','FOR_EACH_IMM_USE_FAST','FOR_EACH_IMM_USE_ON_STMT','FOR_EACH_IMM_USE_STMT','FOR_EACH_INSN','FOR_EACH_INSN_1','FOR_EACH_INSN_DEF','FOR_EACH_INSN_EQ_USE','FOR_EACH_INSN_INFO_DEF','FOR_EACH_INSN_INFO_EQ_USE','FOR_EACH_INSN_INFO_MW','FOR_EACH_INSN_INFO_USE','FOR_EACH_INSN_USE','FOR_EACH_LOCAL_DECL','FOR_EACH_LOOP','FOR_EACH_LOOP_FN','F!

O!

  R!

  _EACH_OBJE
CT','FOR_EACH_OBJECT_CONFLICT','FOR_EACH_PHI_ARG','FOR_EACH_PHI_OR_STMT_DEF','FOR_EACH_PHI_OR_STMT_USE','FOR_EACH_PREF','FOR_EACH_SCALAR','FOR_EACH_SSA_DEF_OPERAND','FOR_EACH_SSA_TREE_OPERAND','FOR_EACH_SSA_USE_OPERAND','FOR_EACH_STATIC_INITIALIZER','FOR_EACH_SUBRTX','FOR_EACH_SUBRTX_PTR','FOR_EACH_SUBRTX_VAR','FOR_EACH_SUCC','FOR_EACH_SUCC_1','FOR_EACH_SYMBOL','FOR_EACH_VARIABLE','FOR_EACH_VEC_ELT','FOR_EACH_VEC_ELT_FROM','FOR_EACH_VEC_ELT_REVERSE','FOR_EACH_VEC_SAFE_ELT','FOR_EACH_VEC_SAFE_ELT_REVERSE','_FOR_EACH_X','_FOR_EACH_X_1','FOREACH_FUNCTION_ARGS','FOREACH_FUNCTION_ARGS_PTR']

+ForEachMacros: 
['FOR_ALL_BB_FN','FOR_ALL_EH_REGION','FOR_ALL_EH_REGION_AT','FOR_ALL_EH_REGION_FN','FOR_ALL_INHERITED_FIELDS','FOR_ALL_PREDICATES','FOR_BB_BETWEEN','FOR_BB_INSNS','FOR_BB_INSNS_REVERSE','FOR_BB_INSNS_REVERSE_SAFE','FOR_BB_INSNS_SAFE','FOR_BODY','FOR_COND','FOR_EACH_AGGR_INIT_EXPR_ARG','FOR_EACH_ALIAS','FOR_EACH_ALLOCNO','FOR_EACH_ALLOCNO_OBJECT','FOR_EACH_ARTIFICIAL_DEF','FOR_EACH_ARTIFICIAL_USE','FOR_EACH_BB_FN','FOR_EACH_BB_REVERSE_FN','FOR_EACH_BIT_IN_MINMAX_SET','FOR_EACH_CALL_EXPR_ARG','FOR_EACH_CLONE','FOR_EACH_CONST_CALL_EXPR_ARG','FOR_EACH_CONSTRUCTOR_ELT','FOR_EACH_CONSTRUCTOR_VALUE','FOR_EACH_COPY','FOR_EACH_DEF','FOR_EACH_DEFINED_FUNCTION','FOR_EACH_DEFINED_SYMBOL','FOR_EACH_DEFINED_VARIABLE','FOR_EACH_DEP','FOR_EACH_EDGE','FOR_EACH_EXPR','FOR_EACH_EXPR_1','FOR_EACH_FUNCTION','FOREACH_FUNCTION_ARGS','FOREACH_FUNCTION_ARGS_PTR','FOR_EACH_FUNCTION_WITH_GIMPLE_BODY','FOR_EACH_HASH_TABLE_ELEMENT','FOR_EACH_IMM_USE_FAST','FOR_EACH_IMM_USE_ON_STMT','!

F!

  O!

  R_EACH_IMM
_USE_STMT','FOR_EACH_INSN','FOR_EACH_INSN_1','FOR_EACH_INSN_DEF','FOR_EACH_INSN_EQ_USE','FOR_EACH_INSN_INFO_DEF','FOR_EACH_INSN_INFO_EQ_USE','FOR_EACH_INSN_INFO_MW','FOR_EACH_INSN_INFO_USE','FOR_EACH_INSN_USE','FOR_EACH_LOCAL_DECL','FOR_EACH_LOOP','FOR_EACH_LOOP_FN','FOR_EACH_OBJECT','FOR_EACH_OBJECT_CONFLICT','FOR_EACH_PHI_ARG','FOR_EACH_PHI_OR_STMT_DEF','FOR_EACH_PHI_OR_STMT_USE','FOR_EACH_PREF','FOR_EACH_SCALAR','FOR_EACH_SSA_DEF_OPERAND','FOR_EACH_SSA_TREE_OPERAND','FOR_EACH_SSA_USE_OPERAND','FOR_EACH_STATIC_INITIALIZER','FOR_EACH_SUBRTX','FOR_EACH_SUBRTX_PTR','FOR_EACH_SUBRTX_VAR','FOR_EACH_SUCC','FOR_EACH_SUCC_1','FOR_EACH_SYMBOL','FOR_EACH_VARIABLE','FOR_EACH_VEC_ELT','FOR_EACH_VEC_ELT_FROM','FOR_EACH_VEC_ELT_REVERSE','FOR_EACH_VEC_SAFE_ELT','FOR_EACH_VEC_SAFE_ELT_REVERSE','FOR_EXPR','FOR_INIT_STMT','FOR_SCOPE']

  IndentCaseLabels: false


I don't know the tool's syntax here, but it's usually much better to
keep entries like these one per line (or at least a few only).  Then
changes to the list are _much_ easier to review and maintain, like:

...
  ForEachMacros: [
  'FOR_ALL_BB_FN',
  'FOR_ALL_EH_REGION',
-'FOR_ALL_EH_REGION_AT',
-'FOR_ALL_EH_REGION',
+'FOR_ALL_EH_WHATNOT,
+'FOR_ALL_EH_WHATNOT_AT,
  'FOR_ALL_INHERITED_FIELDS',
  'FOR_ALL_PREDICATES',
  'FOR_BB_BETWEEN',
  'FOR_BB_INSNS',
...

vs a single-line hunk that's basically unintelligible.

Thanks,
Pedro Alves



Hi Pedro.

Fully agree with you, there's suggested patch.
Hope I can install the patch for trunk?
Yes.  In fact, I think that as long as you're moving towards coercing 
clang-format to handle GNU style that you should have a free reign to 
make changes to this config file.


jeff

Re: [PATCH] Add clang-format config to contrib folder

[Martin Liška]

On 11/19/2015 12:09 AM, Sebastian Pop wrote:

Martin, thanks for getting this patch out.  I like the patch.
Jeff, clang-format has scripts that allow formatting only the lines
touched by a patch.
It also has a script to integrate with git:
https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/git-clang-format
We could use those scripts in a commit hook to automatically enforce
correct formatting of new patches.

Sebastian


Hi.

Thanks for pointing out the script touching just modified lines,
I mentioned the script in the clang-format file.

Sending v2 that I'm going to install.

Thanks,
Martin



On Wed, Nov 18, 2015 at 8:10 AM, Martin Liška  wrote:

Hello.

Following patch adds a clang-format config file that should respect the GNU 
coding standards.
As the file is not part of build process, I hope the patch can be applied even 
though
we've just skipped to stage3? The patch adds a hunk to Makefile which can 
create symlink
to the root directory of the GCC compiler. The clang-format automatically loads 
style from
the configuration file.

clang-format (version 3.8) provides rich variety of configuration options that 
can
ensure the GNU coding style.

Limitations:
+ placement of opening brace of an initializer can't be requested
+ sometimes, '(' is the trailing symbol at the end of a line, which can look 
weird

As we've been continuously converting our source base to C++, the clang-format 
should
provide better results than a collection of regular expressions 
(check_GNU_style.sh).

As a reference file I attach gcc/tree-ssa-uninit.c file.
Feel free to comment the suggested configuration file.

Thanks,
Martin



From c344396c77ce462b576204697ccd48f1eaa17c7b Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 18 Nov 2015 14:40:58 +0100
Subject: [PATCH] Add clang-format config to contrib folder

ChangeLog:

2015-11-18  Martin Liska  

	* Makefile.in: Add clang-format.
	* Makefile.tpl: Likewise.

contrib/ChangeLog:

2015-11-18  Martin Liska  

	* clang-format: New file.
---
 Makefile.in  |  9 +
 Makefile.tpl |  9 +
 contrib/clang-format | 55 
 3 files changed, 73 insertions(+)
 create mode 100644 contrib/clang-format

diff --git a/Makefile.in b/Makefile.in
index bc2bae6..75caafd 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -2461,6 +2461,15 @@ vimrc: $(srcdir)/.local.vimrc $(srcdir)/.lvimrc
 
 .PHONY: vimrc
 
+# clang-format config
+
+$(srcdir)/.clang-format:
+	$(LN_S) contrib/clang-format $@
+
+clang-format: $(srcdir)/.clang-format
+
+.PHONY: clang-format
+
 # Installation targets.
 
 .PHONY: install uninstall
diff --git a/Makefile.tpl b/Makefile.tpl
index 660e1e4..beabadc 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -889,6 +889,15 @@ vimrc: $(srcdir)/.local.vimrc $(srcdir)/.lvimrc
 
 .PHONY: vimrc
 
+# clang-format config
+
+$(srcdir)/.clang-format:
+	$(LN_S) contrib/clang-format $@
+
+clang-format: $(srcdir)/.clang-format
+
+.PHONY: clang-format
+
 # Installation targets.
 
 .PHONY: install uninstall
diff --git a/contrib/clang-format b/contrib/clang-format
new file mode 100644
index 000..8a6fd4b
--- /dev/null
+++ b/contrib/clang-format
@@ -0,0 +1,55 @@
+# Copyright (C) 2015 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+# clang-format 3.8+ (Mon Nov 16) is required
+#
+# To utilize the tool to lines just touched by a patch, use
+# clang-format-diff.py script, which can be downloaded here:
+# https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/clang-format-diff.py
+
+---
+Language: Cpp
+AccessModifierOffset: -2
+AlwaysBreakAfterDefinitionReturnType: All
+BinPackArguments: true
+BinPackParameters: true
+BraceWrapping:
+  AfterClass: true
+  AfterControlStatement: true
+  AfterEnum: true
+  AfterFunction: true
+  AfterNamespace: false
+  AfterObjCDeclaration: true
+  AfterStruct: true
+  AfterUnion: true
+  BeforeCatch: true
+  BeforeElse: true
+  IndentBraces: true
+BreakBeforeBinaryOperators: All
+BreakBeforeBraces: Custom
+BreakBeforeTernaryOperators: true
+ColumnLimit: 80
+ConstructorInitializerIndentWidth: 2
+ContinuationIndentWidth: 2
+ForEachMacros: 

Re: [PATCH] Add clang-format config to contrib folder

[Martin Liška]

On 11/19/2015 11:40 AM, Martin Liška wrote:
> On 11/19/2015 12:09 AM, Sebastian Pop wrote:
>> Martin, thanks for getting this patch out.  I like the patch.
>> Jeff, clang-format has scripts that allow formatting only the lines
>> touched by a patch.
>> It also has a script to integrate with git:
>> https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/git-clang-format
>> We could use those scripts in a commit hook to automatically enforce
>> correct formatting of new patches.
>>
>> Sebastian
> 
> Hi.
> 
> Thanks for pointing out the script touching just modified lines,
> I mentioned the script in the clang-format file.
> 
> Sending v2 that I'm going to install.
> 
> Thanks,
> Martin
> 
>>
>> On Wed, Nov 18, 2015 at 8:10 AM, Martin Liška  wrote:
>>> Hello.
>>>
>>> Following patch adds a clang-format config file that should respect the GNU 
>>> coding standards.
>>> As the file is not part of build process, I hope the patch can be applied 
>>> even though
>>> we've just skipped to stage3? The patch adds a hunk to Makefile which can 
>>> create symlink
>>> to the root directory of the GCC compiler. The clang-format automatically 
>>> loads style from
>>> the configuration file.
>>>
>>> clang-format (version 3.8) provides rich variety of configuration options 
>>> that can
>>> ensure the GNU coding style.
>>>
>>> Limitations:
>>> + placement of opening brace of an initializer can't be requested
>>> + sometimes, '(' is the trailing symbol at the end of a line, which can 
>>> look weird
>>>
>>> As we've been continuously converting our source base to C++, the 
>>> clang-format should
>>> provide better results than a collection of regular expressions 
>>> (check_GNU_style.sh).
>>>
>>> As a reference file I attach gcc/tree-ssa-uninit.c file.
>>> Feel free to comment the suggested configuration file.
>>>
>>> Thanks,
>>> Martin
>>>
> 

There's a small follow-up where I enhance list of FOR_EACH macros as spotted
by octoploid.

As the change is obvious in a new file, I'm going to install the patch.

Thanks,
Martin
From f5571ac8379a376056aa5dc4846d3adb2d1db7b8 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 19 Nov 2015 13:48:47 +0100
Subject: [PATCH] clang-format: Enhance list of FOR_EACH macros

contrib/ChangeLog:

2015-11-19  Martin Liska  

	* clang-format: Enhance list of FOR_EACH macros.
---
 contrib/clang-format | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/clang-format b/contrib/clang-format
index 8a6fd4b..76a9d87 100644
--- a/contrib/clang-format
+++ b/contrib/clang-format
@@ -43,7 +43,7 @@ BreakBeforeTernaryOperators: true
 ColumnLimit: 80
 ConstructorInitializerIndentWidth: 2
 ContinuationIndentWidth: 2
-ForEachMacros: ['_FOR_EACH','_FOR_EACH_1','FOR_EACH_AGGR_INIT_EXPR_ARG','FOR_EACH_ALIAS','FOR_EACH_ALLOCNO','FOR_EACH_ALLOCNO_OBJECT','FOR_EACH_ARTIFICIAL_DEF','FOR_EACH_ARTIFICIAL_USE','FOR_EACH_BB_FN','FOR_EACH_BB_REVERSE_FN','FOR_EACH_BIT_IN_MINMAX_SET','FOR_EACH_CALL_EXPR_ARG','FOR_EACH_CLONE','FOR_EACH_CONST_CALL_EXPR_ARG','FOR_EACH_CONSTRUCTOR_ELT','FOR_EACH_CONSTRUCTOR_VALUE','FOR_EACH_COPY','FOR_EACH_DEF','FOR_EACH_DEFINED_FUNCTION','FOR_EACH_DEFINED_SYMBOL','FOR_EACH_DEFINED_VARIABLE','FOR_EACH_DEP','FOR_EACH_EDGE','FOR_EACH_EXPR','FOR_EACH_EXPR_1','FOR_EACH_FUNCTION','FOR_EACH_FUNCTION_WITH_GIMPLE_BODY','FOR_EACH_HASH_TABLE_ELEMENT','FOR_EACH_IMM_USE_FAST','FOR_EACH_IMM_USE_ON_STMT','FOR_EACH_IMM_USE_STMT','FOR_EACH_INSN','FOR_EACH_INSN_1','FOR_EACH_INSN_DEF','FOR_EACH_INSN_EQ_USE','FOR_EACH_INSN_INFO_DEF','FOR_EACH_INSN_INFO_EQ_USE','FOR_EACH_INSN_INFO_MW','FOR_EACH_INSN_INFO_USE','FOR_EACH_INSN_USE','FOR_EACH_LOCAL_DECL','FOR_EACH_LOOP','FOR_EACH_LOOP_FN','FOR_EACH_OBJECT','FOR_EACH_OBJECT_CONFLICT','FOR_EACH_PHI_ARG','FOR_EACH_PHI_OR_STMT_DEF','FOR_EACH_PHI_OR_STMT_USE','FOR_EACH_PREF','FOR_EACH_SCALAR','FOR_EACH_SSA_DEF_OPERAND','FOR_EACH_SSA_TREE_OPERAND','FOR_EACH_SSA_USE_OPERAND','FOR_EACH_STATIC_INITIALIZER','FOR_EACH_SUBRTX','FOR_EACH_SUBRTX_PTR','FOR_EACH_SUBRTX_VAR','FOR_EACH_SUCC','FOR_EACH_SUCC_1','FOR_EACH_SYMBOL','FOR_EACH_VARIABLE','FOR_EACH_VEC_ELT','FOR_EACH_VEC_ELT_FROM','FOR_EACH_VEC_ELT_REVERSE','FOR_EACH_VEC_SAFE_ELT','FOR_EACH_VEC_SAFE_ELT_REVERSE','_FOR_EACH_X','_FOR_EACH_X_1','FOREACH_FUNCTION_ARGS','FOREACH_FUNCTION_ARGS_PTR']
+ForEachMacros: 

Re: [PATCH] Add clang-format config to contrib folder

[Hans-Peter Nilsson]

On Wed, 18 Nov 2015, Jeff Law wrote:
> Given that gnu-indent seems to muck up C++ badly in my
> experience, clang-format may be a better long term solution.  I'd really like
> to get to a point one day where formatting is a commit hook so that things are
> always kept properly formatted.

I hope you don't mean the direct interpretation; that the result
of indent (any indent program) should be blindly committed, but
that it should (perhaps) be used to (re)format and the commit
refused if there's a diff.  No tool should be trusted to
override the committers' edits.  Really.

brgds, H-P

Re: [PATCH] Add clang-format config to contrib folder

[Pedro Alves]

On 11/19/2015 12:54 PM, Martin Liška wrote:
>  ContinuationIndentWidth: 2
> -ForEachMacros: 
> ['_FOR_EACH','_FOR_EACH_1','FOR_EACH_AGGR_INIT_EXPR_ARG','FOR_EACH_ALIAS','FOR_EACH_ALLOCNO','FOR_EACH_ALLOCNO_OBJECT','FOR_EACH_ARTIFICIAL_DEF','FOR_EACH_ARTIFICIAL_USE','FOR_EACH_BB_FN','FOR_EACH_BB_REVERSE_FN','FOR_EACH_BIT_IN_MINMAX_SET','FOR_EACH_CALL_EXPR_ARG','FOR_EACH_CLONE','FOR_EACH_CONST_CALL_EXPR_ARG','FOR_EACH_CONSTRUCTOR_ELT','FOR_EACH_CONSTRUCTOR_VALUE','FOR_EACH_COPY','FOR_EACH_DEF','FOR_EACH_DEFINED_FUNCTION','FOR_EACH_DEFINED_SYMBOL','FOR_EACH_DEFINED_VARIABLE','FOR_EACH_DEP','FOR_EACH_EDGE','FOR_EACH_EXPR','FOR_EACH_EXPR_1','FOR_EACH_FUNCTION','FOR_EACH_FUNCTION_WITH_GIMPLE_BODY','FOR_EACH_HASH_TABLE_ELEMENT','FOR_EACH_IMM_USE_FAST','FOR_EACH_IMM_USE_ON_STMT','FOR_EACH_IMM_USE_STMT','FOR_EACH_INSN','FOR_EACH_INSN_1','FOR_EACH_INSN_DEF','FOR_EACH_INSN_EQ_USE','FOR_EACH_INSN_INFO_DEF','FOR_EACH_INSN_INFO_EQ_USE','FOR_EACH_INSN_INFO_MW','FOR_EACH_INSN_INFO_USE','FOR_EACH_INSN_USE','FOR_EACH_LOCAL_DECL','FOR_EACH_LOOP','FOR_EACH_LOOP_FN','FOR!
 _EACH_OBJE
CT','FOR_EACH_OBJECT_CONFLICT','FOR_EACH_PHI_ARG','FOR_EACH_PHI_OR_STMT_DEF','FOR_EACH_PHI_OR_STMT_USE','FOR_EACH_PREF','FOR_EACH_SCALAR','FOR_EACH_SSA_DEF_OPERAND','FOR_EACH_SSA_TREE_OPERAND','FOR_EACH_SSA_USE_OPERAND','FOR_EACH_STATIC_INITIALIZER','FOR_EACH_SUBRTX','FOR_EACH_SUBRTX_PTR','FOR_EACH_SUBRTX_VAR','FOR_EACH_SUCC','FOR_EACH_SUCC_1','FOR_EACH_SYMBOL','FOR_EACH_VARIABLE','FOR_EACH_VEC_ELT','FOR_EACH_VEC_ELT_FROM','FOR_EACH_VEC_ELT_REVERSE','FOR_EACH_VEC_SAFE_ELT','FOR_EACH_VEC_SAFE_ELT_REVERSE','_FOR_EACH_X','_FOR_EACH_X_1','FOREACH_FUNCTION_ARGS','FOREACH_FUNCTION_ARGS_PTR']
> +ForEachMacros: 
> ['FOR_ALL_BB_FN','FOR_ALL_EH_REGION','FOR_ALL_EH_REGION_AT','FOR_ALL_EH_REGION_FN','FOR_ALL_INHERITED_FIELDS','FOR_ALL_PREDICATES','FOR_BB_BETWEEN','FOR_BB_INSNS','FOR_BB_INSNS_REVERSE','FOR_BB_INSNS_REVERSE_SAFE','FOR_BB_INSNS_SAFE','FOR_BODY','FOR_COND','FOR_EACH_AGGR_INIT_EXPR_ARG','FOR_EACH_ALIAS','FOR_EACH_ALLOCNO','FOR_EACH_ALLOCNO_OBJECT','FOR_EACH_ARTIFICIAL_DEF','FOR_EACH_ARTIFICIAL_USE','FOR_EACH_BB_FN','FOR_EACH_BB_REVERSE_FN','FOR_EACH_BIT_IN_MINMAX_SET','FOR_EACH_CALL_EXPR_ARG','FOR_EACH_CLONE','FOR_EACH_CONST_CALL_EXPR_ARG','FOR_EACH_CONSTRUCTOR_ELT','FOR_EACH_CONSTRUCTOR_VALUE','FOR_EACH_COPY','FOR_EACH_DEF','FOR_EACH_DEFINED_FUNCTION','FOR_EACH_DEFINED_SYMBOL','FOR_EACH_DEFINED_VARIABLE','FOR_EACH_DEP','FOR_EACH_EDGE','FOR_EACH_EXPR','FOR_EACH_EXPR_1','FOR_EACH_FUNCTION','FOREACH_FUNCTION_ARGS','FOREACH_FUNCTION_ARGS_PTR','FOR_EACH_FUNCTION_WITH_GIMPLE_BODY','FOR_EACH_HASH_TABLE_ELEMENT','FOR_EACH_IMM_USE_FAST','FOR_EACH_IMM_USE_ON_STMT','FO!
 R_EACH_IMM
_USE_STMT','FOR_EACH_INSN','FOR_EACH_INSN_1','FOR_EACH_INSN_DEF','FOR_EACH_INSN_EQ_USE','FOR_EACH_INSN_INFO_DEF','FOR_EACH_INSN_INFO_EQ_USE','FOR_EACH_INSN_INFO_MW','FOR_EACH_INSN_INFO_USE','FOR_EACH_INSN_USE','FOR_EACH_LOCAL_DECL','FOR_EACH_LOOP','FOR_EACH_LOOP_FN','FOR_EACH_OBJECT','FOR_EACH_OBJECT_CONFLICT','FOR_EACH_PHI_ARG','FOR_EACH_PHI_OR_STMT_DEF','FOR_EACH_PHI_OR_STMT_USE','FOR_EACH_PREF','FOR_EACH_SCALAR','FOR_EACH_SSA_DEF_OPERAND','FOR_EACH_SSA_TREE_OPERAND','FOR_EACH_SSA_USE_OPERAND','FOR_EACH_STATIC_INITIALIZER','FOR_EACH_SUBRTX','FOR_EACH_SUBRTX_PTR','FOR_EACH_SUBRTX_VAR','FOR_EACH_SUCC','FOR_EACH_SUCC_1','FOR_EACH_SYMBOL','FOR_EACH_VARIABLE','FOR_EACH_VEC_ELT','FOR_EACH_VEC_ELT_FROM','FOR_EACH_VEC_ELT_REVERSE','FOR_EACH_VEC_SAFE_ELT','FOR_EACH_VEC_SAFE_ELT_REVERSE','FOR_EXPR','FOR_INIT_STMT','FOR_SCOPE']
>  IndentCaseLabels: false

I don't know the tool's syntax here, but it's usually much better to
keep entries like these one per line (or at least a few only).  Then
changes to the list are _much_ easier to review and maintain, like:

...
 ForEachMacros: [
 'FOR_ALL_BB_FN',
 'FOR_ALL_EH_REGION',
-'FOR_ALL_EH_REGION_AT',
-'FOR_ALL_EH_REGION',
+'FOR_ALL_EH_WHATNOT,
+'FOR_ALL_EH_WHATNOT_AT,
 'FOR_ALL_INHERITED_FIELDS',
 'FOR_ALL_PREDICATES',
 'FOR_BB_BETWEEN',
 'FOR_BB_INSNS',
...

vs a single-line hunk that's basically unintelligible.

Thanks,
Pedro Alves

Re: [PATCH] Add clang-format config to contrib folder

[Jeff Law]

On 11/18/2015 10:34 AM, Manuel López-Ibáñez wrote:


Which is a sad demonstration of how the refusal of GCC's FEs being
re-used for other purposes by GNU tools (and others) was and is a
mistake, and it is leading to GNU tools being replaced by LLVM-based
ones (ultimately affecting GCC and GDB themselves).
I think most developers would agree at this point.  Some may have always 
agreed.  But it was the decision of the FSF long ago to design GCC in 
this way and anyone working on GCC had to abide by that decision.


Things are changing, but undoing that fundamental design decision is a 
*lot* of work.   Don't expect it to land anytime soon.



jeff

Re: [PATCH] Add clang-format config to contrib folder

[Jeff Law]

On 11/18/2015 07:10 AM, Martin Liška wrote:

Hello.

Following patch adds a clang-format config file that should respect the GNU 
coding standards.
As the file is not part of build process, I hope the patch can be applied even 
though
we've just skipped to stage3? The patch adds a hunk to Makefile which can 
create symlink
to the root directory of the GCC compiler. The clang-format automatically loads 
style from
the configuration file.

clang-format (version 3.8) provides rich variety of configuration options that 
can
ensure the GNU coding style.

Limitations:
+ placement of opening brace of an initializer can't be requested
+ sometimes, '(' is the trailing symbol at the end of a line, which can look 
weird

As we've been continuously converting our source base to C++, the clang-format 
should
provide better results than a collection of regular expressions 
(check_GNU_style.sh).

As a reference file I attach gcc/tree-ssa-uninit.c file.
Feel free to comment the suggested configuration file.
This is fine.  Given that gnu-indent seems to muck up C++ badly in my 
experience, clang-format may be a better long term solution.  I'd really 
like to get to a point one day where formatting is a commit hook so that 
things are always kept properly formatted.


jeff

Re: [PATCH] Add clang-format config to contrib folder

[Manuel López-Ibáñez]

On 18/11/15 17:05, Jeff Law wrote:

As we've been continuously converting our source base to C++, the
clang-format should
provide better results than a collection of regular expressions
(check_GNU_style.sh).

As a reference file I attach gcc/tree-ssa-uninit.c file.
Feel free to comment the suggested configuration file.

This is fine.  Given that gnu-indent seems to muck up C++ badly in my
experience, clang-format may be a better long term solution.  I'd really like
to get to a point one day where formatting is a commit hook so that things are
always kept properly formatted.


Which is a sad demonstration of how the refusal of GCC's FEs being re-used for 
other purposes by GNU tools (and others) was and is a mistake, and it is 
leading to GNU tools being replaced by LLVM-based ones (ultimately affecting 
GCC and GDB themselves).


Cheers,

Manuel.

Re: [PATCH] Add clang-format config to contrib folder

[Markus Trippelsdorf]

On 2015.11.18 at 15:37 +0100, Markus Trippelsdorf wrote:
> On 2015.11.18 at 15:10 +0100, Martin Liška wrote:
> > Hello.
> > 
> > Following patch adds a clang-format config file that should respect the GNU 
> > coding standards.
> > As the file is not part of build process, I hope the patch can be applied 
> > even though
> > we've just skipped to stage3? The patch adds a hunk to Makefile which can 
> > create symlink
> > to the root directory of the GCC compiler. The clang-format automatically 
> > loads style from
> > the configuration file. 
> > 
> > clang-format (version 3.8) provides rich variety of configuration options 
> > that can
> > ensure the GNU coding style.
> > 
> > Limitations:
> > + placement of opening brace of an initializer can't be requested
> > + sometimes, '(' is the trailing symbol at the end of a line, which can 
> > look weird
> > 
> > As we've been continuously converting our source base to C++, the 
> > clang-format should
> > provide better results than a collection of regular expressions 
> > (check_GNU_style.sh).
> > 
> > As a reference file I attach gcc/tree-ssa-uninit.c file.
> > Feel free to comment the suggested configuration file.
> 
> Thanks for doing this. It works fine except one problem: You should
> delete "SortIncludes: false" from the contrib/clang-format file, because
> it is the default for custom styles and doesn't work in .clang-format
> (it is a command line option instead):
> 
> markus@x4 gcc % clang-format -style=file gcc-main.c
> YAML:47:15: error: unknown key 'SortIncludes'

Ah, the option was added only yesterday to clang-format! 
My version wasn't new enough...

-- 
Markus

Re: [PATCH] Add clang-format config to contrib folder

[Markus Trippelsdorf]

On 2015.11.18 at 15:10 +0100, Martin Liška wrote:
> Hello.
> 
> Following patch adds a clang-format config file that should respect the GNU 
> coding standards.
> As the file is not part of build process, I hope the patch can be applied 
> even though
> we've just skipped to stage3? The patch adds a hunk to Makefile which can 
> create symlink
> to the root directory of the GCC compiler. The clang-format automatically 
> loads style from
> the configuration file. 
> 
> clang-format (version 3.8) provides rich variety of configuration options 
> that can
> ensure the GNU coding style.
> 
> Limitations:
> + placement of opening brace of an initializer can't be requested
> + sometimes, '(' is the trailing symbol at the end of a line, which can look 
> weird
> 
> As we've been continuously converting our source base to C++, the 
> clang-format should
> provide better results than a collection of regular expressions 
> (check_GNU_style.sh).
> 
> As a reference file I attach gcc/tree-ssa-uninit.c file.
> Feel free to comment the suggested configuration file.

Thanks for doing this. It works fine except one problem: You should
delete "SortIncludes: false" from the contrib/clang-format file, because
it is the default for custom styles and doesn't work in .clang-format
(it is a command line option instead):

markus@x4 gcc % clang-format -style=file gcc-main.c
YAML:47:15: error: unknown key 'SortIncludes'

Otherwise it looks good to me.

-- 
Markus

Re: [PATCH] Add clang-format config to contrib folder

[Sebastian Pop]

Martin, thanks for getting this patch out.  I like the patch.
Jeff, clang-format has scripts that allow formatting only the lines
touched by a patch.
It also has a script to integrate with git:
https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/git-clang-format
We could use those scripts in a commit hook to automatically enforce
correct formatting of new patches.

Sebastian

On Wed, Nov 18, 2015 at 8:10 AM, Martin Liška  wrote:
> Hello.
>
> Following patch adds a clang-format config file that should respect the GNU 
> coding standards.
> As the file is not part of build process, I hope the patch can be applied 
> even though
> we've just skipped to stage3? The patch adds a hunk to Makefile which can 
> create symlink
> to the root directory of the GCC compiler. The clang-format automatically 
> loads style from
> the configuration file.
>
> clang-format (version 3.8) provides rich variety of configuration options 
> that can
> ensure the GNU coding style.
>
> Limitations:
> + placement of opening brace of an initializer can't be requested
> + sometimes, '(' is the trailing symbol at the end of a line, which can look 
> weird
>
> As we've been continuously converting our source base to C++, the 
> clang-format should
> provide better results than a collection of regular expressions 
> (check_GNU_style.sh).
>
> As a reference file I attach gcc/tree-ssa-uninit.c file.
> Feel free to comment the suggested configuration file.
>
> Thanks,
> Martin
>