Bug 39427

Summary: F2003: Procedures with same name as types/type constructors
Product: gcc Reporter: Damian Rouson <damian>
Component: fortranAssignee: Tobias Burnus <burnus>
Status: RESOLVED FIXED    
Severity: enhancement CC: burnus, d, gcc-bugs, janus, norm.clerman, townsend
Priority: P3    
Version: 4.4.0   
Target Milestone: ---   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2009-03-11 22:41:58
Bug Depends on:    
Bug Blocks: 20585    
Attachments: First draft patch
Second draft patch
Third draft patch
Fourth version of the draft patch
Fifth draft patch - with test case
Updated draft
Re-diff (at the bottom: testcase, list naming the 70 failing tests)

Description Damian Rouson 2009-03-10 23:32:33 UTC
The Fortran 2003 standard Section C.1.6 allows a generic procedure to have the same name as a type , but gfotran gives "Error: DERIVED attribute of 'foo' conflicts with PROCEDURE attribute at (1)" on the following code:

module foo_module
  type foo
    integer :: bar
  end type
  interface foo
    procedure constructor
  end interface
contains
  type(foo) function constructor()
    constructor%bar = 1
  end function
end module

I use this style throughout a textbook I'm writing because it seems like a
natural way to extend the intrinsic structure constructor syntax.  A more extensive example of this technique is also in Figure 17.1 of Metcalf, Reid & Cohen.

Damian
Comment 1 Tobias Burnus 2009-03-11 22:41:58 UTC
Confirm; this F2003 feature is not yet implemented.
Comment 2 Tobias Burnus 2009-12-03 21:02:16 UTC
Quote from the standard:

"12.3.2 Specification of the procedure interface" [...]
"A generic name may be the same as a derived-type name, in which case all of the procedures in the interface block shall be functions."

Currently, one gets the message:
"Error: DERIVED attribute of 'foo' conflicts with PROCEDURE attribute at (1)"
Comment 3 janus 2010-01-23 17:25:19 UTC
Another related quote from the F2003 standard:

