[wwwdocs] Update coding conventions for C++

On 6/25/12, Lawrence Crowl <crowl@google.com> wrote:
> On 6/25/12, Joseph S. Myers <joseph@codesourcery.com> wrote:
>> On Mon, 25 Jun 2012, Diego Novillo wrote:
>> > [ Added doc maintainers in CC ]
>> >
>> > While I'm not particularly interested in the details of the
>> > coding conventions, I am interested in getting them in getting
>> > them installed before we merge cxx-conversion to trunk.
>> >
>> > Joseph, Gerald, do we have a process for accepting changes to
>> > coding conventions?
>>
>> I suggest sending smaller (so easier to review) patches that are
>> concrete and avoid speculation about possible future changes.
>
> The speculative parts come from the wiki.  Unless I had specific
> reason to remove them, I left them.
>
>> > > +&nbsp;&nbsp;&nbsp;&nbsp;<a href="#C_Options">Compiler Options</a><br
>> > > />
>>
>> I don't like this repeated &nbsp; presentational markup; use
>> appropriate logical markup instead.
>
> Done.
>
>> > > +<p>
>> > > +We will periodically pick a stable version of GCC,
>> > > +and require that that version of GCC be able to build
>> > > +all versions of GCC up to and including the next stable version.
>> > > +E.g., we may decide that all newer versions of GCC
>> > > +should be buildable with GCC 4.3.5.
>>
>> The current version required is more important than speculation
>> about future changes.  E.g., say that code must build with GCC
>> 4.1 and later.
>
> After some off-line discussion with Diego, I have removed some of
> text in this area.  It now focuses on being portable, rather than
> on arbitrary version numbers.
>
>> > > +<h4><a name="Assertions">Assertions</a></h4>
>> > > +
>> > > +<p>Code should use <code>gcc_assert(EXPR)</code> to check
>> > > invariants.
>> > > +Use <code>gcc_unreachable()</code> to mark places that should never
>> > > be
>> > > +reachable (such as an unreachable <code>default</code> case of a
>> > > +switch).  Do not use <code>gcc_assert(0)</code> for such purposes,
>> > > as
>> > > +<code>gcc_unreachable</code> gives the compiler more information.
>> > > The
>> > > +assertions are enabled unless explicitly configured off with
>> > > +<code>--enable-checking=none</code>.  Do not use <code>abort</code>.
>> > > +User input should never be validated by either
>> > > <code>gcc_assert</code>
>> > > +or <code>gcc_unreachable</code>.  If the checks are expensive or the
>> > > +compiler can reasonably carry on after the error, they may be
>> > > +conditioned on <code>--enable-checking</code>.</p>
>>
>> I don't think any of this is new - perhaps you could separate
>> rearrangement from the actual changes and C++ conventions?
>
> The section titles are new, but the text itself is not.  There did
> not seem to be a reasonable way to separate the rearrangment from
> the new context in which it was placed.
>
>> > > +<h4><a name="Calls">Function Calls</a></h4>
>> > > +
>> > > +<p>
>> > > +All current GCC code uses a space between the function name
>> > > +and the left parenthesis in a function call.
>> > > +Essentially all C++ code omits that space,
>> > > +which is more consistent with C++ syntax.
>> > > +For the present we will retain the space.
>> > > +It is possible that in the future we will switch the code with a flag
>> > > day.
>> > > +</p>
>>
>> Avoid speculation.  Avoid duplicating the GNU Coding Standards.
>> Formatting can be assumed to follow the GNU Coding Standards
>> except where they are meaningless for C++ or something explicitly
>> overrides them.
>
> That paragraph came from the wiki.  I have removed it.
>
>> > > +<p>
>> > > +Think carefully about the size and performance impact
>> > > +of virtual functions and virtual bases
>> > > +before using them.
>> > > +</p>
>>
>> Explain in more detail what the size and performance impact is
>> for people not familiar with the details of how C++ features
>> are implemented.
>
> I have added a bit more in the rationale, reached through the link
> at the end of that section.
>
>> > > +<p>
>> > > +Indent protection labels by one space.
>> > > +</p>
>> > > +
>> > > +<p>
>> > > +Indent class members by two spaces.
>>
>> Do all the listed indentation rules correspond to what a <TAB>
>> will do by default when editing C++ code in GNU Emacs?  If not,
>> we have conflicting notions of GNU C++ indentation conventions.
>
> I have no idea.  I don't use emacs.  The two-space rule for members
> comes from the wiki.  The one-space rule for protection labels is
> common practice.  If folks want something else, changes are fine
> with me.
>
> I have also made a few other edits requested offline by Benjamin
> Kosnik.
>
> --
> Lawrence Crowl

Sorry, I forgot to add the patch.  Note that I have removed some
html headers that cause gcc mail grief.

Hey Lawrence, thanks for this work and for keeping the public
up-to-date with this gcc-patches posting.

