[C++] Coding rule enforcement

[C++] Coding rule enforcement

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

Jason,
somme of our customers have 'interesting' C++ coding rules, they'd like to have 
the compiler enforced.  They want to disable:

1) namespace definitions
2) template declarations
3) multiple inheritance
4) virtual inheritance

But they want to use the STL.  This patch implements 4 new flags, intended to be 
use in the -fno-FOO form.  They're only active outside of system header files.

1) -fno-namespaces disables namespace definitions, but not using directives or 
qualified names.

2) -fno-templates disables primary template declarations, but not template 
instantiations or specializations (partial or otherwise).

3) -fno-multiple-inheritance disables direct multiple inheritance, but not 
indirect multiple inheritance

4) -fno-virtual-inheritance disables direct virtual inheritance, but not 
indirect virtual inheritance.

Thus you can't use any of these features directly, but one can use the STL and 
have it behave as intended.

WDYT?

nathan

[-- Attachment #2: disable.patch --]
[-- Type: text/x-patch, Size: 7320 bytes --]

2015-09-15  Nathan Sidwell  <nathan@codesourcery.com>

	c-family/
	* c.opt (fmultiple-inheritance, fvirtual-inheritance, ftemplates,
	fnamespaces): New C++ options.

	cp/
	* decl.c (xref_basetypes): Check virtual and/or multiple
	inheritance not disabled.
	* parser.c (cp_parser_namespace_definition): Check namespaces
	not disabled.
	* pt.c (push_template_decl_real): Check templates not disabled.

	* doc/invoke.texi  (-fmultiple-inheritance, -fvirtual-inheritance,
	-ftemplates, -namespaces): Document.

	testsuite/
	* g++.dg/diagostic/disable.C: New.

Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 227761)
+++ c-family/c.opt	(working copy)
@@ -1257,9 +1257,17 @@ C ObjC C++ ObjC++ Ignore Warn(switch %qs
 fmudflapir
 C ObjC C++ ObjC++ Ignore Warn(switch %qs is no longer supported)
 
+fmultiple-inheritance
+C++ ObjC++ Var(flag_multiple_inheritance) Init(1)
+Enable multiple inheritance
+
 fname-mangling-version-
 C++ ObjC++ Joined Ignore Warn(switch %qs is no longer supported)
 
+fnamespaces
+C++ ObjC++ Var(flag_namespaces) Init(1)
+Enable namespaces
+
 fnew-abi
 C++ ObjC++ Ignore Warn(switch %qs is no longer supported)
 
@@ -1446,6 +1454,10 @@ ftabstop=
 C ObjC C++ ObjC++ Joined RejectNegative UInteger
 -ftabstop=<number>	Distance between tab stops for column reporting
 
+ftemplates
+C++ ObjC++ Var(flag_templates) Init(1)
+Enable templates
+
 ftemplate-backtrace-limit=
 C++ ObjC++ Joined RejectNegative UInteger Var(template_backtrace_limit) Init(10)
 Set the maximum number of template instantiation notes for a single warning or error
@@ -1480,6 +1492,10 @@ fuse-cxa-get-exception-ptr
 C++ ObjC++ Var(flag_use_cxa_get_exception_ptr) Init(2)
 Use __cxa_get_exception_ptr in exception handling
 
+fvirtual-inheritance
+C++ ObjC++ Var(flag_virtual_inheritance) Init(1)
+Enable virtual inheritance
+
 fvisibility-inlines-hidden
 C++ ObjC++
 Marks all inlined functions and methods as having hidden visibility
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 227761)
+++ cp/decl.c	(working copy)
@@ -12814,6 +12814,9 @@ xref_basetypes (tree ref, tree base_list
 	  error ("Java class %qT cannot have multiple bases", ref);
           return false;
         }
+      else if (!flag_multiple_inheritance
+	       && !in_system_header_at (input_location))
+	error ("%qT defined with multiple bases", ref);
     }
 
   if (max_vbases)
@@ -12825,6 +12828,9 @@ xref_basetypes (tree ref, tree base_list
 	  error ("Java class %qT cannot have virtual bases", ref);
           return false;
         }
