Re: C++: enable-checking patches

["Martin v. Loewis" <martin at mira dot isdn dot cs dot tu-berlin dot de>]

> This is fine.

Thanks. I'll commit it once I get everything approved :-)

> 
>     Martin> 2. In warn_extern_redeclared_static, DECL_THIS_STATIC was
>     Martin> invoked for a TEMPLATE_DECL. Do not consider
>     Martin> TEMPLATE_DECLs in warn_extern_redeclared_static now.
> 
> Hmm.  Will we still catch:
>   
>   template <class T> extern void f();
>   template <class T> static void f();
> 
> ?

Yes. olddecl, in this function, will be

 <function_decl 0x40119a00 f
    type <function_type 0x40112780
        type <void_type 0x4010fb80 void
            permanent type_6 VOID
            size <integer_cst 0x4010e580 constant permanent 0>
            align 8 symtab 0 alias set -1
            pointer_to_this <pointer_type 0x4010fc00 __builtin_va_list>>
       
        permanent DI
        size <integer_cst 0x401114c0 constant permanent 64>
        align 32 symtab 0 alias set -1
        arg-types <tree_list 0x4010efa0
            unsigned permanent value <void_type 0x4010fb80 void>>>
   
    permanent public external decl_2 QI file a.cc line 1
   
    decl-main-variant 0x40119a00 template-info 0x4011a780 chain <type_decl 0x40119900 T>>

Notice how it has a template-info, which in turn has the template_decl.

>     Martin> 3. In warn_extern_redeclared_static, there was a check
>     Martin> that it is not a TEMPLATE_DECL, when the comment said it
>     Martin> should be a function or a variable. enable-checking
>     Martin> detected that DECL_TEMPLATE_INFO was invoked for a
>     Martin> CONST_DECL (which had no template info). Explicitly check
>     Martin> for function or variable now.
> 
> But you didn't.  You have:
> 
> !   if (TREE_CODE (newdecl) == TYPE_DECL 
> !       || TREE_CODE (newdecl) == TEMPLATE_DECL)
>       return;
> 
> If you want to make this explicitly for functions or variables, you
> should have:
> 
>   if (TREE_CODE (newdecl) != FUNCTION_DECL
>       && TREE_CODE (newdecl) != VAR_DECL)
>     return;
> 
> right?

Well, my commentary should read: In mark_used ...
There, I have the patch

*************** mark_used (decl)
*** 5146,5156 ****
  
    /* If this is a function or variable that is an instance of some
       template, we now know that we will need to actually do the
!      instantiation.  A TEMPLATE_DECL may also have DECL_TEMPLATE_INFO,
!      if it's a partial instantiation, but there's no need to
!      instantiate such a thing.  We check that DECL is not an explicit
       instantiation because that is not checked in instantiate_decl.  */
!   if (TREE_CODE (decl) != TEMPLATE_DECL
        && DECL_LANG_SPECIFIC (decl) && DECL_TEMPLATE_INFO (decl)
        && !DECL_EXPLICIT_INSTANTIATION (decl))
      instantiate_decl (decl);
--- 5146,5154 ----
  
    /* If this is a function or variable that is an instance of some
       template, we now know that we will need to actually do the
!      instantiation. We check that DECL is not an explicit
       instantiation because that is not checked in instantiate_decl.  */
!   if ((TREE_CODE (decl) == FUNCTION_DECL || TREE_CODE (decl) == VAR_DECL)
        && DECL_LANG_SPECIFIC (decl) && DECL_TEMPLATE_INFO (decl)
        && !DECL_EXPLICIT_INSTANTIATION (decl))
      instantiate_decl (decl);

This actually relates to the change

1998-07-27  Mark Mitchell  <mark@markmitchell.com>

	(mark_used): Don't call instantiate_decl for a TEMPLATE_DECL.

I widened the test, and removed the explanation why you checked for
TEMPLATE_DECL. Sorry about the confusion.


>     Martin> 4. finish_unary_op_expr reported a problem when
>     Martin> TREE_NEGATED_INT was set for a NEGATE_EXPR (when it was
>     Martin> documented only for INTEGER_CSTs). The problem was that
>     Martin> build_x_unary_op would normally fold INTEGER_CSTs - except
>     Martin> inside templates. Fold these constants now even inside
>     Martin> templates.
> 
> I don't think we should do this.  I'm nervous about doing much of
> anything inside a template.  Instead, just have finish_unary_op_expr
> check to see that the resulting thing is an INTEGER_CST.

What is the official interface for folding such a thing? I tried to
call fold() on the resulting expression, ie. 

{
  tree result = build_x_unary_op (code, expr);
  if (code == NEGATE_EXPR && TREE_CODE (expr) == INTEGER_CST)
    {
       result = fold (result);
       TREE_NEGATED_INT (result) = 1;
    }
  overflow_warning (result);
  return result;
}

but that died, because fold expects the TREE_TYPE of the outermost
expression to be correct, whereas build_min_nt sets that to NULL_TREE.

I then used build_new_op; that worked, but I had to throw away the
outcome of the original build_x_unary_op. Also, build_new_op does not
even have a comment, so I'm not sure whether it is 'official' (also,
the name does not fire any mental associations).

Any guidance appreciated,

Martin