This looks pretty good to me. 

The only remaining issue for me is the
fuzzyness/handwaving around inlining. I think the only way to really
enforce "what can be inlined" is not to have people use their best
judgement, or what they think is a small function, or what they
intend for the compiler to inline, but to have -Winline mandatory. And
thus flag things that are written as inlined, but cannot then be
inlined by the compiler. 

This area seems historically fraught, in terms of expectations. I'm not
trying to poke at the hornets nest here.

-benjamin

On 6/25/12, Benjamin Kosnik <bkoz@redhat.com> wrote:
> The only remaining issue for me is the fuzzyness/handwaving
> around inlining.  I think the only way to really enforce "what
> can be inlined" is not to have people use their best judgement,
> or what they think is a small function, or what they intend
> for the compiler to inline, but to have -Winline mandatory.
> And thus flag things that are written as inlined, but cannot then
> be inlined by the compiler.
>
> This area seems historically fraught, in terms of expectations.
> I'm not trying to poke at the hornets nest here.

I see two problems with trying to use -Winline.

First, the implication is that if the compiler does not complain with
-Winline, then the inlining is fine.  The compiler only warns when
it cannot inline, not when that inlining is counter productive.  That
is, we want inlining when we win in either space or time, an -Winline
does not tell us that.  So, in a sense, the warning is too weak.

Second, together -Winline and -Werror lead to a tempermental build.
The documentation for -Winline says:

  Warn if a function can not be inlined and it was declared as
  inline. Even with this option, the compiler will not warn about
  failures to inline functions declared in system headers.

  The compiler uses a variety of heuristics to determine whether
  or not to inline a function. For example, the compiler takes
  into account the size of the function being inlined and the
  amount of inlining that has already been done in the current
  function. Therefore, seemingly insignificant changes in the
  source program can cause the warnings produced by -Winline to
  appear or disappear.

IIUC, a small change to a function in a header file could cause
a hundred other functions to suddenly start emitting the warning,
causing the edit of files that would not normally be considered part
of the patch.  In the worst case, these functions would shift from
header to source file, causing even more source churn.  Even more
disconcerting, the warning might not show up until later in the boot.

If everyone keeps their inlines small and clean, maybe the second
problem will not be so much of a problem.  My concern is that writing
it into the coding conventions shifts the practical consequences
to places that are hard to anticipate and hard to deal with.
For instance, suppose I inline foo() to meet performance goals,
but in doing so bar() becomes to large to inline.  Was bar() too
performance critical to make non-inline?  If so, how do I best
refactor bar()?  Will I need another maintainer to review the code
because of that refactoring?

With that concern stated, I will write into the conventions whatever
concensus arises.

Of course, I have no objection to an occasional inline cleanup.
That is, build with -Werror and adjusting inlines that have,
through the course of time, become larger than is useful.

On Jun 25, 2012, Lawrence Crowl <crowl@google.com> wrote:

> +These conventions will change over time,
> +but changing them requires that a convincing rationale.

s/that//

> +Complex heirarchies are to be avoided.

s/heir/hier/

Hi,

On Mon, Jun 25, 2012 at 03:26:01PM -0700, Lawrence Crowl wrote:
...
> > I have no idea.  I don't use emacs.  The two-space rule for members
> > comes from the wiki.  The one-space rule for protection labels is
> > common practice.  If folks want something else, changes are fine
> > with me.

I'll also need an emacs C++ indentation style that conforms to this in
order to really be able to produce complying code myself.  So if
anybody else will be working on that, I'm interested (to use it and
perhaps help crafting it) and I guess a number of other people on this
list are too...

Furthermore, even though this mainly reminds me how little I actually
know about C++, I have a few general tiny comments:

> -<h2>Miscellaneous Conventions</h2>
> -
> -<p>Code should use <code>gcc_assert(EXPR)</code> to check invariants.
> -Use <code>gcc_unreachable()</code> to mark places that should never be
> -reachable (such as an unreachable <code>default</code> case of a
> -switch).  Do not use <code>gcc_assert(0)</code> for such purposes, as
> -<code>gcc_unreachable</code> gives the compiler more information.  The
> -assertions are enabled unless explicitly configured off with
> -<code>--enable-checking=none</code>.  Do not use <code>abort</code>.
> -User input should never be validated by either <code>gcc_assert</code>
> -or <code>gcc_unreachable</code>.  If the checks are expensive or the
> -compiler can reasonably carry on after the error, they may be
> -conditioned on <code>--enable-checking</code>.</p>

It seems we should mention gcc_checking_assert here then.

...

> +<p>
> +Single inheritance is permitted.
> +Use public inheritance to describe interface inheritance,
> +i.e. 'is-a' relationships.
> +Use private and protected inheritance
> +to describe implementation inheritance.
> +Implementation inheritance can be expediant,