+      else if (!flag_virtual_inheritance
+	       && !in_system_header_at (input_location))
+	error ("%qT defined with virtual base", ref);
     }
 
   for (igo_prev = binfo; base_list; base_list = TREE_CHAIN (base_list))
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 227761)
+++ cp/parser.c	(working copy)
@@ -16798,6 +16798,9 @@ cp_parser_namespace_definition (cp_parse
 
   has_visibility = handle_namespace_attrs (current_namespace, attribs);
 
+  if (!flag_namespaces && !in_system_header_at (input_location))
+    error  ("namepace %qD entered", current_namespace);
+
   /* Parse the body of the namespace.  */
   cp_parser_namespace_body (parser);
 
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 227761)
+++ cp/pt.c	(working copy)
@@ -5088,6 +5088,9 @@ push_template_decl_real (tree decl, bool
 
   if (is_primary)
     {
+      if (!flag_templates && !in_system_header_at (input_location))
+	error ("template %qD declared", decl);
+
       if (DECL_CLASS_SCOPE_P (decl))
 	member_template_p = true;
       if (TREE_CODE (decl) == TYPE_DECL
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 227761)
+++ doc/invoke.texi	(working copy)
@@ -183,19 +183,19 @@ in the following sections.
 -ffor-scope  -fno-for-scope  -fno-gnu-keywords @gol
 -fno-implicit-templates @gol
 -fno-implicit-inline-templates @gol
--fno-implement-inlines  -fms-extensions @gol
--fno-nonansi-builtins  -fnothrow-opt  -fno-operator-names @gol
+-fno-implement-inlines  -fms-extensions -fno-multiple-inheritance &gol
+-fno-namespaces -fno-nonansi-builtins  -fnothrow-opt  -fno-operator-names @gol
 -fno-optional-diags  -fpermissive @gol
 -fno-pretty-templates @gol
 -frepo  -fno-rtti -fsized-deallocation @gol
 -fstats  -ftemplate-backtrace-limit=@var{n} @gol
--ftemplate-depth=@var{n} @gol
+-ftemplate-depth=@var{n} -fno-templates @gol
 -fno-threadsafe-statics  -fuse-cxa-atexit @gol
 -fno-weak  -nostdinc++ @gol
 -fvisibility-inlines-hidden @gol
 -fvtable-verify=@r{[}std@r{|}preinit@r{|}none@r{]} @gol
 -fvtv-counts -fvtv-debug @gol
--fvisibility-ms-compat @gol
+-fno-virtual-inheritance -fvisibility-ms-compat @gol
 -fext-numeric-literals @gol
 -Wabi=@var{n}  -Wabi-tag  -Wconversion-null  -Wctor-dtor-privacy @gol
 -Wdelete-non-virtual-dtor -Wliteral-suffix -Wnarrowing @gol
@@ -2292,6 +2292,14 @@ errors if these functions are not inline
 Disable Wpedantic warnings about constructs used in MFC, such as implicit
 int and getting a pointer to member function via non-standard syntax.
 
+@item -fno-multiple-inheritance
+@opindex fno-multiple-inheritance
+Do not permit multiple inheritance, except in system header files.
+
+@item -fno-namespaces
+@opindex fno-namespacess
+Do not permit namespace definitions, except in system header files.
+
 @item -fno-nonansi-builtins
 @opindex fno-nonansi-builtins
 Disable built-in declarations of functions that are not mandated by
@@ -2399,6 +2407,10 @@ conforming programs must not rely on a m
 (changed to 1024 in C++11).  The default value is 900, as the compiler
 can run out of stack space before hitting 1024 in some situations.
 
+@item -fno-templates
+@opindex fno-templates
+Do not permit primary template declarations, except in system header files.
+
 @item -fno-threadsafe-statics
 @opindex fno-threadsafe-statics
 Do not emit the extra code to use the routines specified in the C++
@@ -2420,6 +2432,10 @@ Don't use the @code{__cxa_get_exception_
 causes @code{std::uncaught_exception} to be incorrect, but is necessary
 if the runtime routine is not available.
 
+@item -fno-virtual-inheritance
+@opindex fno-virtual-inheritance
+Do not permit virtual inheritance, except in system header files.
+
 @item -fvisibility-inlines-hidden
 @opindex fvisibility-inlines-hidden
 This switch declares that the user does not attempt to compare
Index: testsuite/g++.dg/diagnostic/disable.C
===================================================================
--- testsuite/g++.dg/diagnostic/disable.C	(revision 0)
+++ testsuite/g++.dg/diagnostic/disable.C	(working copy)
@@ -0,0 +1,23 @@
+// { dg-options "-fno-templates -fno-multiple-inheritance -fno-virtual-inheritance -fno-namespaces" }
+
+#include <iostream>
+#include <algorithm>
+
+namespace foo { } // { dg-error "namepace" }
+
+template <typename X> X Foo (); // { dg-error "template" }
+
+struct B1 {};
+struct B2 {};
+struct V {};
+
+struct D :  B1, B2 {}; //  { dg-error "multiple" }
+
+struct E : virtual V {};  // { dg-error "virtual" }
+
+void Baz (int a, int b)
+{
+  std::swap (a, b);
+}
+
+

Re: [C++] Coding rule enforcement

[Richard Biener]

On Tue, Sep 15, 2015 at 3:02 PM, Nathan Sidwell <nathan@acm.org> wrote:
> Jason,
> somme of our customers have 'interesting' C++ coding rules, they'd like to
> have the compiler enforced.  They want to disable:
>
> 1) namespace definitions
> 2) template declarations
> 3) multiple inheritance
> 4) virtual inheritance
>
> But they want to use the STL.  This patch implements 4 new flags, intended
> to be use in the -fno-FOO form.  They're only active outside of system
> header files.
>
> 1) -fno-namespaces disables namespace definitions, but not using directives
> or qualified names.
>
> 2) -fno-templates disables primary template declarations, but not template
> instantiations or specializations (partial or otherwise).
>
> 3) -fno-multiple-inheritance disables direct multiple inheritance, but not
> indirect multiple inheritance
>
> 4) -fno-virtual-inheritance disables direct virtual inheritance, but not
> indirect virtual inheritance.

Wouldn't warning flags be better so you can decide whether it's an error
or a warning via -Werror=virtual-inheritance vs. -Wvirtual-inheritance?

> Thus you can't use any of these features directly, but one can use the STL
> and have it behave as intended.
>
> WDYT?
>
> nathan

Re: [C++] Coding rule enforcement

[Nathan Sidwell]

On 09/15/15 09:26, Richard Biener wrote:

> Wouldn't warning flags be better so you can decide whether it's an error
> or a warning via -Werror=virtual-inheritance vs. -Wvirtual-inheritance?

I agree.  I didn't know of the -Werror=FOO trick to make them individually errors.

nathan

Re: [C++] Coding rule enforcement

[Jason Merrill]

On 09/15/2015 09:26 AM, Richard Biener wrote:
> Wouldn't warning flags be better so you can decide whether it's an error
> or a warning via -Werror=virtual-inheritance vs. -Wvirtual-inheritance?

Yep.  That also handles the system header exemption (unless 
-Wsystem-headers).

Jason

Re: [C++] Coding rule enforcement

[Manuel López-Ibáñez]

On 15/09/15 15:26, Richard Biener wrote:
> On Tue, Sep 15, 2015 at 3:02 PM, Nathan Sidwell <nathan@acm.org> wrote:
>> Jason,
>> somme of our customers have 'interesting' C++ coding rules, they'd like to
>> have the compiler enforced.  They want to disable:
>>
>> 1) namespace definitions
>> 2) template declarations
>> 3) multiple inheritance
>> 4) virtual inheritance
>>
>> But they want to use the STL.  This patch implements 4 new flags, intended
>> to be use in the -fno-FOO form.  They're only active outside of system
>> header files.

