[Patch, Fortran, OOP] PR 46313: OOP-ABI issue, ALLOCATE issue, CLASS renaming issue

[Janus Weil]

Hi all,

with the test cases in this PR Tobias demonstrated that our naming
scheme for class containers and vtables is insufficient. It currently
is based only on the type name. As shown in the PR, naming ambiguities
can be created, e.g. by setting up two derived types with identical
names in different modules, and use-renaming them in the main program.

The patch avoids these naming ambiguities by including the module name
in the naming scheme for class containers and vtabs. Example:

module mo
type :: dt
! ...
end type
class(dt), pointer :: cp
end module

Without the patch, the class container name is "class$dt", with the
patch it will be "class$mo$dt". This makes sure that we get one unique
class container and vtab for each derived type, even with renamed
derived types.

The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?

Cheers,
Janus


2010-11-06 Janus Weil <***@gcc.gnu.org>

PR fortran/46313
* class.c (get_unique_type_string): New function.
(gfc_build_class_symbol): Use 'get_unique_type_string' to construct
uniques names for the class containers.
(gfc_find_derived_vtab): Use 'get_unique_type_string' to construct
uniques names for the vtab symbols.

2010-11-06 Janus Weil <***@gcc.gnu.org>

PR fortran/46313
* gfortran.dg/class_28.f03: New.

[Thomas Koenig]

Post by Janus Weil
The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?

If we are going to change the naming of the OOP stuff anyway, what about
the possible name collisions with -fdollar-ok ?

Should we put in an underscore, an at sign etc. or should we decide that
anybody who creates a name collision like that with a non-standard
extension deserves to lose?

Test case:

***@linux-fd1f:~/Krempel/Class> cat class.f90
program main
type :: dt
! ...
end type dt
class(dt), pointer :: cp
contains
subroutine vtab$dt
end subroutine vtab$dt
end program main
***@linux-fd1f:~/Krempel/Class> gfortran -fdollar-ok class.f90
class.f90:7.20:

subroutine vtab$dt
1
Error: VARIABLE attribute of 'vtab$dt' conflicts with PROCEDURE
attribute at (1)
class.f90:8.5:

end subroutine vtab$dt
1
Error: Expecting END PROGRAM statement at (1)

[Janus Weil]

Hi Thomas,

Post by Thomas Koenig

Post by Janus Weil
The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?

If we are going to change the naming of the OOP stuff anyway, what about
the possible name collisions with -fdollar-ok ?

uh, good point. To be honest I wasn't even aware of this flag ...

Post by Thomas Koenig
Should we put in an underscore, an at sign etc. or should we decide that
anybody who creates a name collision like that with a non-standard
extension deserves to lose?

Well, I can image that someone using a flag called '-fdollar-ok' would
expect that it's ok to use dollar signs without running into trouble.

So, yes, since we're about to change the naming anyway, I think we
should get rid of the dollars. Throwing in @s instead is fine with me,
or is there a downside to this choice, too?

I think the underscore is not an option, since it can also be used in
standard variable names, right?

Cheers,
Janus

Post by Thomas Koenig
program main
 type :: dt
    ! ...
 end type dt
 class(dt), pointer :: cp
contains
 subroutine vtab$dt
 end subroutine vtab$dt
end program main
 subroutine vtab$dt
                   1
Error: VARIABLE attribute of 'vtab$dt' conflicts with PROCEDURE
attribute at (1)
 end subroutine vtab$dt
    1
Error: Expecting END PROGRAM statement at (1)

[Dominique Dhumieres]

Post by Thomas Koenig
If we are going to change the naming of the OOP stuff anyway, what about
the possible name collisions with -fdollar-ok ?

With the patch for PR 46313, the test compiled with -fdollar-ok is
compiled without problem on x86_64-apple-darwin10.

Otherwise, the patch has not disturbed my pet bugs!-)

Thanks,

Dominique

[Janus Weil]

Post by Dominique Dhumieres

Post by Thomas Koenig
If we are going to change the naming of the OOP stuff anyway, what about
the possible name collisions with -fdollar-ok ?

With the patch for PR 46313, the test compiled with -fdollar-ok is
compiled without problem on x86_64-apple-darwin10.

Yes, that is expected, because the patch changes the name of the vtab
to "vtab$main$dt", so one needs to change the name of the subroutine
in the test case in the same way in order to see the failure:

program main
type :: dt
! ...
end type dt
class(dt), pointer :: cp
contains
subroutine vtab$main$dt
end subroutine vtab$main$dt
end program main