s/expediant/expedient/

> +but think twice before using it in code
> +intended to last a long time.

I think all committed code should be expected to have long-lasting
quality.  I would not encourage people to think otherwise and would
drop the "long time" reference here.  If anybody ever commits
something ugly to bridge a short time period, it should only be done
under the "maintainers grant exceptions" rule anyway.

...

> +</p>> +<p>
> +For long-term code, at least for now,
> +we will continue to use <code>printf</code> style I/O
> +rather than <code>&lt;iostream&gt;</code> style I/O.
> +For quick debugging code,
> +<code>&lt;iostream&gt;</code> is permitted.
> +</p>

Similarly here, no quick and dirty debugging output should ever be
committed, we should not 

> +<h4><a name="stdlib">The Standard Library</a></h4>
> +
> +<p>
> +At present, C++ provides no great advantage for i18n.
> +GCC does type checking for <code>printf</code> arguments,
> +so the type insecurity of <code>printf</code> is moot,
> +but the clarity in layout persists.
> +For quick debugging output, &lt;iostream&gt; requires less work.
> +</p>

The same applies here.

Thanks,

Martin


> +
> +<h3>Formatting Conventions</h3>
> +
> +<h4><a href="names">Names</a></h4>
> +
> +<p>
> +Naming data members with a trailing underscore
> +highlights the extra overhead of access to fields over local variables.
> +Think of the trailing underscore
> +like you would Pascal's postfix <code>^</code> dereference operator.
> +</p>
> +
> +<p>
> +When using the above convention,
> +the constructor parameter names
> +and getter member function names
> +can use the more concise non-underscore form.
> +</p>
> +
> +</body>
> +</html>
> 
> -- 
> Lawrence Crowl

On 06/25/2012 06:26 PM, Lawrence Crowl wrote:
> +or<code>gcc_unreachable</code>.  If the checks are expensive or the
> +compiler can reasonably carry on after the error, they may be
> +conditioned on<code>--enable-checking</code>.</p>

by using <code>gcc_checking_assert</code>

[Rationale]
> +FIXME: Discussion of deleting inappropraite special members.

Is this FIXME still needed?  The text following it seems to cover the 
issue well enough.

> +However, by default, RTTI is not permitted
> +and the compiler must build cleanly with <code>-fno-rtti</code>.

This seems like an unnecessary restriction to me.

> +Disabling RTTI will save space in the compiler.

This is a fine reason to disable RTTI if it isn't used, but doesn't seem 
like a strong reason to prohibit using it.  The information is only 
emitted for classes with virtual functions, isn't very large, is 
typically emitted in only one object file, and since it lives in .rodata 
it can be shared between multiple compiler processes.

> +Checking the type of a class at runtime usually indicates a design problem.

The tree_contains_struct machinery recently added to GCC maps directly 
onto dynamic_cast;

if (CODE_CONTAINS_STRUCT(TREE_CODE(t),TS_DECL_WRTL))
   /* do something with t->decl_with_rtl */

translates roughly to to

if (decl_with_rtl *p = dynamic_cast <decl_with_rtl *>(t))
   /* do something with p */

When new interfaces are added partway down an inheritance tree, 
dynamic_cast is the right way to access them.  This isn't checking what 
the type is, it's checking whether the object supports a particular method.

The fact that we've gone to the trouble to implement this functionality 
in C suggests to me that using it isn't always indicative of a design 
problem.  :)

> +If you need to know the type of a class for some other reason,
> +use an enum or a virtual member function
> +that coverts a pointer to the more derived class.
> +For example,
> +</p>
> +
> +<blockquote><pre><code>
> +common_type *p = ....;
> +if (specific_type *q = p-&gt;to_specific ()) {
> +  // We have and can use a specific_type pointed to by q.
> +}
> +</code></pre></blockquote>

This is basically equivalent to dynamic_cast except that you need to 
clutter up your root base class with one virtual function for each 
derived class you want to be able to convert to.

Jason

> With that concern stated, I will write into the conventions whatever
> concensus arises.
> 
> Of course, I have no objection to an occasional inline cleanup.
> That is, build with -Werror and adjusting inlines that have,
> through the course of time, become larger than is useful.

This seems to be what reliably happens in libstdc++-land. 

-benjamin

On 6/25/12, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Jun 25, 2012, Lawrence Crowl <crowl@google.com> wrote:
>> +These conventions will change over time,
>> +but changing them requires that a convincing rationale.
>
> s/that//
>
>> +Complex heirarchies are to be avoided.
>
> s/heir/hier/

Both fixed.