If these are quite specific coding rules, wouldn't this be something ideal for 
a plugin rather than implemented in the compiler proper?

One can implement warnings with plugins (in python if desired!)

http://gcc-python-plugin.readthedocs.org/en/latest/basics.html#generating-custom-errors-and-warnings

Compiler plugins that implement specific coding rules are quite common, alas, 
using clang not GCC: https://wiki.documentfoundation.org/Development/Clang_plugins

It would be more generally useful to extend GCC to fully support this type of 
plugins.

Cheers,

Manuel.

Re: [C++] Coding rule enforcement

[Jason Merrill]

On 09/15/2015 01:20 PM, Manuel López-Ibáñez wrote:
> On 15/09/15 15:26, Richard Biener wrote:
>> On Tue, Sep 15, 2015 at 3:02 PM, Nathan Sidwell <nathan@acm.org> wrote:
>>> Jason,
>>> somme of our customers have 'interesting' C++ coding rules, they'd
>>> like to
>>> have the compiler enforced.  They want to disable:
>>>
>>> 1) namespace definitions
>>> 2) template declarations
>>> 3) multiple inheritance
>>> 4) virtual inheritance
>>>
>>> But they want to use the STL.  This patch implements 4 new flags,
>>> intended
>>> to be use in the -fno-FOO form.  They're only active outside of system
>>> header files.
>
> If these are quite specific coding rules, wouldn't this be something
> ideal for a plugin rather than implemented in the compiler proper?
>
> One can implement warnings with plugins (in python if desired!)
>
> http://gcc-python-plugin.readthedocs.org/en/latest/basics.html#generating-custom-errors-and-warnings
>
>
> Compiler plugins that implement specific coding rules are quite common,
> alas, using clang not GCC:
> https://wiki.documentfoundation.org/Development/Clang_plugins
>
> It would be more generally useful to extend GCC to fully support this
> type of plugins.

Good point.

Jason

Re: [C++] Coding rule enforcement

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 396 bytes --]

On 09/15/15 09:33, Jason Merrill wrote:
> On 09/15/2015 09:26 AM, Richard Biener wrote:
>> Wouldn't warning flags be better so you can decide whether it's an error
>> or a warning via -Werror=virtual-inheritance vs. -Wvirtual-inheritance?
>
> Yep.  That also handles the system header exemption (unless -Wsystem-headers).

This  version implements Richard's suggestion of warning flags.

nathan


[-- Attachment #2: disable.patch --]
[-- Type: text/x-patch, Size: 8083 bytes --]

2015-09-16  Nathan Sidwell  <nathan@codesourcery.com>

	c-family/
	* c.opt (Wmultiple-inheritance, Wvirtual-inheritance, Wtemplates,
	Wnamespaces): New C++ warnings.

	cp/
	* decl.c (xref_basetypes): Check virtual and/or multiple
	inheritance warning.
	* parser.c (cp_parser_namespace_definition): Check namespaces
	warning.
	* pt.c (push_template_decl_real): Check templates warning.

	* doc/invoke.texi  (-Wmultiple-inheritance, -Wvirtual-inheritance,
	-Wtemplates, -Wnamespaces): Document.

	testsuite/
	* g++.dg/diagostic/disable.C: New.

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 227761)
+++ gcc/c-family/c.opt	(working copy)
@@ -573,6 +573,14 @@ Wmissing-field-initializers
 C ObjC C++ ObjC++ Var(warn_missing_field_initializers) Warning EnabledBy(Wextra)
 Warn about missing fields in struct initializers
 
+Wmultiple-inheritance
+C++ ObjC++ Var(warn_multiple_inheritance) Warninng
+Warn on direct multiple inheritance
+
+Wnamespaces
+C++ ObjC++ Var(warn_namespaces) Warning
+Warn on namespace definition
+
 Wsized-deallocation
 C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra)
 Warn about missing sized deallocation functions
@@ -610,6 +618,10 @@ Wswitch-bool
 C ObjC C++ ObjC++ Var(warn_switch_bool) Warning Init(1)
 Warn about switches with boolean controlling expression
 
+Wtemplates
+C++ ObjC++ Var(warn_templates) Warning
+Warn on primary template declaration
+
 Wmissing-format-attribute
 C ObjC C++ ObjC++ Alias(Wsuggest-attribute=format)
 ;
@@ -936,6 +948,10 @@ Wvolatile-register-var
 C ObjC C++ ObjC++ Var(warn_volatile_register_var) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn when a register variable is declared volatile
 
+Wvirtual-inheritance
+C++ ObjC++ Var(warn_virtual_inheritance) Warning
+Warn on direct virtual inheritance
+
 Wvirtual-move-assign
 C++ ObjC++ Var(warn_virtual_move_assign) Warning Init(1)
 Warn if a virtual base has a non-trivial move assignment operator
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 227761)
+++ gcc/cp/decl.c	(working copy)
@@ -12729,6 +12729,7 @@ xref_basetypes (tree ref, tree base_list
   tree binfo, base_binfo;
   unsigned max_vbases = 0; /* Maximum direct & indirect virtual bases.  */
   unsigned max_bases = 0;  /* Maximum direct bases.  */
+  unsigned max_dvbases = 0; /* Maximum direct virtual bases.  */
   int i;
   tree default_access;
   tree igo_prev; /* Track Inheritance Graph Order.  */
@@ -12766,12 +12767,13 @@ xref_basetypes (tree ref, tree base_list
 	{
 	  max_bases++;
 	  if (TREE_TYPE (*basep))
-	    max_vbases++;
+	    max_dvbases++;
 	  if (CLASS_TYPE_P (basetype))
 	    max_vbases += vec_safe_length (CLASSTYPE_VBASECLASSES (basetype));
 	  basep = &TREE_CHAIN (*basep);
 	}
     }
+  max_vbases += max_dvbases;
 
   TYPE_MARKED_P (ref) = 1;
 
@@ -12814,6 +12816,9 @@ xref_basetypes (tree ref, tree base_list
 	  error ("Java class %qT cannot have multiple bases", ref);
           return false;
         }
+      else if (warn_multiple_inheritance)
+	warning (OPT_Wmultiple_inheritance,
+		 "%qT defined with multiple direct bases", ref);
     }
 
   if (max_vbases)
