Re: [PATCH 04/17] C frontend: capture BLT information

[Martin Sebor <msebor at gmail dot com>]

On 07/24/2017 02:05 PM, David Malcolm wrote:
> This patch extends the C frontend so that it optionally builds a BLT tree,
> by using an auto_blt_node class within the recursive descent through the
> parser, so that its ctor/dtors build the blt_node hierarchy; this is
> rapidly (I hope) rejected in the no -fblt case, so that by default, no
> blt nodes are created and it's (I hope) close to a no-op.
...
> @@ -4627,6 +4629,8 @@ start_decl (struct c_declarator *declarator, struct c_declspecs *declspecs,
>  			 deprecated_state);
>    if (!decl || decl == error_mark_node)
>      return NULL_TREE;
> +  if (declarator->bltnode)
> +    declarator->bltnode->set_tree (decl);

FWIW, if this checking were to become wide-spred, an alternative
to introducing conditionals like this is to set declarator->bltnode
to an object of a dummy type that doesn't do anything if -fblt isn't
specified, and to an object of the real "workhorse" type that does
the work with -fblt.  The object can either be polymorphic if the
cost of the virtual call isn't a concern, or it can be a static
wrapper around a polymorphic interface, where the (inline member
functions of the) static wrapper do the checking under the hood
so the users don't have to worry about it.  (A more C-like
alternative is to simply hide the check in an inline function.)

> +/* A RAII-style class for optionally building a concrete parse tree (or
> +   at least, something close to it) as the recursive descent parser runs.
> +
> +   Close to a no-op if -fblt is not selected.  */
> +
> +class auto_blt_node
> +{
> +public:
> +  auto_blt_node (c_parser* parser, enum blt_kind kind);
> +  ~auto_blt_node ();

Unless the class is safely copyable and assignable (I can't tell
from the dtor definition) I would suggest to define its copy ctor
and assignment operator private to avoid introducing subtle bugs
by inadvertently making copies.

Martin