On 6/26/12, Martin Jambor <mjambor@suse.cz> wrote:
> On Mon, Jun 25, 2012 at 03:26:01PM -0700, Lawrence Crowl wrote:
> > > I have no idea.  I don't use emacs.  The two-space rule for
> > > members comes from the wiki.  The one-space rule for protection
> > > labels is common practice.  If folks want something else,
> > > changes are fine with me.
>
> I'll also need an emacs C++ indentation style that conforms to
> this in order to really be able to produce complying code myself.
> So if anybody else will be working on that, I'm interested (to
> use it and perhaps help crafting it) and I guess a number of
> other people on this list are too...

Alternatively, one could change the conventions to match an emacs
style.  Either is fine we me, as long as the style is reasonable.

> > -<h2>Miscellaneous Conventions</h2>
> > -
> > -<p>Code should use <code>gcc_assert(EXPR)</code> to check invariants.
> > -Use <code>gcc_unreachable()</code> to mark places that should never be
> > -reachable (such as an unreachable <code>default</code> case of a
> > -switch).  Do not use <code>gcc_assert(0)</code> for such purposes, as
> > -<code>gcc_unreachable</code> gives the compiler more information.  The
> > -assertions are enabled unless explicitly configured off with
> > -<code>--enable-checking=none</code>.  Do not use <code>abort</code>.
> > -User input should never be validated by either <code>gcc_assert</code>
> > -or <code>gcc_unreachable</code>.  If the checks are expensive or the
> > -compiler can reasonably carry on after the error, they may be
> > -conditioned on <code>--enable-checking</code>.</p>
>
> It seems we should mention gcc_checking_assert here then.

Jason had a suggestion, and I used that.

> > +<p>
> > +Single inheritance is permitted.
> > +Use public inheritance to describe interface inheritance,
> > +i.e. 'is-a' relationships.
> > +Use private and protected inheritance
> > +to describe implementation inheritance.
> > +Implementation inheritance can be expediant,

> s/expediant/expedient/

Fixed.

> > +but think twice before using it in code
> > +intended to last a long time.
>
> I think all committed code should be expected to have long-lasting
> quality.  I would not encourage people to think otherwise and would
> drop the "long time" reference here.  If anybody ever commits
> something ugly to bridge a short time period, it should only be done
> under the "maintainers grant exceptions" rule anyway.
>
> > +</p>> +<p>
> > +For long-term code, at least for now,
> > +we will continue to use <code>printf</code> style I/O
> > +rather than <code>&lt;iostream&gt;</code> style I/O.
> > +For quick debugging code,
> > +<code>&lt;iostream&gt;</code> is permitted.
> > +</p>
>
> Similarly here, no quick and dirty debugging output should ever be
> committed, we should not
>
> > +<h4><a name="stdlib">The Standard Library</a></h4>
> > +
> > +<p>
> > +At present, C++ provides no great advantage for i18n.
> > +GCC does type checking for <code>printf</code> arguments,
> > +so the type insecurity of <code>printf</code> is moot,
> > +but the clarity in layout persists.
> > +For quick debugging output, &lt;iostream&gt; requires less work.
> > +</p>
>
> The same applies here.

The value of these changes depends on when the rules are enforced.
If they are enforced only on trunk, then the changes seem fine
to me.  However, if they are enforced on branches, then they could
unnecessarily slow down development.

Comments?

On 6/26/12, Jason Merrill <jason@redhat.com> wrote:
> On 06/25/2012 06:26 PM, Lawrence Crowl wrote:
> > +or<code>gcc_unreachable</code>.  If the checks are expensive or the
> > +compiler can reasonably carry on after the error, they may be
> > +conditioned on<code>--enable-checking</code>.</p>
>
> by using <code>gcc_checking_assert</code>

I inserted that suggestion, but note that I did not create that text,
only moved it.

> [Rationale]
> > +FIXME: Discussion of deleting inappropraite special members.
>
> Is this FIXME still needed?  The text following it seems to cover the
> issue well enough.

No longer needed.

> > +However, by default, RTTI is not permitted
> > +and the compiler must build cleanly with <code>-fno-rtti</code>.
>
> This seems like an unnecessary restriction to me.
>
> > +Disabling RTTI will save space in the compiler.
>
> This is a fine reason to disable RTTI if it isn't used, but doesn't
> seem like a strong reason to prohibit using it.  The information is
> only emitted for classes with virtual functions, isn't very large,
> is typically emitted in only one object file, and since it lives
> in .rodata it can be shared between multiple compiler processes.
>
> > +Checking the type of a class at runtime usually indicates a design problem.
>
> The tree_contains_struct machinery recently added to GCC maps
> directly onto dynamic_cast;
>
> if (CODE_CONTAINS_STRUCT(TREE_CODE(t),TS_DECL_WRTL))
>   /* do something with t->decl_with_rtl */
>
> translates roughly to to
>
> if (decl_with_rtl *p = dynamic_cast <decl_with_rtl *>(t))
>   /* do something with p */
>
> When new interfaces are added partway down an inheritance tree,
> dynamic_cast is the right way to access them.  This isn't checking
> what the type is, it's checking whether the object supports a
> particular method.
>
> The fact that we've gone to the trouble to implement this
> functionality in C suggests to me that using it isn't always
> indicative of a design problem.  :)