@@ -12825,6 +12830,9 @@ xref_basetypes (tree ref, tree base_list
 	  error ("Java class %qT cannot have virtual bases", ref);
           return false;
         }
+      else if (max_dvbases && warn_virtual_inheritance)
+	warning (OPT_Wvirtual_inheritance,
+		 "%qT defined with direct virtual base", ref);
     }
 
   for (igo_prev = binfo; base_list; base_list = TREE_CHAIN (base_list))
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 227761)
+++ gcc/cp/parser.c	(working copy)
@@ -16798,6 +16798,9 @@ cp_parser_namespace_definition (cp_parse
 
   has_visibility = handle_namespace_attrs (current_namespace, attribs);
 
+  if (warn_namespaces)
+    warning  (OPT_Wnamespaces, "namepace %qD entered", current_namespace);
+
   /* Parse the body of the namespace.  */
   cp_parser_namespace_body (parser);
 
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 227761)
+++ gcc/cp/pt.c	(working copy)
@@ -5088,6 +5088,9 @@ push_template_decl_real (tree decl, bool
 
   if (is_primary)
     {
+      if (warn_templates)
+	warning (OPT_Wtemplates, "template %qD declared", decl);
+      
       if (DECL_CLASS_SCOPE_P (decl))
 	member_template_p = true;
       if (TREE_CODE (decl) == TYPE_DECL
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 227761)
+++ gcc/doc/invoke.texi	(working copy)
@@ -198,12 +198,13 @@ in the following sections.
 -fvisibility-ms-compat @gol
 -fext-numeric-literals @gol
 -Wabi=@var{n}  -Wabi-tag  -Wconversion-null  -Wctor-dtor-privacy @gol
--Wdelete-non-virtual-dtor -Wliteral-suffix -Wnarrowing @gol
+-Wdelete-non-virtual-dtor -Wliteral-suffix -Wmultiple-inheritance @gol
+-Wnamespaces -Wnarrowing @gol
 -Wnoexcept -Wnon-virtual-dtor  -Wreorder @gol
--Weffc++  -Wstrict-null-sentinel @gol
+-Weffc++  -Wstrict-null-sentinel -Wtemplates @gol
 -Wno-non-template-friend  -Wold-style-cast @gol
 -Woverloaded-virtual  -Wno-pmf-conversions @gol
--Wsign-promo}
+-Wsign-promo -Wvirtual-inheritance}
 
 @item Objective-C and Objective-C++ Language Options
 @xref{Objective-C and Objective-C++ Dialect Options,,Options Controlling
@@ -2903,6 +2904,37 @@ enumerated type to a signed type, over a
 the same size.  Previous versions of G++ tried to preserve
 unsignedness, but the standard mandates the current behavior.
 
+@item -Wtemplates @r{(C++ and Objective-C++ only)}
+@opindex Wtemplates
+Warn when a primary template declaration is encountered.  Some coding
+rules disallow templates, and this may be used to enforce that rule.
+The warning is inactive inside a system header file, such as the STL, so
+one can still use the STL.  One may also instantiate or specialize
+templates.
+
+@item -Wmultiple-inheritance @r{(C++ and Objective-C++ only)}
+@opindex Wmultiple-inheritance
+Warn when a class is defined with multiple direct base classes.  Some
+coding rules disallow multiple inheritance, and this may be used to
+enforce that rule.  The warning is inactive inside a system header file,
+such as the STL, so one can still use the STL.  One may also define
+classes that indirectly use multiple inheritance.
+
+@item -Wvirtual-inheritance
+@opindex Wvirtual-inheritance
+Warn when a class is defined with a virtual direct base classe.  Some
+coding rules disallow multiple inheritance, and this may be used to
+enforce that rule.  The warning is inactive inside a system header file,
+such as the STL, so one can still use the STL.  One may also define
+classes that indirectly use virtual inheritance.
+
+@item -Wnamespaces
+@opindex Wnamespaces
+Warn when a namespace definition is opened.  Some coding rules disallow
+namespaces, and this may be used to enforce that rule.  The warning is
+inactive inside a system header file, such as the STL, so one can still
+use the STL.  One may also use using directives and qualified names.
+
 @item -Wno-terminate @r{(C++ and Objective-C++ only)}
 @opindex Wterminate
 @opindex Wno-terminate
Index: gcc/testsuite/g++.dg/diagnostic/disable.C
===================================================================
--- gcc/testsuite/g++.dg/diagnostic/disable.C	(revision 0)
+++ gcc/testsuite/g++.dg/diagnostic/disable.C	(working copy)
@@ -0,0 +1,27 @@
+// { dg-options "-Wtemplates -Wmultiple-inheritance -Wvirtual-inheritance -Wnamespaces" }
+
+#include <iostream>
+#include <algorithm>
+
+namespace foo { } // { dg-warning "namepace" }
+
+template <typename X> X Foo (); // { dg-warning "template" }
+
+struct B1 {};
+struct B2 {};
+struct V {};
+
+struct D :  B1, B2 {}; //  { dg-warning "multiple" }
+
+struct E : virtual V {};  // { dg-warning "virtual" }
+
+struct F1 : E {};
+
+struct F2 : D {};
+
+void Baz (int a, int b)
+{
+  std::swap (a, b);
+}
+
+

Re: [C++] Coding rule enforcement

[Jason Merrill]

On 09/16/2015 08:02 AM, Nathan Sidwell wrote:
> +      else if (warn_multiple_inheritance)
> +	warning (OPT_Wmultiple_inheritance,
> +		 "%qT defined with multiple direct bases", ref);

You don't need to guard the warning with a check of the warning flag; 
warning will only give the warning if the option is enabled.

Jason

Re: [C++] Coding rule enforcement

[Nathan Sidwell]

On 09/16/15 10:23, Jason Merrill wrote:
> On 09/16/2015 08:02 AM, Nathan Sidwell wrote:
>> +      else if (warn_multiple_inheritance)
>> +    warning (OPT_Wmultiple_inheritance,
>> +         "%qT defined with multiple direct bases", ref);
>
> You don't need to guard the warning with a check of the warning flag; warning
> will only give the warning if the option is enabled.

hm, it didn't seem to be doing that.  Perhaps  I'd got something wrong ...looking

Re: [C++] Coding rule enforcement

[Manuel López-Ibáñez]

On 16/09/15 16:36, Nathan Sidwell wrote:
> On 09/16/15 10:23, Jason Merrill wrote:
>> On 09/16/2015 08:02 AM, Nathan Sidwell wrote:
>>> +      else if (warn_multiple_inheritance)
>>> +    warning (OPT_Wmultiple_inheritance,
>>> +         "%qT defined with multiple direct bases", ref);
>>
>> You don't need to guard the warning with a check of the warning flag; warning
>> will only give the warning if the option is enabled.
>
> hm, it didn't seem to be doing that.  Perhaps  I'd got something wrong ...looking

Perhaps because you misspelled Warning here?

+Wmultiple-inheritance
+C++ ObjC++ Var(warn_multiple_inheritance) Warninng
+Warn on direct multiple inheritance
+

The awk scripts that parse the .opt files are not as smart as they could be.

Cheers,

Manuel.

Re: [C++] Coding rule enforcement

[Bernhard Reutner-Fischer]

On September 16, 2015 6:32:46 PM GMT+02:00, "Manuel López-Ibáñez" <lopezibanez@gmail.com> wrote:
>On 16/09/15 16:36, Nathan Sidwell wrote:
>> On 09/16/15 10:23, Jason Merrill wrote:
>>> On 09/16/2015 08:02 AM, Nathan Sidwell wrote:
>>>> +      else if (warn_multiple_inheritance)
>>>> +    warning (OPT_Wmultiple_inheritance,
>>>> +         "%qT defined with multiple direct bases", ref);
>>>
>>> You don't need to guard the warning with a check of the warning
>flag; warning
>>> will only give the warning if the option is enabled.
>>
>> hm, it didn't seem to be doing that.  Perhaps  I'd got something
>wrong ...looking
>
>Perhaps because you misspelled Warning here?
>
>+Wmultiple-inheritance
>+C++ ObjC++ Var(warn_multiple_inheritance) Warninng
>+Warn on direct multiple inheritance
>+
>
>The awk scripts that parse the .opt files are not as smart as they
>could be.

Ouch, unrecognised keywords there should barf really loud.
Great bug.

Re: [C++] Coding rule enforcement

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

On 09/16/15 10:23, Jason Merrill wrote:
> On 09/16/2015 08:02 AM, Nathan Sidwell wrote:
>> +      else if (warn_multiple_inheritance)
>> +    warning (OPT_Wmultiple_inheritance,
>> +         "%qT defined with multiple direct bases", ref);
>
> You don't need to guard the warning with a check of the warning flag; warning
> will only give the warning if the option is enabled.

the spelling mistake on the option I used to experiment on didn't help, but I 
discovered one must have the Var clause in the options file too, which I didn't 
expect.

Anyway, fixed now.  ok?

nathan

[-- Attachment #2: disable.patch --]
[-- Type: text/x-patch, Size: 7976 bytes --]

2015-09-17  Nathan Sidwell  <nathan@codesourcery.com>

	c-family/
	* c.opt (Wmultiple-inheritance, Wvirtual-inheritance, Wtemplates,
	Wnamespaces): New C++ warnings.

	cp/
	* decl.c (xref_basetypes): Check virtual and/or multiple
	inheritance warning..
	* parser.c (cp_parser_namespace_definition): Check namespaces
	warning.
	* pt.c (push_template_decl_real): Check templates warning.

	* doc/invoke.texi  (-Wmultiple-inheritance, -Wvirtual-inheritance,
	-Wtemplates, -Wnamespaces): Document.

	testsuite/
	* g++.dg/diagostic/disable.C: New.

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 227761)
+++ gcc/c-family/c.opt	(working copy)
@@ -573,6 +573,14 @@ Wmissing-field-initializers
 C ObjC C++ ObjC++ Var(warn_missing_field_initializers) Warning EnabledBy(Wextra)
 Warn about missing fields in struct initializers
 
+Wmultiple-inheritance
+C++ ObjC++ Var(warn_multiple_inheritance) Warning
+Warn on direct multiple inheritance
+
+Wnamespaces
+C++ ObjC++ Var(warn_namespaces) Warning
+Warn on namespace definition
+
 Wsized-deallocation
 C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra)
 Warn about missing sized deallocation functions
