Patchwork [v7,03/10] checkpatch.pl: Check .cpp files

login
register
mail settings
Submitter Tomoki Sekiyama
Date July 15, 2013, 4:20 p.m.
Message ID <20130715162037.16676.9660.stgit@outback>
Download mbox | patch
Permalink /patch/259112/
State New
Headers show

Comments

Tomoki Sekiyama - July 15, 2013, 4:20 p.m.
Enable checkpatch.pl to apply the same checks as C source files for
C++ files with .cpp extensions. It also adds some exceptions for C++
sources to suppress errors for:
  - <> used in C++ template arguments (e.g. template <class T>)
  - :: used to represent namespaces   (e.g. SomeClass::method())
  - : used in class declaration       (e.g. class T : public Super)
  - ~ used in destructor method name  (e.g. T::~T())
  - spacing around 'catch'            (e.g. catch (...))

Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
---
 scripts/checkpatch.pl |   37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)
Michael Roth - July 22, 2013, 9:21 p.m.
Quoting Tomoki Sekiyama (2013-07-15 11:20:37)
> Enable checkpatch.pl to apply the same checks as C source files for
> C++ files with .cpp extensions. It also adds some exceptions for C++
> sources to suppress errors for:
>   - <> used in C++ template arguments (e.g. template <class T>)
>   - :: used to represent namespaces   (e.g. SomeClass::method())
>   - : used in class declaration       (e.g. class T : public Super)
>   - ~ used in destructor method name  (e.g. T::~T())
>   - spacing around 'catch'            (e.g. catch (...))
> 
> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>

One small nit below about documentation. Looks good otherwise.