Personally, I am fine with using dynamic_cast.  I was writing up what
I thought was a consensus on the wiki.  I can live without it, or I
can use it profitably.  Whatever you all decide, I will write up.

> > +If you need to know the type of a class for some other reason,
> > +use an enum or a virtual member function
> > +that coverts a pointer to the more derived class.
> > +For example,
> > +</p>
> > +
> > +<blockquote><pre><code>
> > +common_type *p = ....;
> > +if (specific_type *q = p-&gt;to_specific ()) {
> > +  // We have and can use a specific_type pointed to by q.
> > +}
> > +</code></pre></blockquote>
>
> This is basically equivalent to dynamic_cast except that you need
> to clutter up your root base class with one virtual function for
> each derived class you want to be able to convert to.

Note quite equivalent, as you can implement to_specific with
non-virtual classes using the TREE_CODE.  Thus would could implement
a type-save dynamic pointer converter before converting to virtual
classes.

OTOH, we could probably work up a template function that looks like
dynamic_cast but uses the TREE_CODE instead, achieving the same
intermediate step.

I agree that it does clutter up the base class.

Shall we enable RTTI?

Lawrence Crowl <crowl@google.com> writes:

[...]

| Shall we enable RTTI?

Benjamin made compelling arguments for doing this.  And I agree.

-- Gaby

On Tue, 26 Jun 2012, Lawrence Crowl wrote:

> Shall we enable RTTI?

I think any proposal for adding a new C++ feature to the standards should 
go in its own thread on the gcc list with a meaningful subject, not this 
thread which should simply be about integrating uncontroversial standards 
in the existing documents.

We should start by documenting standards that are a conservative and 
uncontroversial extension of the C coding standards currently used - if we 
need to ask the question about whether something should be allowed, it 
should not be allowed in the initial standards coming out of this general 
thread.  Then, to add individual features to the list we can discuss those 
features and standards for them separately.

On 06/26/2012 02:47 PM, Joseph S. Myers wrote:
> We should start by documenting standards that are a conservative and
> uncontroversial extension of the C coding standards currently used - if we
> need to ask the question about whether something should be allowed, it
> should not be allowed in the initial standards coming out of this general
> thread.

Sure; I don't mind initially disabling RTTI with a note that someone 
wanting to use it is free to propose adding it to the coding standards. 
  I was mainly objecting to the rationale.

Jason

On Jun 26, 2012, at 11:06 AM, Lawrence Crowl wrote:
> Alternatively, one could change the conventions to match an emacs
> style.  Either is fine we me, as long as the style is reasonable.

It is really nice to be able to use 5 year old emacsen out of the box on the source tree and have it just work.  I'd vote for that...

On Wed, Jun 27, 2012 at 2:19 AM, Lawrence Crowl <crowl@google.com> wrote:
> On 6/26/12, Jason Merrill <jason@redhat.com> wrote:
>> On 06/25/2012 06:26 PM, Lawrence Crowl wrote:
>> > +or<code>gcc_unreachable</code>.  If the checks are expensive or the
>> > +compiler can reasonably carry on after the error, they may be
>> > +conditioned on<code>--enable-checking</code>.</p>
>>
>> by using <code>gcc_checking_assert</code>
>
> I inserted that suggestion, but note that I did not create that text,
> only moved it.
>
>> [Rationale]
>> > +FIXME: Discussion of deleting inappropraite special members.
>>
>> Is this FIXME still needed?  The text following it seems to cover the
>> issue well enough.
>
> No longer needed.
>
>> > +However, by default, RTTI is not permitted
>> > +and the compiler must build cleanly with <code>-fno-rtti</code>.
>>
>> This seems like an unnecessary restriction to me.
>>
>> > +Disabling RTTI will save space in the compiler.
>>
>> This is a fine reason to disable RTTI if it isn't used, but doesn't
>> seem like a strong reason to prohibit using it.  The information is
>> only emitted for classes with virtual functions, isn't very large,
>> is typically emitted in only one object file, and since it lives
>> in .rodata it can be shared between multiple compiler processes.
>>
>> > +Checking the type of a class at runtime usually indicates a design problem.
>>
>> The tree_contains_struct machinery recently added to GCC maps
>> directly onto dynamic_cast;
>>
>> if (CODE_CONTAINS_STRUCT(TREE_CODE(t),TS_DECL_WRTL))
>>   /* do something with t->decl_with_rtl */
>>
>> translates roughly to to
>>
>> if (decl_with_rtl *p = dynamic_cast <decl_with_rtl *>(t))
>>   /* do something with p */
>>
>> When new interfaces are added partway down an inheritance tree,
>> dynamic_cast is the right way to access them.  This isn't checking
>> what the type is, it's checking whether the object supports a
>> particular method.
>>
>> The fact that we've gone to the trouble to implement this
>> functionality in C suggests to me that using it isn't always
>> indicative of a design problem.  :)
>
> Personally, I am fine with using dynamic_cast.  I was writing up what
> I thought was a consensus on the wiki.  I can live without it, or I
> can use it profitably.  Whatever you all decide, I will write up.
>