@@ -610,6 +618,10 @@ Wswitch-bool
 C ObjC C++ ObjC++ Var(warn_switch_bool) Warning Init(1)
 Warn about switches with boolean controlling expression
 
+Wtemplates
+C++ ObjC++ Var(warn_templates) Warning
+Warn on primary template declaration
+
 Wmissing-format-attribute
 C ObjC C++ ObjC++ Alias(Wsuggest-attribute=format)
 ;
@@ -936,6 +948,10 @@ Wvolatile-register-var
 C ObjC C++ ObjC++ Var(warn_volatile_register_var) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn when a register variable is declared volatile
 
+Wvirtual-inheritance
+C++ ObjC++ Var(warn_virtual_inheritance) Warning
+Warn on direct virtual inheritance
+
 Wvirtual-move-assign
 C++ ObjC++ Var(warn_virtual_move_assign) Warning Init(1)
 Warn if a virtual base has a non-trivial move assignment operator
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 227761)
+++ gcc/cp/decl.c	(working copy)
@@ -12729,6 +12729,7 @@ xref_basetypes (tree ref, tree base_list
   tree binfo, base_binfo;
   unsigned max_vbases = 0; /* Maximum direct & indirect virtual bases.  */
   unsigned max_bases = 0;  /* Maximum direct bases.  */
+  unsigned max_dvbases = 0; /* Maximum direct virtual bases.  */
   int i;
   tree default_access;
   tree igo_prev; /* Track Inheritance Graph Order.  */
@@ -12766,12 +12767,13 @@ xref_basetypes (tree ref, tree base_list
 	{
 	  max_bases++;
 	  if (TREE_TYPE (*basep))
-	    max_vbases++;
+	    max_dvbases++;
 	  if (CLASS_TYPE_P (basetype))
 	    max_vbases += vec_safe_length (CLASSTYPE_VBASECLASSES (basetype));
 	  basep = &TREE_CHAIN (*basep);
 	}
     }
+  max_vbases += max_dvbases;
 
   TYPE_MARKED_P (ref) = 1;
 
@@ -12814,6 +12816,9 @@ xref_basetypes (tree ref, tree base_list
 	  error ("Java class %qT cannot have multiple bases", ref);
           return false;
         }
+      else
+	warning (OPT_Wmultiple_inheritance,
+		 "%qT defined with multiple direct bases", ref);
     }
 
   if (max_vbases)