C489 (R457) If derived-type-spec is a type name that is the same as a generic name, the component-spec-list shall not be a valid actual-arg-spec-list for a function reference that is resolvable as a generic reference (12.4.4.1).
Comment 4 Steven Bosscher 2010-05-04 12:09:59 UTC
What a horrible rule...
Comment 5 Damian Rouson 2010-05-04 14:25:32 UTC
(In reply to comment #4)
> What a horrible rule...
> 

I'm not sure why you don't like it, but the reason for the rule is to have the ability to overload the intrinsic structure constructors.  The intrinsic structure constructors are virtually useless for object-oriented programming (OOP) because they can't be used if the type has private data (required by the information-hiding philosophy of OOP) unless all components have default initializations (which only helps in trivial cases).  It's standard in C++ (and I'd guess in most other OOP languages) for constructors to have the same name as the type.  What could be more clear to the reader than

type(vector) :: f
f = vector(1.0,2.0)

Once one adopts this approach, it touches every type you develop, so a compiler that doesn't support it becomes useless unless you're willing to change every constructor name to suit that particular compiler.  It's already supported by IBM XL Fortran (my primary development platform), by Cray, by NAG (I think), and Intel has promised to support it in its next release.
Comment 6 Tobias Burnus 2010-05-04 16:10:15 UTC
Created attachment 20557 [details]
First draft patch

(In reply to comment #5)
> (In reply to comment #4)
> > What a horrible rule...

What do you mean? Allowing "name()" as (generic) constructor (function)? Why not? That matches the natural use. If you mean the constraint in comment 3: That just ensures that one cannot have use a structure constructor and generic function simultaneously.


> Intel has promised to support it in its next release.

Well, I can also promise that _one_ of the next releases will have it :P

At least there are plans to improve OOP and there is also a Summer of Code projects for OOP, cf. http://gcc.gnu.org/wiki/SummerOfCode

 * * *

Attached is a draft patch which should work in simple cases.

TODO:

* Call structure constructor if no generic function matches; that means: Converting the actual arguments into a constructor and do the checking (cf. FIXME in resolve.c)

* Add a is-function check: "A generic name may be the same as a derived-type name, in which case all of the procedures in the interface block shall be functions." (Note: That's not a constraint thus one does not need to catch all cases.)

* Fix the case of first defining the generic name and then the type:
interface t
  procedure t2
end interface t
type t
end type t

* Add a new "%X" (X = some letter) to finally solve the "@" printing problem in error messages for PPC and for constructors (this PR): gfc_error should then simply strip the @... from the message for arguments to %X while for %s no extra stripping is done as it is currently done. One then only needs to change all relevant %s to %X and one is done. (using a more-intuitive different latter than %X would be helpful; maybe %S = Symbol String?.)
Comment 7 Tobias Burnus 2010-05-04 16:13:02 UTC
(In reply to comment #6)
> Created an attachment (id=20557) [edit]
> First draft patch

       if (type != current_interface.type
-	  || strcmp (current_interface.sym->name, name) != 0)
+	  && strcmp (current_interface.sym->name, name) != 0)

And that change is of course wrong.
Comment 8 Tobias Burnus 2010-05-05 18:09:00 UTC
Created attachment 20567 [details]
Second draft patch

(In reply to comment #6)
> First draft patch

Updated patch: Fixes the reversed order, fixes .mod read, and has a test case included.

TODO: Call structure constructor if no generic function matches; function constraint; error print fix.
Comment 9 Tobias Burnus 2010-05-05 18:14:24 UTC
(In reply to comment #8)
> Created an attachment (id=20567) [edit]
> Second draft patch

And another omission:
--- a/gcc/fortran/module.c
+++ b/gcc/fortran/module.c
@@ -3781,7 +3787,7 @@ static void
 load_generic_interfaces (void)
 {
   const char *p;
-  char name[GFC_MAX_SYMBOL_LEN + 1], module[GFC_MAX_SYMBOL_LEN + 1];
+  char name[GFC_MAX_SYMBOL_LEN + 3], module[GFC_MAX_SYMBOL_LEN + 1];
   gfc_symbol *sym;
   gfc_interface *generic = NULL, *gen = NULL;
   int n, i, renamed;
Comment 10 Tobias Burnus 2010-05-07 09:16:38 UTC
Created attachment 20592 [details]
Third draft patch

Updated patch: Support structure constructor if no generic function matches, function
constraint. Thus: All valid programs should work.

TODO: Clean up and maybe extend test cases. Error print fix (likely to be deferred to PR 39695)
Comment 11 Tobias Burnus 2010-05-07 09:20:12 UTC
(In reply to comment #10)
> Created an attachment (id=20592) [edit] Third draft patch

To continue the tradition: There is again something "wrong" with the patch: It accidentally also contains the patch for PR 43945 (resolve.c, cf. http://gcc.gnu.org/ml/fortran/2010-05/msg00037.html).
Comment 12 Tobias Burnus 2010-05-07 16:37:15 UTC
Created attachment 20599 [details]
Fourth version of the draft patch

And fourth version. I have just realized that the patch is too simplistic and that I might need go back to the drawing board. The following is currently not handled:

a) Constraint check (cf. comment 3 [F03: C489, F08: C496]): the component-spec-list has to be different from the interface of any of the generic procedures (i.e. an actual-arg-spec-list matching the component-spec-list shall not match any of the generic functions). Note: This included handling default initializers as they act as optional arguments.

b) The algorithm does not handle: Define function in one module,* define type in a different module, and use associate them [including constraint checking, cf. (a)]. Currently, the renaming is only handled if either the generic interface or the derived type is available before the other type is declared.

(* That's possible because the functions do not need to return the derived type - even though that's the most common case.)

I probably solve this by renaming derived types to <name>@ and automatically generating an associated generic function with the specific name <name>@ and a dummy arg list [including DT parents (plural)]. One needs to modify gfc_typename to print the type without @ and then one has also no problem with the trailing @ sign.
Comment 13 Tobias Burnus 2010-05-18 21:51:52 UTC
Created attachment 20696 [details]
Fifth draft patch - with test case

New approach. The attached patch now also works with twisted modules (cf. test case in the attachment). However, it needs still some clean up as there are test suite failures.

Additional tasks: (a) Check whether one can get rid of gfc_match_structure_constructor. (b) Add check to ensure that the generic name only contains functions and that the type name does not exist as specific function name. (c) Do general clean up, bug fixing, and add test cases.

Regarding C489 [F2003] and C496 [F2008], see also:
http://groups.google.com/group/comp.lang.fortran/browse_thread/thread/b3580ffd988330d7
Comment 14 Tobias Burnus 2010-05-20 21:13:56 UTC
Created attachment 20714 [details]
Updated draft

Use Bugzilla as backup to make sure the patch does not get lost.

Mostly regtests now (353 FAILS with 73 tests), though there are still some issues, e.g., the DWARF name should be without @  (for "(gdb) pt"). Some clean up is needed, cf. also comment  #13. Test cases: See bottom of attachment 20567 [details] and attachment 20599 [details].
Comment 15 Tobias Burnus 2010-05-25 19:55:35 UTC
(In reply to comment #14)
> Created an attachment (id=20714) [edit]

In module.c's import_iso_c_binding_module, one needs to replace:
	      local_name = gfc_get_string ("%s@", u->local_name);
by
 	      local_name = (u->local_name[0] != '\0')
 			   ? gfc_get_string ("%s@", u->local_name) : NULL;

I now will try the other approach of using ns->derived_types to see whether the code will be cleaner.
Comment 16 Tobias Burnus 2010-07-30 09:03:03 UTC
Remove assignment. I think I won't work on this in the next weeks and "New" is better in allowing others to pick it up. If not, I will look at it again later. 

The patch of attachment 20714 [details] plus the fix in comment 15 (plus the test cases at the bottom of attachment 20696 [details]) mostly work OK - except for a handful regressions. The implementation was using "DT-name" as generic function and created the derived type gfc_symbol using the suffix "@".

It was suggested to do the following - which might be cleaner:
- Create the <dt name> as generic function (as before)
- Use ns->derived_types to save the derived types (instead of generating another symbol)

I think one can mostly recycle the resolve.c part of the patch - and the gfc_convert_to_structure_constructor's part in primary.c (one can probably get rid of gfc_match_structure_constructor after some modifications). However, all the rest has to be adapted.
Comment 17 Damian Rouson 2010-07-31 02:30:40 UTC
Tobias,

Thanks for your continued efforts on this.  It will ultimately have a huge impact on the usability of gfortran for my purposes.   I look forward to hearing more when you get back to it or when others do.  

Since there are regressions, I assume I won't get access to your patch just by doing an update using macports (my preferred method for building), but I look forward to trying it out once it's available.

Damian

(In reply to comment #16)
> Remove assignment. I think I won't work on this in the next weeks and "New" is
> better in allowing others to pick it up. If not, I will look at it again later. 
> 
> The patch of attachment 20714 [details] [edit] plus the fix in comment 15 (plus the test cases
> at the bottom of attachment 20696 [details] [edit]) mostly work OK - except for a handful
> regressions. The implementation was using "DT-name" as generic function and
> created the derived type gfc_symbol using the suffix "@".
> 
> It was suggested to do the following - which might be cleaner:
> - Create the <dt name> as generic function (as before)
> - Use ns->derived_types to save the derived types (instead of generating
> another symbol)
> 
> I think one can mostly recycle the resolve.c part of the patch - and the
> gfc_convert_to_structure_constructor's part in primary.c (one can probably get
> rid of gfc_match_structure_constructor after some modifications). However, all
> the rest has to be adapted.
> 

Comment 18 Tobias Burnus 2010-08-01 07:25:56 UTC
*** Bug 45155 has been marked as a duplicate of this bug. ***
Comment 19 Damian Rouson 2010-09-28 18:38:12 UTC
Could someone please comment on the relevance (or lack thereof) of the component being public in the example I submitted?  My real goal is to have all data components private, but I left that out in the test case I submitted.  The question of whether the code would still be valid appears to be a sticking point in getting one vendor to support the requested feature.
Comment 20 Tobias Burnus 2010-09-30 15:49:37 UTC
(In reply to comment #19)
> Could someone please comment on the relevance (or lack thereof) of the
> component being public in the example I submitted?

This issue was solved off list/PR. The question was how to interpret the following, where [F2008's] R455 defines a "structure-constructor":

"C496 (R455) If derived-type-spec is a type name that is the same as a generic name, the component-spec-list shall not be a valid actual-arg-spec-list for a function reference that is resolvable as a generic reference to that name (12.5.5.2)."

My interpretation is that this only disallows structure constructors, i.e., that generic functions have a higher precedence than structure constructors. Thus, one can always override a constructor by a generic function. (And I do not see how private components could alter the interpretation of C496 - even if one reads it differently.)

F2008 also has the following note, which supports the overriding interpretation:

"NOTE 4.58   The form 'name(...)' is interpreted as a generic function-reference if possible; it is interpreted as a structure-constructor only if it cannot be interpreted as a generic function-reference."
Comment 21 Tobias Burnus 2010-10-01 08:02:36 UTC
Created attachment 21932 [details]
Re-diff (at the bottom: testcase, list naming the 70 failing tests)

Simple re-diff of previous patch for the current trunk. Currently, 70 test cases are failing. Seemingly, there is still some ISO_C_BINDING issue, some type-extension issue and some various issues.
Comment 22 Tobias Burnus 2010-11-03 09:14:59 UTC
Last draft patch: http://gcc.gnu.org/ml/fortran/2010-10/msg00274.html
Comment 23 Tobias Burnus 2010-12-08 13:11:02 UTC
Using the last draft patch: With the attachment 22685 [details] to bug 46849, one gets an error as "fun_qag" is regarded as FL_PROCEDURE and not as DERIVED; the issue seems to be unrelated to the other PR and should be checked/fixed in the constructor patch.
Comment 24 Daniel Franke 2010-12-28 21:49:39 UTC
*** Bug 40824 has been marked as a duplicate of this bug. ***
Comment 25 janus 2011-01-09 21:42:04 UTC
A loosely related bug is PR 42418, which demands that generic interfaces can have the same name as specific procedures. When fixing this bug, one should ideally find a solution which also fixes that issue (e.g. one could have a separate symtree for generics in a namespace).
Comment 26 janus 2011-03-16 11:23:17 UTC
*** Bug 48145 has been marked as a duplicate of this bug. ***
Comment 27 Tobias Burnus 2011-10-11 16:16:44 UTC
Mine (again after 1.5 years). This time, I plan to finish the patch and make it read for inclusion in 4.7. The patch is essentially the one linked in comment 22 but with a couple of regressions fixed. (Current status: 15 GCC test-suite failures and one real-world failure [due to seemingly about 10 separate issues].)

Draft patch and current status:
  https://userpage.physik.fu-berlin.de/~tburnus/tmp/constructor.diff
Comment 28 Tobias Burnus 2011-11-09 13:51:52 UTC
Submitted patch (review is pending):
  http://gcc.gnu.org/ml/fortran/2011-11/msg00061.html
Comment 29 Tobias Burnus 2011-11-16 21:37:48 UTC
Author: burnus
Date: Wed Nov 16 21:37:43 2011
New Revision: 181425

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181425
Log:
gcc/fortran
2011-11-16  Tobias Burnus  <burnus@net-b.de>

        PR fortran/39427
        PR fortran/37829
        * decl.c (match_data_constant, match_data_constant,
        * variable_decl,
        gfc_match_decl_type_spec, access_attr_decl,
        check_extended_derived_type, gfc_match_derived_decl,
        gfc_match_derived_decl, gfc_match_derived_decl) Modified to deal
        with DT constructors.
        * gfortran.h (gfc_find_dt_in_generic,
        gfc_convert_to_structure_constructor): New function prototypes.
        * interface.c (check_interface0, check_interface1,
        gfc_search_interface): Ignore DT constructors in generic list.
        * match.h (gfc_match_structure_constructor): Update prototype.
        * match.c (match_derived_type_spec): Ensure that one uses the DT
        not the generic function.
        * module.c (MOD_VERSION): Bump.
        (dt_lower_string, dt_upper_string): New functions.
        (find_use_name_n, find_use_operator, compare_true_names,
        find_true_name, add_true_name, fix_mio_expr, load_needed,
        read_module, write_dt_extensions, write_symbol): Changes to deal with
        different symtree vs. sym names.
        (create_derived_type): Create also generic procedure.
        * parse.c (gfc_fixup_sibling_symbols): Don't regard DT and
        * generic
        function as the same.
        * primary.c (gfc_convert_to_structure_constructor): New
        * function.
        (gfc_match_structure_constructor): Restructured; calls
        gfc_convert_to_structure_constructor.
        (build_actual_constructor, gfc_match_rvalue): Update for DT generic
        functions.
        * resolve.c (resolve_formal_arglist, resolve_structure_cons,
        is_illegal_recursion, resolve_generic_f, resolve_variable,
        resolve_fl_variable_derived, resolve_fl_derived0,
        resolve_symbol): Handle DT and DT generic constructors.
        * symbol.c (gfc_use_derived, gfc_undo_symbols,
        gen_special_c_interop_ptr, gen_cptr_param,
        generate_isocbinding_symbol, gfc_get_derived_super_type): Handle
        derived-types, which are hidden in the generic type.
        (gfc_find_dt_in_generic): New function
        * trans-array.c (gfc_conv_array_initializer): Replace
        * FL_PARAMETER
        expr by actual value.
        * trans-decl.c (gfc_get_module_backend_decl,
        * gfc_trans_use_stmts):
        Ensure that we use the DT and not the generic function.
        * trans-types.c (gfc_get_derived_type): Ensure that we use the
        * DT
        and not the generic procedure.

gcc/testsuite/
2011-11-16  Tobias Burnus  <burnus@net-b.de>

        PR fortran/39427
        PR fortran/37829
        * gfortran.dg/constructor_1.f90: New.
        * gfortran.dg/constructor_2.f90: New.
        * gfortran.dg/constructor_3.f90: New.
        * gfortran.dg/constructor_4.f90: New.
        * gfortran.dg/constructor_5.f90: New.
        * gfortran.dg/constructor_6.f90: New.
        * gfortran.dg/use_only_5.f90: New.
        * gfortran.dg/c_ptr_tests_17.f90: New.
        * gfortran.dg/c_ptr_tests_18.f90: New.
        * gfortran.dg/used_types_25.f90: New.
        * gfortran.dg/used_types_26.f90: New
        * gfortran.dg/type_decl_3.f90: New.
        * gfortran.dg/function_types_3.f90: Update dg-error.
        * gfortran.dg/result_1.f90: Ditto.
        * gfortran.dg/structure_constructor_3.f03: Ditto.
        * gfortran.dg/structure_constructor_4.f03: Ditto.

Added:
    trunk/gcc/testsuite/gfortran.dg/c_ptr_tests_17.f90
    trunk/gcc/testsuite/gfortran.dg/c_ptr_tests_18.f90
    trunk/gcc/testsuite/gfortran.dg/constructor_1.f90
    trunk/gcc/testsuite/gfortran.dg/constructor_2.f90
    trunk/gcc/testsuite/gfortran.dg/constructor_3.f90
    trunk/gcc/testsuite/gfortran.dg/constructor_4.f90
    trunk/gcc/testsuite/gfortran.dg/constructor_5.f90
    trunk/gcc/testsuite/gfortran.dg/constructor_6.f90
    trunk/gcc/testsuite/gfortran.dg/type_decl_3.f90
    trunk/gcc/testsuite/gfortran.dg/use_only_5.f90
    trunk/gcc/testsuite/gfortran.dg/used_types_25.f90
    trunk/gcc/testsuite/gfortran.dg/used_types_26.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/decl.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/interface.c
    trunk/gcc/fortran/match.c
    trunk/gcc/fortran/match.h
    trunk/gcc/fortran/module.c
    trunk/gcc/fortran/parse.c
    trunk/gcc/fortran/primary.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/symbol.c
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans-types.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/function_types_3.f90
    trunk/gcc/testsuite/gfortran.dg/result_1.f90
    trunk/gcc/testsuite/gfortran.dg/structure_constructor_3.f03
    trunk/gcc/testsuite/gfortran.dg/structure_constructor_4.f03
Comment 30 Tobias Burnus 2011-11-16 21:54:10 UTC
FIXED on the 4.7 trunk.
Comment 31 Damian Rouson 2011-11-17 01:43:38 UTC
This is awesome news!

:D

On Wed, Nov 16, 2011 at 1:54 PM, burnus at gcc dot gnu.org <
gcc-bugzilla@gcc.gnu.org> wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39427
>
> Tobias Burnus <burnus at gcc dot gnu.org> changed:
>
>           What    |Removed                     |Added
>
> ----------------------------------------------------------------------------
>             Status|ASSIGNED                    |RESOLVED
>         Resolution|                            |FIXED
>
> --- Comment #30 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-11-16
> 21:54:10 UTC ---
> FIXED on the 4.7 trunk.
>
> --
> Configure bugmail: http://gcc.gnu.org/bugzilla/userprefs.cgi?tab=email
> ------- You are receiving this mail because: -------
> You reported the bug.
>