dynamic_cast use RTTI, while TREE_CODE are poor man's type info. RTTI
is better than TREE_CODE. But, If you decide to use RTTI,  TREE_CODE
become redundant, that means all use of TREE_CODE should be removed,
sooner or later. Are you prepared for that ?

If every types in the type hierarchy, not just the root type and
leaves types,  has a TREE_CODE,  and you maintain a database of
inheritance relationship and other info of types of different
TREE_CODEs,  then,  for a object of a given TREE_CODE,  you know its
TREE_CODE and type info,   you know whether or not the object can be
cast to another type,  and,  you can know other info about the type,
like the size of object of the type, string name of the type.

This can be implemented as below. For every type in the type
hierarchy, static define a object of meta type( meta class, or class
class) to describe the type info of the specific type for a given
TREE_CODE. All of the meta type objects of different TREE_CODEs are
organized as an array, which is indexed by the value of TREE_CODE. And
you provide a set of C functions , that judge whether a object that
has a given TREE_CODE can be cast to a type that has another TREE_CODE
and retrieve the type info of a object that has a given TREE_CODE.


>> > +If you need to know the type of a class for some other reason,
>> > +use an enum or a virtual member function
>> > +that coverts a pointer to the more derived class.
>> > +For example,
>> > +</p>
>> > +
>> > +<blockquote><pre><code>
>> > +common_type *p = ....;
>> > +if (specific_type *q = p-&gt;to_specific ()) {
>> > +  // We have and can use a specific_type pointed to by q.
>> > +}
>> > +</code></pre></blockquote>
>>
>> This is basically equivalent to dynamic_cast except that you need
>> to clutter up your root base class with one virtual function for
>> each derived class you want to be able to convert to.
>
> Note quite equivalent, as you can implement to_specific with
> non-virtual classes using the TREE_CODE.  Thus would could implement
> a type-save dynamic pointer converter before converting to virtual
> classes.
>
> OTOH, we could probably work up a template function that looks like
> dynamic_cast but uses the TREE_CODE instead, achieving the same
> intermediate step.
>
> I agree that it does clutter up the base class.
>

Template function may be not necessary.
And, virtual method is not necessary.
Just normal C functions can work if you have the type info of a given TREE_CODE.

> Shall we enable RTTI?
Are you prepared to remove all use of TREE_CODE ?

On Wed, Jun 27, 2012 at 2:35 PM, Chiheng Xu <chiheng.xu@gmail.com> wrote:
> On Wed, Jun 27, 2012 at 2:19 AM, Lawrence Crowl <crowl@google.com> wrote:
>> On 6/26/12, Jason Merrill <jason@redhat.com> wrote:
>>> On 06/25/2012 06:26 PM, Lawrence Crowl wrote:
>>> > +or<code>gcc_unreachable</code>.  If the checks are expensive or the
>>> > +compiler can reasonably carry on after the error, they may be
>>> > +conditioned on<code>--enable-checking</code>.</p>
>>>
>>> by using <code>gcc_checking_assert</code>
>>
>> I inserted that suggestion, but note that I did not create that text,
>> only moved it.
>>
>>> [Rationale]
>>> > +FIXME: Discussion of deleting inappropraite special members.
>>>
>>> Is this FIXME still needed?  The text following it seems to cover the
>>> issue well enough.
>>
>> No longer needed.
>>
>>> > +However, by default, RTTI is not permitted
>>> > +and the compiler must build cleanly with <code>-fno-rtti</code>.
>>>
>>> This seems like an unnecessary restriction to me.
>>>
>>> > +Disabling RTTI will save space in the compiler.
>>>
>>> This is a fine reason to disable RTTI if it isn't used, but doesn't
>>> seem like a strong reason to prohibit using it.  The information is
>>> only emitted for classes with virtual functions, isn't very large,
>>> is typically emitted in only one object file, and since it lives
>>> in .rodata it can be shared between multiple compiler processes.
>>>
>>> > +Checking the type of a class at runtime usually indicates a design problem.
>>>
>>> The tree_contains_struct machinery recently added to GCC maps
>>> directly onto dynamic_cast;
>>>
>>> if (CODE_CONTAINS_STRUCT(TREE_CODE(t),TS_DECL_WRTL))
>>>   /* do something with t->decl_with_rtl */
>>>
>>> translates roughly to to
>>>
>>> if (decl_with_rtl *p = dynamic_cast <decl_with_rtl *>(t))
>>>   /* do something with p */
>>>
>>> When new interfaces are added partway down an inheritance tree,
>>> dynamic_cast is the right way to access them.  This isn't checking
>>> what the type is, it's checking whether the object supports a
>>> particular method.
>>>
>>> The fact that we've gone to the trouble to implement this
>>> functionality in C suggests to me that using it isn't always
>>> indicative of a design problem.  :)
>>
>> Personally, I am fine with using dynamic_cast.  I was writing up what
>> I thought was a consensus on the wiki.  I can live without it, or I
>> can use it profitably.  Whatever you all decide, I will write up.
>>
>
> dynamic_cast use RTTI, while TREE_CODE are poor man's type info. RTTI
> is better than TREE_CODE.