@@ -12825,6 +12830,9 @@ xref_basetypes (tree ref, tree base_list
 	  error ("Java class %qT cannot have virtual bases", ref);
           return false;
         }
+      else if (max_dvbases)
+	warning (OPT_Wvirtual_inheritance,
+		 "%qT defined with direct virtual base", ref);
     }
 
   for (igo_prev = binfo; base_list; base_list = TREE_CHAIN (base_list))
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 227761)
+++ gcc/cp/parser.c	(working copy)
@@ -16798,6 +16798,8 @@ cp_parser_namespace_definition (cp_parse
 
   has_visibility = handle_namespace_attrs (current_namespace, attribs);
 
+  warning  (OPT_Wnamespaces, "namepace %qD entered", current_namespace);
+
   /* Parse the body of the namespace.  */
   cp_parser_namespace_body (parser);
 
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 227761)
+++ gcc/cp/pt.c	(working copy)
@@ -5088,6 +5088,8 @@ push_template_decl_real (tree decl, bool
 
   if (is_primary)
     {
+      warning (OPT_Wtemplates, "template %qD declared", decl);
+      
       if (DECL_CLASS_SCOPE_P (decl))
 	member_template_p = true;
       if (TREE_CODE (decl) == TYPE_DECL
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 227761)
+++ gcc/doc/invoke.texi	(working copy)
@@ -198,12 +198,13 @@ in the following sections.
 -fvisibility-ms-compat @gol
 -fext-numeric-literals @gol
 -Wabi=@var{n}  -Wabi-tag  -Wconversion-null  -Wctor-dtor-privacy @gol
--Wdelete-non-virtual-dtor -Wliteral-suffix -Wnarrowing @gol
+-Wdelete-non-virtual-dtor -Wliteral-suffix -Wmultiple-inheritance @gol
+-Wnamespaces -Wnarrowing @gol
 -Wnoexcept -Wnon-virtual-dtor  -Wreorder @gol
--Weffc++  -Wstrict-null-sentinel @gol
+-Weffc++  -Wstrict-null-sentinel -Wtemplates @gol
 -Wno-non-template-friend  -Wold-style-cast @gol
 -Woverloaded-virtual  -Wno-pmf-conversions @gol
--Wsign-promo}
+-Wsign-promo -Wvirtual-inheritance}
 
 @item Objective-C and Objective-C++ Language Options
 @xref{Objective-C and Objective-C++ Dialect Options,,Options Controlling
@@ -2903,6 +2904,37 @@ enumerated type to a signed type, over a
 the same size.  Previous versions of G++ tried to preserve
 unsignedness, but the standard mandates the current behavior.
 
+@item -Wtemplates @r{(C++ and Objective-C++ only)}
+@opindex Wtemplates
+Warn when a primary template declaration is encountered.  Some coding
+rules disallow templates, and this may be used to enforce that rule.
+The warning is inactive inside a system header file, such as the STL, so
+one can still use the STL.  One may also instantiate or specialize
+templates.
+
+@item -Wmultiple-inheritance @r{(C++ and Objective-C++ only)}
+@opindex Wmultiple-inheritance
+Warn when a class is defined with multiple direct base classes.  Some
+coding rules disallow multiple inheritance, and this may be used to
+enforce that rule.  The warning is inactive inside a system header file,
+such as the STL, so one can still use the STL.  One may also define
+classes that indirectly use multiple inheritance.
+
+@item -Wvirtual-inheritance
+@opindex Wvirtual-inheritance
+Warn when a class is defined with a virtual direct base classe.  Some
+coding rules disallow multiple inheritance, and this may be used to
+enforce that rule.  The warning is inactive inside a system header file,
+such as the STL, so one can still use the STL.  One may also define
+classes that indirectly use virtual inheritance.
+
+@item -Wnamespaces
+@opindex Wnamespaces
+Warn when a namespace definition is opened.  Some coding rules disallow
+namespaces, and this may be used to enforce that rule.  The warning is
+inactive inside a system header file, such as the STL, so one can still
+use the STL.  One may also use using directives and qualified names.
+
 @item -Wno-terminate @r{(C++ and Objective-C++ only)}
 @opindex Wterminate
 @opindex Wno-terminate
Index: gcc/testsuite/g++.dg/diagnostic/disable.C
===================================================================
--- gcc/testsuite/g++.dg/diagnostic/disable.C	(revision 0)
+++ gcc/testsuite/g++.dg/diagnostic/disable.C	(working copy)
@@ -0,0 +1,27 @@
+// { dg-options "-Wtemplates -Wmultiple-inheritance -Wvirtual-inheritance -Wnamespaces" }
+
+#include <iostream>
+#include <algorithm>
+
+namespace foo { } // { dg-warning "namepace" }
+
+template <typename X> X Foo (); // { dg-warning "template" }
+
+struct B1 {};
+struct B2 {};
+struct V {};
+
+struct D :  B1, B2 {}; //  { dg-warning "multiple" }
+
+struct E : virtual V {};  // { dg-warning "virtual" }
+
+struct F1 : E {};
+
+struct F2 : D {};
+
+void Baz (int a, int b)
+{
+  std::swap (a, b);
+}
+
+

Re: [C++] Coding rule enforcement

[Daniel Gutson]

On Tue, Sep 15, 2015 at 3:31 PM, Jason Merrill <jason@redhat.com> wrote:
> On 09/15/2015 01:20 PM, Manuel López-Ibáñez wrote:
>>
>> On 15/09/15 15:26, Richard Biener wrote:
>>>
>>> On Tue, Sep 15, 2015 at 3:02 PM, Nathan Sidwell <nathan@acm.org> wrote:
>>>>
>>>> Jason,
>>>> somme of our customers have 'interesting' C++ coding rules, they'd
>>>> like to
>>>> have the compiler enforced.  They want to disable:
>>>>
>>>> 1) namespace definitions
>>>> 2) template declarations
>>>> 3) multiple inheritance
>>>> 4) virtual inheritance
>>>>
>>>> But they want to use the STL.  This patch implements 4 new flags,
>>>> intended
>>>> to be use in the -fno-FOO form.  They're only active outside of system
>>>> header files.
>>
>>
>> If these are quite specific coding rules, wouldn't this be something
>> ideal for a plugin rather than implemented in the compiler proper?
>>
>> One can implement warnings with plugins (in python if desired!)
>>
>>
>> http://gcc-python-plugin.readthedocs.org/en/latest/basics.html#generating-custom-errors-and-warnings
>>
>>
>> Compiler plugins that implement specific coding rules are quite common,
>> alas, using clang not GCC:
>> https://wiki.documentfoundation.org/Development/Clang_plugins
>>
>> It would be more generally useful to extend GCC to fully support this
>> type of plugins.
>
>
> Good point.