Btw, I just tried the version with '@' instead of '$'. Unfortunately
the assembler doesn't seem to like that. I get tons of errors like:

/tmp/ccToz7yL.s:3: Error: junk at end of line, first unrecognized
character is `@'
/tmp/ccToz7yL.s:4: Error: invalid character '@' in mnemonic


Are there any other special characters we can exploit? What about the
pound sign or the ampersand? Any problems to expect with these? Or
should we rather stay with the dollar and ignore the problems with
-fdollar-ok?

Cheers,
Janus

[Tobias Burnus]

Post by Janus Weil
Yes, that is expected, because the patch changes the name of the vtab
to "vtab$main$dt", so one needs to change the name of the subroutine

Dot? vtab.main.dt?

Tobias

[Janus Weil]

Post by Tobias Burnus

Post by Janus Weil
Yes, that is expected, because the patch changes the name of the vtab
to "vtab$main$dt", so one needs to change the name of the subroutine

Dot? vtab.main.dt?

Yes, we once had this variant. I think the reason why I switched to
dollars was that it made the dumps easier to read (think
"vtab.main.dt..extends..size" etc).

The best option I can currently see is to use leading underscores (as
in "_vtab_main_dt"). This is forbidden in Fortran (cf. F08:R303), but
accepted by the assembler (cf.
http://sourceware.org/binutils/docs-2.20/as/Symbol-Names.html#Symbol-Names).

Attached is a patch which does this change. I also added a few macros
in gfortran.h. Ok for trunk after successful regtest?

Cheers,
Janus

[Tobias Schlüter]

Post by Janus Weil

Post by Tobias Burnus

Post by Janus Weil
Yes, that is expected, because the patch changes the name of the vtab
to "vtab$main$dt", so one needs to change the name of the subroutine

Dot? vtab.main.dt?

Yes, we once had this variant. I think the reason why I switched to
dollars was that it made the dumps easier to read (think
"vtab.main.dt..extends..size" etc).

Sorry I'm late, but gcc has the macro ASM_FORMAT_PRIVATE_NAME which does
the work of making a name collision-free. If you use it you can make
the rest of the name as readable as you want.

Cheers,
- Tobi

Post by Janus Weil
The best option I can currently see is to use leading underscores (as
in "_vtab_main_dt"). This is forbidden in Fortran (cf. F08:R303), but
accepted by the assembler (cf.
http://sourceware.org/binutils/docs-2.20/as/Symbol-Names.html#Symbol-Names).
Attached is a patch which does this change. I also added a few macros
in gfortran.h. Ok for trunk after successful regtest?
Cheers,
Janus

[Tobias Burnus]

Post by Tobias Schlüter

Post by Janus Weil

Post by Janus Weil
Yes, that is expected, because the patch changes the name of the vtab
to "vtab$main$dt", so one needs to change the name of the subroutine

Sorry I'm late, but gcc has the macro ASM_FORMAT_PRIVATE_NAME which does
the work of making a name collision-free. If you use it you can make the
rest of the name as readable as you want.

I think that it won't work. One needs the same assembler name for in
each translation unit as there is one, common, global vtable per base
type. My understanding is that ASM_FORMAT_PRIVATE_NAME would generate
several disjunct assembler names...

Post by Tobias Schlüter

Post by Janus Weil
The best option I can currently see is to use leading underscores (as
in "_vtab_main_dt"). This is forbidden in Fortran (cf. F08:R303), but
accepted by the assembler (cf.
http://sourceware.org/binutils/docs-2.20/as/Symbol-Names.html#Symbol-Names).
Attached is a patch which does this change. I also added a few macros
in gfortran.h. Ok for trunk after successful regtest?

+static void
+get_unique_type_string (char *string, gfc_symbol *derived)
+{
+ if (derived->module)
+ sprintf (string, "%s_%s", derived->module, derived->name);
+ else
+ sprintf (string, "%s_%s", derived->ns->proc_name->name, derived->name);
+}

I wonder whether that works for cases like:

* module name == procedure name
* internal procedures == procedure name / module name.
* Nested submodules (when implemented)

I assume defining the same-named (and disjunct) DT in a module and
separately in a subroutine is weird; with internal procedures and
submodules it will start to get crazy. But shouldn't one do something
like _proc_<procedure_name> for procedures? A {module/procedure
name}_internal-proc-name?

Alternatively, one can defer it until a real-world bug report comes
(which I think is very unlikely).

(And, at some point, the name becomes too long and one could think of
using a hash name as in GCC's C++ compiler.)

Tobias

[Janus Weil]

Post by Tobias Burnus

Post by Janus Weil
The best option I can currently see is to use leading underscores (as
in "_vtab_main_dt"). This is forbidden in Fortran (cf. F08:R303), but
accepted by the assembler (cf.
http://sourceware.org/binutils/docs-2.20/as/Symbol-Names.html#Symbol-Names).
Attached is a patch which does this change. I also added a few macros
in gfortran.h. Ok for trunk after successful regtest?

+static void
+get_unique_type_string (char *string, gfc_symbol *derived)
+{
+  if (derived->module)
+    sprintf (string, "%s_%s", derived->module, derived->name);
+  else
+    sprintf (string, "%s_%s", derived->ns->proc_name->name, derived->name);
+}
* module name == procedure name
* internal procedures == procedure name / module name.
* Nested submodules (when implemented)
I assume defining the same-named (and disjunct) DT in a module and
separately in a subroutine is weird; with internal procedures and submodules
it will start to get crazy. But shouldn't one do something like
_proc_<procedure_name> for procedures? A {module/procedure
name}_internal-proc-name?
Alternatively, one can defer it until a real-world bug report comes (which I
think is very unlikely).

I also think it's unlikely. And in fact we have enough real-world PRs
to deal with right now, so I would prefer to defer this to some point
in late stage 3 when we have fixed all the worst bugs and start to get
bored ;)

The last patch I sent (leading underscore version) regtested fine btw.
Is it ok if I commit it now and leave the PR open for the corner
cases?

Cheers,
Janus

[Tobias Schlüter]

Post by Tobias Burnus

Post by Tobias Schlüter

Post by Janus Weil
Yes, that is expected, because the patch changes the name of the vtab
to "vtab$main$dt", so one needs to change the name of the subroutine

Sorry I'm late, but gcc has the macro ASM_FORMAT_PRIVATE_NAME which does
the work of making a name collision-free. If you use it you can make the
rest of the name as readable as you want.

I think that it won't work. One needs the same assembler name for in
each translation unit as there is one, common, global vtable per base
type. My understanding is that ASM_FORMAT_PRIVATE_NAME would generate
several disjunct assembler names...

I don't think so, it only adds a platform-dependent character to the
variable name. The offered varieties cover exactly what was suggested
in this thread so far:

#ifndef ASM_PN_FORMAT
# ifndef NO_DOT_IN_LABEL
# define ASM_PN_FORMAT "%s.%lu"
# else
# ifndef NO_DOLLAR_IN_LABEL
# define ASM_PN_FORMAT "%s$%lu"
# else
# define ASM_PN_FORMAT "__%s_%lu"
# endif
# endif
#endif /* ! ASM_PN_FORMAT */

#ifndef ASM_FORMAT_PRIVATE_NAME
# define ASM_FORMAT_PRIVATE_NAME(OUTPUT, NAME, LABELNO) \
do { const char *const name_ = (NAME); \
char *const output_ = (OUTPUT) = \
(char *) alloca (strlen (name_) + 32); \
sprintf (output_, ASM_PN_FORMAT, name_, (unsigned long)(LABELNO)); \
} while (0)
#endif

Cheers,
- Tobi

[Janus Weil]

Post by Tobias Schlüter

Post by Tobias Burnus

Post by Tobias Schlüter

Post by Janus Weil
Yes, that is expected, because the patch changes the name of the vtab
to "vtab$main$dt", so one needs to change the name of the subroutine

Sorry I'm late, but gcc has the macro ASM_FORMAT_PRIVATE_NAME which does
the work of making a name collision-free. If you use it you can make the
rest of the name as readable as you want.

I think that it won't work. One needs the same assembler name for in
each translation unit as there is one, common, global vtable per base
type. My understanding is that ASM_FORMAT_PRIVATE_NAME would generate
several disjunct assembler names...

I don't think so, it only adds a platform-dependent character to the
variable name.  The offered varieties cover exactly what was suggested in
#ifndef ASM_PN_FORMAT
# ifndef NO_DOT_IN_LABEL
#  define ASM_PN_FORMAT "%s.%lu"
# else
#  ifndef NO_DOLLAR_IN_LABEL
#   define ASM_PN_FORMAT "%s$%lu"
#  else
#   define ASM_PN_FORMAT "__%s_%lu"
#  endif
# endif
#endif /* ! ASM_PN_FORMAT */
#ifndef ASM_FORMAT_PRIVATE_NAME
# define ASM_FORMAT_PRIVATE_NAME(OUTPUT, NAME, LABELNO) \
 do { const char *const name_ = (NAME); \
      char *const output_ = (OUTPUT) = \
        (char *) alloca (strlen (name_) + 32); \
      sprintf (output_, ASM_PN_FORMAT, name_, (unsigned long)(LABELNO)); \
 } while (0)
#endif

Yes, I think it would do the job. But we actually do not need the
additional number.

Btw, what is the reason for the macro adding *two* underscores in
front, instead of just one?

Cheers,
Janus

[Tobias Schlüter]

Post by Janus Weil
Btw, what is the reason for the macro adding *two* underscores in
front, instead of just one?

I got curious and did some googling. Tthe C standard has this:
7.1.3 Reserved identifiers

Each header declares or defines all identifiers listed in its
associated subclause, and optionally declares or defines identifiers
listed in its associated future library directions subclause and
identifiers which are always reserved either for any use or for use as
file scope identifiers.

* All identifiers that begin with an underscore and either an
uppercase letter or another underscore are always reserved for any use.
...

So if the compiler can't use a character the user can't put into
identifiers ('.' or '$'), it reverts to something that isn't allowed to
put into identifiers: two underscores in the beginning. I think the
lesson for us is: '.' and '$' aren't portable.

Cheers,
- Tobi

[Steve Kargl]

Post by Janus Weil

Post by Tobias Burnus

Post by Janus Weil
Yes, that is expected, because the patch changes the name of the vtab
to "vtab$main$dt", so one needs to change the name of the subroutine

Dot? vtab.main.dt?

Yes, we once had this variant. I think the reason why I switched to
dollars was that it made the dumps easier to read (think
"vtab.main.dt..extends..size" etc).
The best option I can currently see is to use leading underscores (as
in "_vtab_main_dt"). This is forbidden in Fortran (cf. F08:R303), but
accepted by the assembler (cf.
http://sourceware.org/binutils/docs-2.20/as/Symbol-Names.html#Symbol-Names).
Attached is a patch which does this change. I also added a few macros
in gfortran.h. Ok for trunk after successful regtest?

A leading underscore moves the issue from -fdollar-ok to
-fleading_underscore. IIRC, -fdollar-ok was introduced
to gfortran for compatibility with g77, which allows
legacy code to compile. None of the OOP features should
appear in legacy code, so just throw an error if -fdollar-ok
is used when an OO feature is in the code.

[Tobias Burnus]

Post by Janus Weil
The patch avoids these naming ambiguities by including the module name
in the naming scheme for class containers and vtabs.
The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?

OK.

I think we can worry about submodules and similar problem later - when
real-world programs pop up which use them. At least there is then only a
single place to change.

Regarding $ vs. period vs. _: Seemingly, all platforms on which gfortran
is used support the $ as there was not bug report so far. Thus, I think
we can continue using it. Regarding user code: I somehow think it is
unlikely that users have that strange variable names; if they do: It's
their fault as $ is an not allowed in ISO Fortran.

Post by Janus Weil
Btw, what is the reason for the macro adding *two* underscores in
front, instead of just one?

C allows leading underscores -- and according to the ISO C standard,
identifiers which start with two underscores are for the internal use of
the compiler.

Post by Janus Weil
A leading underscore moves the issue from -fdollar-ok to
-fleading_underscore.

Well, there is a difference. Many compilers support a $ sign - some also
as first character (gfortran does not!). Most compilers do not support
leading underscores; gfortran's -fleading_underscore only exists to
generate (before BIND(C) was implemented) __gfortran_... procedures.
-fleading_underscore is also not officially supported (e.g. it is not in
the man page) - and there are some compile-time restrictions.

I also do not think we need to explicitly take care of leading
underscores (which can also be reached via BIND(C)) or $. If a user
wants to shoot into his foot, the compiler does not need to prevent it.
-- It just shouldn't encourage it. And if possible, standard conforming
code should not break. If users plays around with __vtab$... I think its
their fault.

Tobias

[Janus Weil]

Post by Tobias Burnus

Post by Janus Weil
The patch avoids these naming ambiguities by including the module name
in the naming scheme for class containers and vtabs.
The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?

OK.
I think we can worry about submodules and similar problem later - when
real-world programs pop up which use them. At least there is then only a
single place to change.
Regarding $ vs. period vs. _: Seemingly, all platforms on which gfortran is
used support the $ as there was not bug report so far. Thus, I think we can
continue using it. Regarding user code: I somehow think it is unlikely that
users have that strange variable names; if they do: It's their fault as $ is
an not allowed in ISO Fortran.

Post by Janus Weil
Btw, what is the reason for the macro adding *two* underscores in
front, instead of just one?

C allows leading underscores -- and according to the ISO C standard,
identifiers which start with two underscores are for the internal use of the
compiler.

Ok, so it seems to me that using two leading underscores is really the
best option, since it's safe against collisions with Fortran and C
user code, and also safe to use with -fdollar-ok.

The attached patch adds double underscores for the vtabs, vtypes,
class containers and temporaries.

Cheers,
Janus

[Tobias Burnus]

Post by Janus Weil
Ok, so it seems to me that using two leading underscores is really the
best option, since it's safe against collisions with Fortran and C
user code, and also safe to use with -fdollar-ok.
The attached patch adds double underscores for the vtabs, vtypes,
class containers and temporaries.

OK. Thanks for the patch!

Tobias

[Janus Weil]

Post by Tobias Burnus

Post by Janus Weil
Ok, so it seems to me that using two leading underscores is really the
best option, since it's safe against collisions with Fortran and C
user code, and also safe to use with -fdollar-ok.
The attached patch adds double underscores for the vtabs, vtypes,
class containers and temporaries.

OK. Thanks for the patch!

Committed as r166480.

Thanks for all the helpful comments, everyone!

Cheers,
Janus

[Bernhard Reutner-Fischer]

Post by Janus Weil

Post by Tobias Burnus

Post by Janus Weil
Ok, so it seems to me that using two leading underscores is really the
best option, since it's safe against collisions with Fortran and C
user code, and also safe to use with -fdollar-ok.
The attached patch adds double underscores for the vtabs, vtypes,
class containers and temporaries.

OK. Thanks for the patch!

Committed as r166480.
Thanks for all the helpful comments, everyone!

Index: gcc/fortran/module.c
===================================================================
--- gcc/fortran/module.c (revision 166419)
+++ gcc/fortran/module.c (working copy)
@@ -4372,8 +4372,8 @@ read_module (void)
p = name;

/* Exception: Always import vtabs & vtypes. */
- if (p == NULL && (strncmp (name, "vtab$", 5) == 0
- || strncmp (name, "vtype$", 6) == 0))
+ if (p == NULL && (strncmp (name, "__vtab_", 5) == 0
+ || strncmp (name, "__vtype_", 6) == 0))
p = name;

/* Skip symtree nodes not in an ONLY clause, unless there

---8<---

Sorry for the late follow-up but current trunk still has the code
quoted above where we forgot to add 2 to the length parameter of both
strncmp calls.

I think it would be nice to teach the C and C++ frontends to warn
about this even though it might trigger in quite some code in the
wild.

Looking at gcc/fortran alone there are
gcc/fortran/interface.c: if (strncmp (mode, "unformatted", 9) == 0) // 11 !
gcc/fortran/module.c: && (strncmp (name, "__vtab_", 5) == 0 // 7 !
gcc/fortran/module.c: || strncmp (name, "__vtype_", 6) == 0)) // 8!
gcc/fortran/module.c: || (strncmp (name, "__vtab_", 5) != 0 // 7!
gcc/fortran/module.c: && strncmp (name, "__vtype_", 6)
!= 0)) // 8!

so warning by default with -Wall or at least per default in -Wextra
would make sense IMO.

cheers,

[Janus Weil]

Am Mo., 17. Sep. 2018 um 10:59 Uhr schrieb Bernhard Reutner-Fischer

Post by Bernhard Reutner-Fischer

Post by Janus Weil

Post by Tobias Burnus

Post by Janus Weil
Ok, so it seems to me that using two leading underscores is really the
best option, since it's safe against collisions with Fortran and C
user code, and also safe to use with -fdollar-ok.
The attached patch adds double underscores for the vtabs, vtypes,
class containers and temporaries.

OK. Thanks for the patch!

Committed as r166480.
Thanks for all the helpful comments, everyone!

Index: gcc/fortran/module.c
===================================================================
--- gcc/fortran/module.c (revision 166419)
+++ gcc/fortran/module.c (working copy)
@@ -4372,8 +4372,8 @@ read_module (void)
p = name;
/* Exception: Always import vtabs & vtypes. */
- if (p == NULL && (strncmp (name, "vtab$", 5) == 0
- || strncmp (name, "vtype$", 6) == 0))
+ if (p == NULL && (strncmp (name, "__vtab_", 5) == 0
+ || strncmp (name, "__vtype_", 6) == 0))
p = name;
/* Skip symtree nodes not in an ONLY clause, unless there
---8<---
Sorry for the late follow-up

'Late' is a pretty bold understatement for 8 years ;D

But in any case, 'late' is certainly better than 'never' ...

Post by Bernhard Reutner-Fischer
but current trunk still has the code
quoted above where we forgot to add 2 to the length parameter of both
strncmp calls.

True! Thanks for noticing. I'll take care of fixing it.

Post by Bernhard Reutner-Fischer
I think it would be nice to teach the C and C++ frontends to warn
about this even though it might trigger in quite some code in the
wild.

I don't really think this is a good idea. There are actually valid use
cases of strncmp, where the 'num' argument does not correspond to the
length of any of the two strings (in particular if they're not const).

Instead, for the sake of gfortran, how about a macro like this?

#define gfc_str_startswith(str, pref) \
(strncmp ((str), (pref), strlen (pref)) == 0)

(In fact I just noticed that something like this already exists in
trans-intrinsic.c, so I would just move it into gfortran.h and rename
it.)

Post by Bernhard Reutner-Fischer
Looking at gcc/fortran alone there are
gcc/fortran/interface.c: if (strncmp (mode, "unformatted", 9) == 0) // 11 !

I think this one could actually be a 'strcmp'?

Post by Bernhard Reutner-Fischer
gcc/fortran/module.c: && (strncmp (name, "__vtab_", 5) == 0 // 7 !
gcc/fortran/module.c: || strncmp (name, "__vtype_", 6) == 0)) // 8!
gcc/fortran/module.c: || (strncmp (name, "__vtab_", 5) != 0 // 7!
gcc/fortran/module.c: && strncmp (name, "__vtype_", 6)
!= 0)) // 8!

Here the new macro could be applied (and in a few other cases as
well), see attached patch.

I'm regtesting the patch now. Ok for trunk if it passes?

Cheers,
Janus

[Janus Weil]

Post by Janus Weil
Instead, for the sake of gfortran, how about a macro like this?
#define gfc_str_startswith(str, pref) \
(strncmp ((str), (pref), strlen (pref)) == 0)
(In fact I just noticed that something like this already exists in
trans-intrinsic.c, so I would just move it into gfortran.h and rename
it.)

Post by Bernhard Reutner-Fischer
Looking at gcc/fortran alone there are
gcc/fortran/interface.c: if (strncmp (mode, "unformatted", 9) == 0) // 11 !

I think this one could actually be a 'strcmp'?

Post by Bernhard Reutner-Fischer
gcc/fortran/module.c: && (strncmp (name, "__vtab_", 5) == 0 // 7 !
gcc/fortran/module.c: || strncmp (name, "__vtype_", 6) == 0)) // 8!
gcc/fortran/module.c: || (strncmp (name, "__vtab_", 5) != 0 // 7!
gcc/fortran/module.c: && strncmp (name, "__vtype_", 6)
!= 0)) // 8!

Here the new macro could be applied (and in a few other cases as
well), see attached patch.
I'm regtesting the patch now. Ok for trunk if it passes?

The regtest was successful. I don't think the off-by-two error for the
vtab/vtype comparisons is a big problem in practice, since the number
of internal symbols with leading underscores is very limited, but of
course it should still be fixed ...

Cheers,
Janus

[Bernhard Reutner-Fischer]

Post by Janus Weil
The regtest was successful. I don't think the off-by-two error for the
vtab/vtype comparisons is a big problem in practice, since the number
of internal symbols with leading underscores is very limited, but of
course it should still be fixed ...

Luckily it should make no difference indeed as "__vta" and "__vtyp"
are only used for this one purpose.
I don't think the DTIO op keyword fix would hit any real user either.
Thanks for taking care of it, patch LGTM.

cheers,

[Janus Weil]

Am Mi., 19. Sep. 2018 um 16:50 Uhr schrieb Bernhard Reutner-Fischer

Post by Bernhard Reutner-Fischer

Post by Janus Weil
The regtest was successful. I don't think the off-by-two error for the
vtab/vtype comparisons is a big problem in practice, since the number
of internal symbols with leading underscores is very limited, but of
course it should still be fixed ...

Luckily it should make no difference indeed as "__vta" and "__vtyp"
are only used for this one purpose.
I don't think the DTIO op keyword fix would hit any real user either.
Thanks for taking care of it, patch LGTM.

I have now committed this as r264448.

Cheers,
Janus