RTTI requires more space than TREE_CODE, so it's not universally better.

> But, If you decide to use RTTI,  TREE_CODE
> become redundant, that means all use of TREE_CODE should be removed,
> sooner or later. Are you prepared for that ?
>
> If every types in the type hierarchy, not just the root type and
> leaves types,  has a TREE_CODE,  and you maintain a database of
> inheritance relationship and other info of types of different
> TREE_CODEs,  then,  for a object of a given TREE_CODE,  you know its
> TREE_CODE and type info,   you know whether or not the object can be
> cast to another type,  and,  you can know other info about the type,
> like the size of object of the type, string name of the type.
>
> This can be implemented as below. For every type in the type
> hierarchy, static define a object of meta type( meta class, or class
> class) to describe the type info of the specific type for a given
> TREE_CODE. All of the meta type objects of different TREE_CODEs are
> organized as an array, which is indexed by the value of TREE_CODE. And
> you provide a set of C functions , that judge whether a object that
> has a given TREE_CODE can be cast to a type that has another TREE_CODE
> and retrieve the type info of a object that has a given TREE_CODE.

We already have all (or most) of this.

>>> > +If you need to know the type of a class for some other reason,
>>> > +use an enum or a virtual member function
>>> > +that coverts a pointer to the more derived class.
>>> > +For example,
>>> > +</p>
>>> > +
>>> > +<blockquote><pre><code>
>>> > +common_type *p = ....;
>>> > +if (specific_type *q = p-&gt;to_specific ()) {
>>> > +  // We have and can use a specific_type pointed to by q.
>>> > +}
>>> > +</code></pre></blockquote>
>>>
>>> This is basically equivalent to dynamic_cast except that you need
>>> to clutter up your root base class with one virtual function for
>>> each derived class you want to be able to convert to.
>>
>> Note quite equivalent, as you can implement to_specific with
>> non-virtual classes using the TREE_CODE.  Thus would could implement
>> a type-save dynamic pointer converter before converting to virtual
>> classes.
>>
>> OTOH, we could probably work up a template function that looks like
>> dynamic_cast but uses the TREE_CODE instead, achieving the same
>> intermediate step.
>>
>> I agree that it does clutter up the base class.
>>
>
> Template function may be not necessary.
> And, virtual method is not necessary.
> Just normal C functions can work if you have the type info of a given TREE_CODE.
>
>> Shall we enable RTTI?
> Are you prepared to remove all use of TREE_CODE ?

To answer, no - we should not enable RTTI (nor exceptions).

Richard.

Hi,

On Tue, Jun 26, 2012 at 11:06:15AM -0700, Lawrence Crowl wrote:
> On 6/26/12, Martin Jambor <mjambor@suse.cz> wrote:
> > On Mon, Jun 25, 2012 at 03:26:01PM -0700, Lawrence Crowl wrote:
> > > > I have no idea.  I don't use emacs.  The two-space rule for
> > > > members comes from the wiki.  The one-space rule for protection
> > > > labels is common practice.  If folks want something else,
> > > > changes are fine with me.
> >
> > I'll also need an emacs C++ indentation style that conforms to
> > this in order to really be able to produce complying code myself.
> > So if anybody else will be working on that, I'm interested (to
> > use it and perhaps help crafting it) and I guess a number of
> > other people on this list are too...
> 
> Alternatively, one could change the conventions to match an emacs
> style.  Either is fine we me, as long as the style is reasonable.

That would be very nice :-) 
Of course, if many people do not find it reasonable, I'm sure there is
an easy way to tweak it.

...