FWIW, we could make this plugin in 2 weeks (w already have static
checkers as plugins for our customers). I understand Nathan that you
may have some deadlines, but if we could have the opportunity to
implement it, we could accomplish a clean isolation of a particular
business needs (despite I acknowledge that a warning about virtual
inheritance may be useful for a broader audience). OTOH, a plugin can
receive arguments, such as a configuration file which could point to
specific sources or hints about where to apply the warning, or a
suppression file, both things useful for large legacy code. IMVHO I
think this is a superior solution.
Please let me know if we could collaborate to get both a better gcc
and a better static checker.


>
> Jason
>
>



-- 

Daniel F. Gutson
Chief Engineering Officer, SPD

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina

Phone:   +54 351 4217888 / +54 351 4218211
Skype:    dgutson
LinkedIn: http://ar.linkedin.com/in/danielgutson

Re: [C++] Coding rule enforcement

[Manuel López-Ibáñez]

On 21 September 2015 at 15:46, Daniel Gutson
<daniel.gutson@tallertechnologies.com> wrote:
>
> FWIW, we could make this plugin in 2 weeks (w already have static
> checkers as plugins for our customers). I understand Nathan that you
> may have some deadlines, but if we could have the opportunity to
> implement it, we could accomplish a clean isolation of a particular
> business needs (despite I acknowledge that a warning about virtual
> inheritance may be useful for a broader audience). OTOH, a plugin can
> receive arguments, such as a configuration file which could point to
> specific sources or hints about where to apply the warning, or a
> suppression file, both things useful for large legacy code. IMVHO I
> think this is a superior solution.
> Please let me know if we could collaborate to get both a better gcc
> and a better static checker.

My opinion is that if people want to "donate" their plugins to the
FSF, they are willing to maintain them, and they are not overtly
complex or require any third-party software, they should be added to
the GCC repository and build/tested. They could serve as examples and
extra testing for the plugin framework, with the expectation that they
may get removed if they become unmaintained.

Cheers,

Manuel.

Re: [C++] Coding rule enforcement

[Jason Merrill]

On 09/21/2015 10:01 AM, Manuel López-Ibáñez wrote:
> On 21 September 2015 at 15:46, Daniel Gutson
> <daniel.gutson@tallertechnologies.com> wrote:
>>
>> FWIW, we could make this plugin in 2 weeks (w already have static
>> checkers as plugins for our customers). I understand Nathan that you
>> may have some deadlines, but if we could have the opportunity to
>> implement it, we could accomplish a clean isolation of a particular
>> business needs (despite I acknowledge that a warning about virtual
>> inheritance may be useful for a broader audience). OTOH, a plugin can
>> receive arguments, such as a configuration file which could point to
>> specific sources or hints about where to apply the warning, or a
>> suppression file, both things useful for large legacy code. IMVHO I
>> think this is a superior solution.
>> Please let me know if we could collaborate to get both a better gcc
>> and a better static checker.
>
> My opinion is that if people want to "donate" their plugins to the
> FSF, they are willing to maintain them, and they are not overtly
> complex or require any third-party software, they should be added to
> the GCC repository and build/tested. They could serve as examples and
> extra testing for the plugin framework, with the expectation that they
> may get removed if they become unmaintained.

Absolutely.

Jason

Re: [C++] Coding rule enforcement

[Daniel Gutson]

