* [Patch, Fortran] PR 50163 - ICE with nonconst expr in init expr
@ 2011-08-23 14:19 Tobias Burnus
2011-08-24 11:25 ` Mikael Morin
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Burnus @ 2011-08-23 14:19 UTC (permalink / raw)
To: gcc patches, gfortran
[-- Attachment #1: Type: text/plain, Size: 437 bytes --]
The bug is a regression: An error was printed with 4.1.x but since 4.3.x
one gets an ICE. [No idea what GCC 4.2 does.] The solution is simply:
Returning if there is a MATCH_ERROR.
See PR (esp. comment 2) for a more detailed description:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50163#c0
Build and regtested on x86-64-linux.
OK for the trunk? And to which version should it be backported? Only
4.6? Also 4.5? Or even 4.4?
Tobias
[-- Attachment #2: init-err-fix.diff --]
[-- Type: text/x-patch, Size: 1010 bytes --]
2011-08-23 Tobias Burnus <burnus@net-b.de>
PR fortran/50163
* check_init_expr (check_init_expr): Return when an error occured.
2011-08-23 Tobias Burnus <burnus@net-b.de>
PR fortran/50163
* gfortran.dg/initialization_28.f90: New.
diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index 9922094..b050b11 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -2481,6 +2481,9 @@ check_init_expr (gfc_expr *e)
m = MATCH_ERROR;
}
+ if (m == MATCH_ERROR)
+ return FAILURE;
+
/* Try to scalarize an elemental intrinsic function that has an
array argument. */
isym = gfc_find_function (e->symtree->n.sym->name);
--- /dev/null 2011-08-23 07:28:57.751883742 +0200
+++ gcc/gcc/testsuite/gfortran.dg/initialization_28.f90 2011-08-23 14:02:02.000000000 +0200
@@ -0,0 +1,9 @@
+! { dg-do compile }
+!
+! PR fortran/50163
+!
+! Contributed by Philip Mason
+!
+character(len=2) :: xx ='aa'
+integer :: iloc=index(xx,'bb') ! { dg-error "has not been declared or is a variable" }
+end
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, Fortran] PR 50163 - ICE with nonconst expr in init expr
2011-08-23 14:19 [Patch, Fortran] PR 50163 - ICE with nonconst expr in init expr Tobias Burnus
@ 2011-08-24 11:25 ` Mikael Morin
2011-08-24 14:23 ` Tobias Burnus
0 siblings, 1 reply; 4+ messages in thread
From: Mikael Morin @ 2011-08-24 11:25 UTC (permalink / raw)
To: fortran; +Cc: Tobias Burnus, gcc patches
On Tuesday 23 August 2011 14:26:59 Tobias Burnus wrote:
> The bug is a regression: An error was printed with 4.1.x but since 4.3.x
> one gets an ICE. [No idea what GCC 4.2 does.] The solution is simply:
> Returning if there is a MATCH_ERROR.
>
> See PR (esp. comment 2) for a more detailed description:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50163#c0
>
> Build and regtested on x86-64-linux.
> OK for the trunk?
OK.
> And to which version should it be backported? Only
> 4.6? Also 4.5? Or even 4.4?
I have no strong opinion about it; I would say 4.6 and 4.5.
If you want 4.4 too.
Isn't there some rules about backporting? The way we do it now, it looks
completely arbitrary.
Mikael
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, Fortran] PR 50163 - ICE with nonconst expr in init expr
2011-08-24 11:25 ` Mikael Morin
@ 2011-08-24 14:23 ` Tobias Burnus
2011-08-25 17:42 ` Mikael Morin
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Burnus @ 2011-08-24 14:23 UTC (permalink / raw)
To: Mikael Morin; +Cc: fortran, gcc patches
On 08/24/2011 12:12 PM, Mikael Morin wrote:
> On Tuesday 23 August 2011 14:26:59 Tobias Burnus wrote:
>> OK for the trunk?
> OK.
Thanks for the review! Committed to the 4.7 trunk (Rev. 178038); I will
backport the patch later. (For cross-readers: That's a patch for a 4.3+
ice-on-invalid-code regression.)
> Isn't there some rules about backporting? The way we do it now, it
> looks completely arbitrary.
I think it *is* arbitrary - and unavoidable so.
The main idea behind regression fixing is to make sure that what once
worked should continue to work. But what always had been broken can
remain broken and will be only fixed on the trunk. Reason: If you fix
more, the behaviour on the branch changes and you may introduce
regressions. If thinks are known to be broken, you can simply work
around them.
Additional ingredients are: How serious is the problem? A wrong-code
issue occurring in a potentially often used part has a different
priority than an accepts-invalid or ice-on-invalid-code issue. Also, a
patch which is huge is less suited than a small "trivial" patch.
Regressions, which existed for a long time are typically also less
important - otherwise they would have been fixed or found before.
But there are also other items such as: Which is the last maintained
version in GCC, which versions are still being used (such that it makes
sense to backport), and which patches (Linux) distributions want to see.
Additionally, as backporting takes time (bootstrap, regtesting, and
maybe even adapting the patch slightly): How much time wants the
developer spend on backporting.
My impression is that gfortran is currently doing too much
non-regression backporting, which should be left to serious ICE-on-valid
code and wrong-code issues. Especially as older versions do not see as
much testing as the trunk.
On the other hand, backporting simple fixes to regressions or really bad
wrong-code issues (which we hadn't for a while) down to 4.4 should be
also fine.
I think every modern (= F2003) developer can relatively simply update to
4.6 or the trunk. The older versions are mostly for those using the
default compiler of the system; those typically only need some Fortran
77/90 compiler, for which the older versions (4.4 or 4.5) should be fine.
But at the end it is question of style. That's similar to Linux
distributions. I think Jakub/Red Hat ports many patches (bug fixes but
also features) back to their old versions, while for instance
Richard/SUSE's package has only few patches and essentially grabs the
current branch. Both approaches makes sense and either one has
advantages and disadvantages.
I think we should try extra hard to avoid regressions on the branches
and mostly concentrate on the trunk, but we can still backport one patch
or the other to the branches.
I didn't really answer your question, did I?
Tobias
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, Fortran] PR 50163 - ICE with nonconst expr in init expr
2011-08-24 14:23 ` Tobias Burnus
@ 2011-08-25 17:42 ` Mikael Morin
0 siblings, 0 replies; 4+ messages in thread
From: Mikael Morin @ 2011-08-25 17:42 UTC (permalink / raw)
To: fortran; +Cc: Tobias Burnus, gcc patches
On Wednesday 24 August 2011 15:31:17 Tobias Burnus wrote:
> > Isn't there some rules about backporting? The way we do it now, it
> > looks completely arbitrary.
>
> I think it *is* arbitrary - and unavoidable so.
>
> The main idea behind regression fixing is to make sure that what once
> worked should continue to work. But what always had been broken can
> remain broken and will be only fixed on the trunk. Reason: If you fix
> more, the behaviour on the branch changes and you may introduce
> regressions. If thinks are known to be broken, you can simply work
> around them.
>
> Additional ingredients are: How serious is the problem? A wrong-code
> issue occurring in a potentially often used part has a different
> priority than an accepts-invalid or ice-on-invalid-code issue. Also, a
> patch which is huge is less suited than a small "trivial" patch.
> Regressions, which existed for a long time are typically also less
> important - otherwise they would have been fixed or found before.
>
> But there are also other items such as: Which is the last maintained
> version in GCC, which versions are still being used (such that it makes
> sense to backport), and which patches (Linux) distributions want to see.
> Additionally, as backporting takes time (bootstrap, regtesting, and
> maybe even adapting the patch slightly): How much time wants the
> developer spend on backporting.
>
OK, it's a complex problem, and that's the very reason for my remark.
> My impression is that gfortran is currently doing too much
> non-regression backporting, which should be left to serious ICE-on-valid
> code and wrong-code issues. Especially as older versions do not see as
> much testing as the trunk.
>
I didn't have that impression; a matter of style probably.
Yes we could try to be more carefull in the future.
> [...]
>
> But at the end it is question of style.
> [...]
>
Well, I was asking whether we could decide on our own style.
> [...]
>
> I didn't really answer your question, did I?
>
You exposed your point of view clearly, which is certainly a step forward.
There are some basic rules for backports, on which everybody agrees; but in
the end the same question is raised over and over again, and nobody seems to
know really: how far should we backport?
Currently the GCC rules are (basically):
not a regression -> no backport (unless serious bug/trivial fix)
regression -> backport
On the other hand, we have three open branches (trunk apart) and it is not
clear to me whether we should apply the same rules to all of them or shade the
seriousness and trivialness trigger levels into the 3 levels of backport we
have.
Furthermore we have to take into account our (lack of) ressources and (amount
of) interest for doing the backport.
So I was proposing to include version numbers into the rules, and be more
specific about them, like for example:
- Serious (wrong-code, ice-on-valid) non-regression bugs with a simple fix
are backported to N-1 only. [N is trunk]
- Non-serious regressions are not backported beyond N-2.
- ...
Of course in the end, what is simple, what is serious, are arbitrary.
Maybe you are right, it's unavoidable.
Mikael
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-08-25 16:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-23 14:19 [Patch, Fortran] PR 50163 - ICE with nonconst expr in init expr Tobias Burnus
2011-08-24 11:25 ` Mikael Morin
2011-08-24 14:23 ` Tobias Burnus
2011-08-25 17:42 ` Mikael Morin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).