> > > +but think twice before using it in code
> > > +intended to last a long time.
> >
> > I think all committed code should be expected to have long-lasting
> > quality.  I would not encourage people to think otherwise and would
> > drop the "long time" reference here.  If anybody ever commits
> > something ugly to bridge a short time period, it should only be done
> > under the "maintainers grant exceptions" rule anyway.
> >
> > > +</p>> +<p>
> > > +For long-term code, at least for now,
> > > +we will continue to use <code>printf</code> style I/O
> > > +rather than <code>&lt;iostream&gt;</code> style I/O.
> > > +For quick debugging code,
> > > +<code>&lt;iostream&gt;</code> is permitted.
> > > +</p>
> >
> > Similarly here, no quick and dirty debugging output should ever be
> > committed, we should not
> >
> > > +<h4><a name="stdlib">The Standard Library</a></h4>
> > > +
> > > +<p>
> > > +At present, C++ provides no great advantage for i18n.
> > > +GCC does type checking for <code>printf</code> arguments,
> > > +so the type insecurity of <code>printf</code> is moot,
> > > +but the clarity in layout persists.
> > > +For quick debugging output, &lt;iostream&gt; requires less work.
> > > +</p>
> >
> > The same applies here.
> 
> The value of these changes depends on when the rules are enforced.
> If they are enforced only on trunk, then the changes seem fine
> to me.  However, if they are enforced on branches, then they could
> unnecessarily slow down development.
> 
> Comments?

I think that if you have a private branch, you are basically its
maintainer and can grant yourself any exception from any rule you
want.  Of course, that might make your life harder if you later want
to contribute the changes to the trunk, release branches, other
peple's branches and generally anywhere.

Thanks,

Martin

On 06/27/2012 08:35 AM, Chiheng Xu wrote:
> dynamic_cast use RTTI, while TREE_CODE are poor man's type info. RTTI
> is better than TREE_CODE. But, If you decide to use RTTI,  TREE_CODE
> become redundant, that means all use of TREE_CODE should be removed,
> sooner or later. Are you prepared for that ?

I wasn't suggesting we would change trees to use inheritance in the 
forseeable future; my point was that RTTI is used in patterns like what 
we already do with trees, so I don't think that using it indicates a 
design problem.

If we were to change trees to use inheritance and virtual functions, 
which seems unlikely to me, then I think it would make sense to use RTTI 
instead of TREE_CODE.

On 06/27/2012 09:02 AM, Richard Guenther wrote:
> RTTI requires more space than TREE_CODE, so it's not universally better.

RTTI requires no additional space in each object, and no space at all 
for classes with no virtual functions.

> To answer, no - we should not enable RTTI (nor exceptions).

The problem with exceptions is that the compiler is not exception-safe. 
  I don't think there's a good reason to prohibit RTTI.  As I said in 
another message, I don't mind turning it off for now, but I don't think 
doing so has much benefit either, since it only affects classes with 
virtual functions.

Jason

On 6/27/12, Martin Jambor <mjambor@suse.cz> wrote:
> On Tue, Jun 26, 2012 at 11:06:15AM -0700, Lawrence Crowl wrote:
> > On 6/26/12, Martin Jambor <mjambor@suse.cz> wrote:
> > > On Mon, Jun 25, 2012 at 03:26:01PM -0700, Lawrence Crowl wrote:
> > > > +but think twice before using it in code
> > > > +intended to last a long time.
> > >
> > > I think all committed code should be expected to have
> > > long-lasting quality.  I would not encourage people to think
> > > otherwise and would drop the "long time" reference here.
> > > If anybody ever commits something ugly to bridge a short time
> > > period, it should only be done under the "maintainers grant
> > > exceptions" rule anyway.
> > >
> > > > +</p>> +<p>
> > > > +For long-term code, at least for now,
> > > > +we will continue to use <code>printf</code> style I/O
> > > > +rather than <code>&lt;iostream&gt;</code> style I/O.
> > > > +For quick debugging code,
> > > > +<code>&lt;iostream&gt;</code> is permitted.
> > > > +</p>
> > >
> > > Similarly here, no quick and dirty debugging output should
> > > ever be committed, we should not
> > >
> > > > +<h4><a name="stdlib">The Standard Library</a></h4>
> > > > +
> > > > +<p>
> > > > +At present, C++ provides no great advantage for i18n.
> > > > +GCC does type checking for <code>printf</code> arguments,
> > > > +so the type insecurity of <code>printf</code> is moot,
> > > > +but the clarity in layout persists.
> > > > +For quick debugging output, &lt;iostream&gt; requires less work.
> > > > +</p>
> > >
> > > The same applies here.
> >
> > The value of these changes depends on when the rules are
> > enforced.  If they are enforced only on trunk, then the changes
> > seem fine to me.  However, if they are enforced on branches,
> > then they could unnecessarily slow down development.
> >
> > Comments?
>
> I think that if you have a private branch, you are basically
> its maintainer and can grant yourself any exception from any
> rule you want.  Of course, that might make your life harder if
> you later want to contribute the changes to the trunk, release
> branches, other peple's branches and generally anywhere.

I am not concerned about private branches, but public branches for
which sharing might be needed, but for which cleanup before going
into trunk or a release is reasonable.

C++ streams are much more convenient for free-form output than
C-based solutions.  Having said that, does anyone object to removing
the permission to use C++ streams?