On Mon, Sep 21, 2015 at 1:23 PM, Jason Merrill <jason@redhat.com> wrote:
> On 09/21/2015 10:01 AM, Manuel López-Ibáñez wrote:
>>
>> On 21 September 2015 at 15:46, Daniel Gutson
>> <daniel.gutson@tallertechnologies.com> wrote:
>>>
>>>
>>> FWIW, we could make this plugin in 2 weeks (w already have static
>>> checkers as plugins for our customers). I understand Nathan that you
>>> may have some deadlines, but if we could have the opportunity to
>>> implement it, we could accomplish a clean isolation of a particular
>>> business needs (despite I acknowledge that a warning about virtual
>>> inheritance may be useful for a broader audience). OTOH, a plugin can
>>> receive arguments, such as a configuration file which could point to
>>> specific sources or hints about where to apply the warning, or a
>>> suppression file, both things useful for large legacy code. IMVHO I
>>> think this is a superior solution.
>>> Please let me know if we could collaborate to get both a better gcc
>>> and a better static checker.
>>
>>
>> My opinion is that if people want to "donate" their plugins to the
>> FSF, they are willing to maintain them, and they are not overtly
>> complex or require any third-party software, they should be added to
>> the GCC repository and build/tested. They could serve as examples and
>> extra testing for the plugin framework, with the expectation that they
>> may get removed if they become unmaintained.
>
>
> Absolutely.

Three of those plugins are already available in bitbucket. Maybe we
should move them to the official plugins page.
The fourth plugin is under development but I asked for volunteers to
test it in ths C++' std-proposals forum,
since it enhances the semantic of constexpr in order to allocate
memory during constexpr construction and
therefore having associative containers in ROM, so this is a special
case where the main intent is a proof of
concept for a C++ proposal.

>
> Jason
>
>



-- 

Daniel F. Gutson
Chief Engineering Officer, SPD

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina

Phone:   +54 351 4217888 / +54 351 4218211
Skype:    dgutson
LinkedIn: http://ar.linkedin.com/in/danielgutson

Re: [C++] Coding rule enforcement

[Nathan Sidwell]

On 09/21/15 12:23, Jason Merrill wrote:
> On 09/21/2015 10:01 AM, Manuel López-Ibáñez wrote:
>> On 21 September 2015 at 15:46, Daniel Gutson
>> <daniel.gutson@tallertechnologies.com> wrote:
>>>
>>> FWIW, we could make this plugin in 2 weeks (w already have static
>>> checkers as plugins for our customers). I understand Nathan that you
>>> may have some deadlines, but if we could have the opportunity to
>>> implement it, we could accomplish a clean isolation of a particular
>>> business needs (despite I acknowledge that a warning about virtual
>>> inheritance may be useful for a broader audience). OTOH, a plugin can
>>> receive arguments, such as a configuration file which could point to
>>> specific sources or hints about where to apply the warning, or a
>>> suppression file, both things useful for large legacy code. IMVHO I
>>> think this is a superior solution.
>>> Please let me know if we could collaborate to get both a better gcc
>>> and a better static checker.
>>
>> My opinion is that if people want to "donate" their plugins to the
>> FSF, they are willing to maintain them, and they are not overtly
>> complex or require any third-party software, they should be added to
>> the GCC repository and build/tested. They could serve as examples and
>> extra testing for the plugin framework, with the expectation that they
>> may get removed if they become unmaintained.
>
> Absolutely.

I've no objection to going the plugin way, but I'm extremely unlikely to be able 
to devote time to doing that in the foreseeable future.

nathan

Re: [C++] Coding rule enforcement

[Jason Merrill]

On 09/21/2015 12:39 PM, Nathan Sidwell wrote:
> On 09/21/15 12:23, Jason Merrill wrote:
>> On 09/21/2015 10:01 AM, Manuel López-Ibáñez wrote:
>>> On 21 September 2015 at 15:46, Daniel Gutson
>>> <daniel.gutson@tallertechnologies.com> wrote:
>>>>
>>>> FWIW, we could make this plugin in 2 weeks (w already have static
>>>> checkers as plugins for our customers). I understand Nathan that you
>>>> may have some deadlines, but if we could have the opportunity to
>>>> implement it, we could accomplish a clean isolation of a particular
>>>> business needs (despite I acknowledge that a warning about virtual
>>>> inheritance may be useful for a broader audience). OTOH, a plugin can
>>>> receive arguments, such as a configuration file which could point to
>>>> specific sources or hints about where to apply the warning, or a
>>>> suppression file, both things useful for large legacy code. IMVHO I
>>>> think this is a superior solution.
>>>> Please let me know if we could collaborate to get both a better gcc
>>>> and a better static checker.
>>>
>>> My opinion is that if people want to "donate" their plugins to the
>>> FSF, they are willing to maintain them, and they are not overtly
>>> complex or require any third-party software, they should be added to
>>> the GCC repository and build/tested. They could serve as examples and
>>> extra testing for the plugin framework, with the expectation that they
>>> may get removed if they become unmaintained.
>>
>> Absolutely.
>
> I've no objection to going the plugin way, but I'm extremely unlikely to
> be able to devote time to doing that in the foreseeable future.

I like the idea of the plugin, but your patch is very clean, so I think 
let's go ahead and put it in.

Jason

Re: [C++] Coding rule enforcement

[Ville Voutilainen]

>>>+      else if (warn_multiple_inheritance)
>>>+    warning (OPT_Wmultiple_inheritance,
>>>+         "%qT defined with multiple direct bases", ref);
>>You don't need to guard the warning with a check of the warning flag; warning
>>will only give the warning if the option is enabled.
>the spelling mistake on the option I used to experiment on didn't help, but I >discovered one must have the Var clause in the options file too, which I didn't expect.
>Anyway, fixed now.  ok?

Note that there are a couple of places where 'namespace' is misspelled
as 'namepace' in the patch.