> ---
>  scripts/checkpatch.pl |   37 +++++++++++++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index ec0aa4c..0ef72b5 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1363,7 +1363,7 @@ sub process {
>  # Check for incorrect file permissions
>                 if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) {
>                         my $permhere = $here . "FILE: $realfile\n";
> -                       if ($realfile =~ /(Makefile|Kconfig|\.c|\.h|\.S|\.tmpl)$/) {
> +                       if ($realfile =~ /(Makefile|Kconfig|\.c|\.cpp|\.h|\.S|\.tmpl)$/) {
>                                 ERROR("do not set execute permissions for source files\n" . $permhere);
>                         }
>                 }
> @@ -1460,7 +1460,7 @@ sub process {
>                 }
> 
>  # check we are in a valid source file if not then ignore this hunk
> -               next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/);
> +               next if ($realfile !~ /\.(h|c|cpp|s|S|pl|sh)$/);
> 
>  #80 column limit
>                 if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ &&
> @@ -1495,7 +1495,7 @@ sub process {
>                 }
> 
>  # check we are in a valid source file C or perl if not then ignore this hunk
> -               next if ($realfile !~ /\.(h|c|pl)$/);
> +               next if ($realfile !~ /\.(h|c|cpp|pl)$/);
> 
>  # in QEMU, no tabs are allowed
>                 if ($rawline =~ /^\+.*    /) {
> @@ -1505,7 +1505,7 @@ sub process {
>                 }
> 
>  # check we are in a valid C source file if not then ignore this hunk
> -               next if ($realfile !~ /\.(h|c)$/);
> +               next if ($realfile !~ /\.(h|c|cpp)$/);
> 
>  # check for RCS/CVS revision markers
>                 if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {
> @@ -1969,6 +1969,9 @@ sub process {
>                                 asm|__asm__)$/x)
>                         {
> 
> +                       # Ignore 'catch (...)' in C++
> +                       } elsif ($name =~ /^catch$/ && $realfile =~ /(\.cpp|\.h)$/) {
> +
>                         # cpp #define statements have non-optional spaces, ie
>                         # if there is a space between the name and the open
>                         # parenthesis it is simply not a parameter group.
> @@ -1992,7 +1995,7 @@ sub process {
>                                 \+=|-=|\*=|\/=|%=|\^=|\|=|&=|
>                                 =>|->|<<|>>|<|>|=|!|~|
>                                 &&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%|
> -                               \?|:
> +                               \?|::|:
>                         }x;
>                         my @elements = split(/($ops|;)/, $opline);
>                         my $off = 0;
> @@ -2066,7 +2069,8 @@ sub process {
>                                 #   ->
>                                 #   :   when part of a bitfield
>                                 } elsif ($op eq '->' || $opv eq ':B') {
> -                                       if ($ctx =~ /Wx.|.xW/) {
> +                                       if ($ctx =~ /Wx.|.xW/ &&
> +                                               !($opv eq ':B' && $line =~ /class/)) {

Everything else seemed fairly obvious or was well commented, but I couldn't
figure out what this change was for. Could you insert a small comment to
explain?

>                                                 ERROR("spaces prohibited around that '$op' $at\n" . $hereptr);
>                                         }
> 
> @@ -2088,7 +2092,11 @@ sub process {
>                                 } elsif ($op eq '!' || $op eq '~' ||
>                                          $opv eq '*U' || $opv eq '-U' ||
>                                          $opv eq '&U' || $opv eq '&&U') {
> -                                       if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
> +                                       if ($op eq '~' && $ca =~ /::$/ && $realfile =~ /(\.cpp|\.h)$/) {
> +                                               # '~' used as a name of Destructor
> +
> +                                       }
> +                                       elsif ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
>                                                 ERROR("space required before that '$op' $at\n" . $hereptr);
>                                         }
>                                         if ($op eq '*' && $cc =~/\s*$Modifier\b/) {
> @@ -2126,8 +2134,9 @@ sub process {
> 
>                                 # A colon needs no spaces before when it is
>                                 # terminating a case value or a label.
> +                               # Ignored if it is used in class declaration in C++.
>                                 } elsif ($opv eq ':C' || $opv eq ':L') {
> -                                       if ($ctx =~ /Wx./) {
> +                                       if ($ctx =~ /Wx./ && $line !~ /class/) {
>                                                 ERROR("space prohibited before that '$op' $at\n" . $hereptr);
>                                         }
> 
> @@ -2135,6 +2144,18 @@ sub process {
>                                 } elsif ($ctx !~ /[EWC]x[CWE]/) {
>                                         my $ok = 0;
> 
> +                                       if ($realfile =~ /\.cpp|\.h$/) {
> +                                               # Ignore template arguments <...> in C++
> +                                               if (($op eq '<' || $op eq '>') && $line =~ /<.*>/) {
> +                                                       $ok = 1;
> +                                               }
> +
> +                                               # Ignore :: in C++
> +                                               if ($op eq '::') {
> +                                                       $ok = 1;
> +                                               }
> +                                       }
> +
>                                         # Ignore email addresses <foo@bar>
>                                         if (($op eq '<' &&
>                                              $cc =~ /^\S+\@\S+>/) ||
Tomoki Sekiyama - July 23, 2013, 9:49 p.m.
On 7/22/13 17:21 , "Michael Roth" <mdroth@linux.vnet.ibm.com> wrote:

>Quoting Tomoki Sekiyama (2013-07-15 11:20:37)
>> Enable checkpatch.pl to apply the same checks as C source files for
>> C++ files with .cpp extensions. It also adds some exceptions for C++
>> sources to suppress errors for:
>>   - <> used in C++ template arguments (e.g. template <class T>)
>>   - :: used to represent namespaces   (e.g. SomeClass::method())
>>   - : used in class declaration       (e.g. class T : public Super)
>>   - ~ used in destructor method name  (e.g. T::~T())
>>   - spacing around 'catch'            (e.g. catch (...))
>> 
>> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
>
>One small nit below about documentation. Looks good otherwise.
<snip>
>>@@ -2066,7 +2069,8 @@ sub process {
>>                                 #   ->
>>                                 #   :   when part of a bitfield
>>                                 } elsif ($op eq '->' || $opv eq ':B') {
>> -                                       if ($ctx =~ /Wx.|.xW/) {
>> +                                       if ($ctx =~ /Wx.|.xW/ &&
>> +                                               !($opv eq ':B' && $line
>>=~ /class/)) {
>
>Everything else seemed fairly obvious or was well commented, but I
>couldn't
>figure out what this change was for. Could you insert a small comment to
>explain?

Sure. This is suppressing errors for : used in class declaration.
I will do some cleanups for this.

Thanks,
Tomoki Sekiyama

Patch

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ec0aa4c..0ef72b5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1363,7 +1363,7 @@  sub process {
 # Check for incorrect file permissions
 		if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) {
 			my $permhere = $here . "FILE: $realfile\n";
-			if ($realfile =~ /(Makefile|Kconfig|\.c|\.h|\.S|\.tmpl)$/) {
+			if ($realfile =~ /(Makefile|Kconfig|\.c|\.cpp|\.h|\.S|\.tmpl)$/) {
 				ERROR("do not set execute permissions for source files\n" . $permhere);
 			}
 		}
@@ -1460,7 +1460,7 @@  sub process {
 		}
 
 # check we are in a valid source file if not then ignore this hunk
-		next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/);
+		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|sh)$/);
 
 #80 column limit
 		if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ &&
@@ -1495,7 +1495,7 @@  sub process {
 		}
 
 # check we are in a valid source file C or perl if not then ignore this hunk
-		next if ($realfile !~ /\.(h|c|pl)$/);
+		next if ($realfile !~ /\.(h|c|cpp|pl)$/);
 
 # in QEMU, no tabs are allowed
 		if ($rawline =~ /^\+.*\t/) {
@@ -1505,7 +1505,7 @@  sub process {
 		}
 
 # check we are in a valid C source file if not then ignore this hunk
-		next if ($realfile !~ /\.(h|c)$/);
+		next if ($realfile !~ /\.(h|c|cpp)$/);
 
 # check for RCS/CVS revision markers
 		if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {
@@ -1969,6 +1969,9 @@  sub process {
 				asm|__asm__)$/x)
 			{
 
+			# Ignore 'catch (...)' in C++
+			} elsif ($name =~ /^catch$/ && $realfile =~ /(\.cpp|\.h)$/) {
+
 			# cpp #define statements have non-optional spaces, ie
 			# if there is a space between the name and the open
 			# parenthesis it is simply not a parameter group.
@@ -1992,7 +1995,7 @@  sub process {
 				\+=|-=|\*=|\/=|%=|\^=|\|=|&=|
 				=>|->|<<|>>|<|>|=|!|~|
 				&&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%|
-				\?|:
+				\?|::|:
 			}x;
 			my @elements = split(/($ops|;)/, $opline);
 			my $off = 0;
@@ -2066,7 +2069,8 @@  sub process {
 				#   ->
 				#   :   when part of a bitfield
 				} elsif ($op eq '->' || $opv eq ':B') {
-					if ($ctx =~ /Wx.|.xW/) {
+					if ($ctx =~ /Wx.|.xW/ &&
+						!($opv eq ':B' && $line =~ /class/)) {
 						ERROR("spaces prohibited around that '$op' $at\n" . $hereptr);
 					}
 
@@ -2088,7 +2092,11 @@  sub process {
 				} elsif ($op eq '!' || $op eq '~' ||
 					 $opv eq '*U' || $opv eq '-U' ||
 					 $opv eq '&U' || $opv eq '&&U') {
-					if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
+					if ($op eq '~' && $ca =~ /::$/ && $realfile =~ /(\.cpp|\.h)$/) {
+						# '~' used as a name of Destructor
+
+					}
+					elsif ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
 						ERROR("space required before that '$op' $at\n" . $hereptr);
 					}
 					if ($op eq '*' && $cc =~/\s*$Modifier\b/) {
@@ -2126,8 +2134,9 @@  sub process {
 
 				# A colon needs no spaces before when it is
 				# terminating a case value or a label.
+				# Ignored if it is used in class declaration in C++.
 				} elsif ($opv eq ':C' || $opv eq ':L') {
-					if ($ctx =~ /Wx./) {
+					if ($ctx =~ /Wx./ && $line !~ /class/) {
 						ERROR("space prohibited before that '$op' $at\n" . $hereptr);
 					}
 
@@ -2135,6 +2144,18 @@  sub process {
 				} elsif ($ctx !~ /[EWC]x[CWE]/) {
 					my $ok = 0;
 
+					if ($realfile =~ /\.cpp|\.h$/) {
+						# Ignore template arguments <...> in C++
+						if (($op eq '<' || $op eq '>') && $line =~ /<.*>/) {
+							$ok = 1;
+						}
+
+						# Ignore :: in C++
+						if ($op eq '::') {
+							$ok = 1;
+						}
+					}
+
 					# Ignore email addresses <foo@bar>
 					if (($op eq '<' &&
 					     $cc =~ /^\S+\@\S+>/) ||