diff mbox

[Ada] Fix bogus noreturn warning

Message ID AANLkTin5fEghY_T3eap7tb6IpwpduxjnW0cGR8mkYHji@mail.gmail.com
State New
Headers show

Commit Message

Steven Bosscher June 27, 2010, 1:10 p.m. UTC
On Sun, Jun 27, 2010 at 2:45 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> What is this necessary for? No front end should have to know
>> *anything* about tree-flow.h.
>
> block_may_fallthru is declared there.  See c-typeck.c, cp/decl.c and cp/tree.c
> for pre-existing counter-examples.

I'm not looking for counter-examples, just stating the way things
should be, not how things are (the Germans have a beautiful way of
expressing this: "Soll-Stand" vs. "Ist-Stand").  Your change moves us
further away from the way things should be. I hope you agree with
that?

Mind if I add a note, as below?

Ciao!
Steven

Comments

Eric Botcazou June 27, 2010, 1:15 p.m. UTC | #1
> Your change moves us further away from the way things should be. I hope you
> agree with that?

No, I don't, I haven't "broken" anything that wasn't already "broken".
Steven Bosscher June 27, 2010, 1:35 p.m. UTC | #2
On Sun, Jun 27, 2010 at 3:15 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Your change moves us further away from the way things should be. I hope you
>> agree with that?
>
> No, I don't, I haven't "broken" anything that wasn't already "broken".

Dude, breath in, breath out, breath in, breath out, and relax! :-)

I am not saying you broke anything!

I am just saying that, ideally, in a perfect world, you would not
*have* to include tree-flow.h.

If you don't agree with that, also fine. But please don't feel so
attacked for no reason.

Ciao!
Steven
Manuel López-Ibáñez June 27, 2010, 1:52 p.m. UTC | #3
On 27 June 2010 15:10, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Sun, Jun 27, 2010 at 2:45 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> What is this necessary for? No front end should have to know
>>> *anything* about tree-flow.h.
>>
>> block_may_fallthru is declared there.  See c-typeck.c, cp/decl.c and cp/tree.c
>> for pre-existing counter-examples.
>
> I'm not looking for counter-examples, just stating the way things
> should be, not how things are (the Germans have a beautiful way of
> expressing this: "Soll-Stand" vs. "Ist-Stand").  Your change moves us
> further away from the way things should be. I hope you agree with
> that?

In fact, block_may_fallthru is defined in gimple-low.c and it does not
seem to use anything from tree-flow.h, so probably could be moved to
the tree interface for FEs.

Manuel.
Manuel López-Ibáñez June 27, 2010, 2:57 p.m. UTC | #4
On 27 June 2010 15:52, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> On 27 June 2010 15:10, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>> On Sun, Jun 27, 2010 at 2:45 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>> What is this necessary for? No front end should have to know
>>>> *anything* about tree-flow.h.
>>>
>>> block_may_fallthru is declared there.  See c-typeck.c, cp/decl.c and cp/tree.c
>>> for pre-existing counter-examples.
>>
>> I'm not looking for counter-examples, just stating the way things
>> should be, not how things are (the Germans have a beautiful way of
>> expressing this: "Soll-Stand" vs. "Ist-Stand").  Your change moves us
>> further away from the way things should be. I hope you agree with
>> that?
>
> In fact, block_may_fallthru is defined in gimple-low.c and it does not
> seem to use anything from tree-flow.h, so probably could be moved to
> the tree interface for FEs.

Steven,

should FEs include gimple.h? Otherwise, how can they call functions in
gimplify.c?

Cheers,

Manuel.
Steven Bosscher June 27, 2010, 3:15 p.m. UTC | #5
On Sun, Jun 27, 2010 at 4:57 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> should FEs include gimple.h? Otherwise, how can they call functions in
> gimplify.c?

I talked about this with Diego, and we think there should be a
gimplify.h, or all the middle-end #includes should be stripped from
gimple.h (i.e. things front ends should not have to know about, such
as basic-block.h, and tree-ssa*).

But this is difficult, due to all the static inline functions in gimple.h...

Anyway, front ends obviously need to construct gimple, so including
gimple.h is OK, for lack of a better alternative (for now).

Ciao!
Steven
Manuel López-Ibáñez June 27, 2010, 3:17 p.m. UTC | #6
On 27 June 2010 16:57, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> On 27 June 2010 15:52, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
>> On 27 June 2010 15:10, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>>> On Sun, Jun 27, 2010 at 2:45 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>>> What is this necessary for? No front end should have to know
>>>>> *anything* about tree-flow.h.
>>>>
>>>> block_may_fallthru is declared there.  See c-typeck.c, cp/decl.c and cp/tree.c
>>>> for pre-existing counter-examples.
>>>
>>> I'm not looking for counter-examples, just stating the way things
>>> should be, not how things are (the Germans have a beautiful way of
>>> expressing this: "Soll-Stand" vs. "Ist-Stand").  Your change moves us
>>> further away from the way things should be. I hope you agree with
>>> that?
>>
>> In fact, block_may_fallthru is defined in gimple-low.c and it does not
>> seem to use anything from tree-flow.h, so probably could be moved to
>> the tree interface for FEs.
>
> Steven,
>
> should FEs include gimple.h? Otherwise, how can they call functions in
> gimplify.c?
>

It doesn't matter much because there are further dependencies.
c-typeck.c requires function.h and predict.h at least. Currently, it
gets them through tree-flow.h, so there is no easy way to remove
tree-flow.h from here. Since I don't know which headers are supposed
to define the interface for FEs, it is unclear what needs to be moved
elsewhere. How can anyone know that they are trespassing the interface
between FEs and the rest of the compiler when such interface is not
defined anywhere. Why tree-flow.h should not be allowed?

Manuel.
Steven Bosscher June 27, 2010, 3:26 p.m. UTC | #7
On Sun, Jun 27, 2010 at 5:17 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:

> How can anyone know that they are trespassing the interface
> between FEs and the rest of the compiler when such interface is not
> defined anywhere.

Defining an interface is what I've been trying to do, to some extend,
with the project to remove include files from the front ends and/or
move function prototypes around to files that front ends do need (e.g.
tree.h).

But people have got this thing called common sense, and it wouldn't
hurt to use it sometimes. E.g. when, ever, does a front end need to
know about hard register sets?

> Why tree-flow.h should not be allowed?

Do front ends perform GIMPLE data flow? No. So they should not need
tree-flow.h. Therefore the fact that front ends currently have to
include tree-flow.h is a problem.

I was hoping people would take the time to move a function out of a
header file if it is a middle-end header and a function is defined
there that is needed in a front end.

But, as usual, GCC developers prefer quick fixes over doing a thing
properly. Proof once more that hope is postponed disappointment,
especially when it comes to GCC development.

Ciao!
Steven
Manuel López-Ibáñez June 27, 2010, 4 p.m. UTC | #8
On 27 June 2010 17:26, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Sun, Jun 27, 2010 at 5:17 PM, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
>
>> How can anyone know that they are trespassing the interface
>> between FEs and the rest of the compiler when such interface is not
>> defined anywhere.
>
> Defining an interface is what I've been trying to do, to some extend,
> with the project to remove include files from the front ends and/or
> move function prototypes around to files that front ends do need (e.g.
> tree.h).

I really appreciate what you are trying to do but I insist once more
that the ideal interface should be first defined somewhere for people
to read, and then start moving things around to match it. The other
way around will get quickly reverted, as we are seeing. I am actually
surprised this is not happening more often.

>> Why tree-flow.h should not be allowed?
>
> Do front ends perform GIMPLE data flow? No. So they should not need
> tree-flow.h. Therefore the fact that front ends currently have to
> include tree-flow.h is a problem.

For me that is a good explanation. I wish somewhere will list which
headers are suitable for FEs and tree-flow.h not listed there with the
above explanation.

> I was hoping people would take the time to move a function out of a
> header file if it is a middle-end header and a function is defined
> there that is needed in a front end.

The problem is that this is not a trivial task, for two reasons: 1)
where to move it? There is no clearly defined interface between FEs
and other parts of the compiler (is tree.h, gimple.h, basic-block.h?)
2) the chain of hidden dependencies that is broken when such move
occurs.

Why block_may_fallthru is declared in tree-flow.h? It is defined in
gimple-low.c Should be in gimple.h? It doesn't seem to use anything
gimple related. Who may call this function? Notice that there is also
a gimple_stmt_may_fallthru. This is just a little example of why it is
so much easier to just add another header. (And why the GCC code
appears to be a mess for any outsider).

The above are reasons why I think that the headers cleanup, despite
being something eventually necessary, it is pointless to do before
fully specifying the boundaries between modules. Without that, someone
will come and undo your work. Once the boundaries and dependencies are
defined, then moving things around and cleaning up the headers
dependencies is a fairly mechanic task that people will be "forced" to
do whenever they want to use a function that is currently misplaced.

Cheers,

Manuel.
Steven Bosscher June 27, 2010, 4:21 p.m. UTC | #9
On Sun, Jun 27, 2010 at 6:00 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 27 June 2010 17:26, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>> On Sun, Jun 27, 2010 at 5:17 PM, Manuel López-Ibáñez
>> <lopezibanez@gmail.com> wrote:
>>
>>> How can anyone know that they are trespassing the interface
>>> between FEs and the rest of the compiler when such interface is not
>>> defined anywhere.
>>
>> Defining an interface is what I've been trying to do, to some extend,
>> with the project to remove include files from the front ends and/or
>> move function prototypes around to files that front ends do need (e.g.
>> tree.h).
>
> I really appreciate what you are trying to do but I insist once more
> that the ideal interface should be first defined somewhere for people
> to read, and then start moving things around to match it.

The problem is that things are so much entangled now, that it really
is just a matter of disentangling the worst first, then define
something and move to match it.


> The other
> way around will get quickly reverted, as we are seeing. I am actually
> surprised this is not happening more often.

It is not that surprising, because for the most part nothing has been
done yet. I've only cleaned up the really obvious parts so far.


>> I was hoping people would take the time to move a function out of a
>> header file if it is a middle-end header and a function is defined
>> there that is needed in a front end.
>
> The problem is that this is not a trivial task, for two reasons: 1)
> where to move it? There is no clearly defined interface between FEs
> and other parts of the compiler (is tree.h, gimple.h, basic-block.h?)
> 2) the chain of hidden dependencies that is broken when such move
> occurs.
>
> Why block_may_fallthru is declared in tree-flow.h? It is defined in
> gimple-low.c Should be in gimple.h? It doesn't seem to use anything
> gimple related. Who may call this function? Notice that there is also
> a gimple_stmt_may_fallthru. This is just a little example of why it is
> so much easier to just add another header. (And why the GCC code
> appears to be a mess for any outsider).

Fully agreed.  I also don't know off hand where block_may_fallthru
should be moved.

The silly thing is, it is even mis-named because it doesn't look at
blocks but at statement lists.  So it could live in tree.h, or
tree-iterator.h perhaps. Not basic-block.h or gimple.h because it
doesn't have anything to do with those.


> The above are reasons why I think that the headers cleanup, despite
> being something eventually necessary, it is pointless to do before
> fully specifying the boundaries between modules. Without that, someone
> will come and undo your work. Once the boundaries and dependencies are
> defined, then moving things around and cleaning up the headers
> dependencies is a fairly mechanic task that people will be "forced" to
> do whenever they want to use a function that is currently misplaced.

This is how things would be done in an environment with strong
leadership and a common design plan. But this is not how I would
describe the GCC community. You wouldn't be able to force anyone to do
anything, and you have to move things around before you can enforce a
boundary. This is what I have tried with the RTL headers (and note:
the 3 places that violate this have not received any attention to
finish the job).

Ciao!
Steven
Manuel López-Ibáñez June 27, 2010, 4:40 p.m. UTC | #10
On 27 June 2010 18:21, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>>
>> I really appreciate what you are trying to do but I insist once more
>> that the ideal interface should be first defined somewhere for people
>> to read, and then start moving things around to match it.
>
> The problem is that things are so much entangled now, that it really
> is just a matter of disentangling the worst first, then define
> something and move to match it.

I see. Still, the most important part is "define something". I guess
also the most difficult.

> The silly thing is, it is even mis-named because it doesn't look at
> blocks but at statement lists.  So it could live in tree.h, or
> tree-iterator.h perhaps. Not basic-block.h or gimple.h because it
> doesn't have anything to do with those.

Somehow, I am not surprise to hear this. ;-)

>> The above are reasons why I think that the headers cleanup, despite
>> being something eventually necessary, it is pointless to do before
>> fully specifying the boundaries between modules. Without that, someone
>> will come and undo your work. Once the boundaries and dependencies are
>> defined, then moving things around and cleaning up the headers
>> dependencies is a fairly mechanic task that people will be "forced" to
>> do whenever they want to use a function that is currently misplaced.
>
> This is how things would be done in an environment with strong
> leadership and a common design plan. But this is not how I would
> describe the GCC community. You wouldn't be able to force anyone to do
> anything, and you have to move things around before you can enforce a
> boundary. This is what I have tried with the RTL headers (and note:
> the 3 places that violate this have not received any attention to
> finish the job).

I don't think you can force anyone to do something, but if somewhere
it is written that tree-flow.h cannot be included in FEs, then you
won't convince anyone to fix the existing cases but you will give a
reason to reviewers to reject a patch and ask for an alternative
version (or to fix the misplaced functions). Of course, we know that
maintainers often break their own rules (no testcases, no comments,
etc.). If the boundary was even stronger (e.g., to include tree-flow.h
in the FE you would need to modify the "tree" module), then FE
maintainers will need middle-end permission to break the boundaries.

Cheers,

Manuel.
Manuel López-Ibáñez June 27, 2010, 4:43 p.m. UTC | #11
On 27 June 2010 17:15, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Sun, Jun 27, 2010 at 4:57 PM, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
>> should FEs include gimple.h? Otherwise, how can they call functions in
>> gimplify.c?
>
> I talked about this with Diego, and we think there should be a
> gimplify.h, or all the middle-end #includes should be stripped from
> gimple.h (i.e. things front ends should not have to know about, such
> as basic-block.h, and tree-ssa*).
>
> But this is difficult, due to all the static inline functions in gimple.h...

gimplify.h would only need the type definitions, that is, gimple*
stuff. I guess those could be forward defined, no? But how would you
forward define gimple type?

Cheers,

Manuel.
Steven Bosscher June 27, 2010, 4:51 p.m. UTC | #12
On Sun, Jun 27, 2010 at 6:43 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 27 June 2010 17:15, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>> On Sun, Jun 27, 2010 at 4:57 PM, Manuel López-Ibáñez
>> <lopezibanez@gmail.com> wrote:
>>> should FEs include gimple.h? Otherwise, how can they call functions in
>>> gimplify.c?
>>
>> I talked about this with Diego, and we think there should be a
>> gimplify.h, or all the middle-end #includes should be stripped from
>> gimple.h (i.e. things front ends should not have to know about, such
>> as basic-block.h, and tree-ssa*).
>>
>> But this is difficult, due to all the static inline functions in gimple.h...
>
> gimplify.h would only need the type definitions, that is, gimple*
> stuff. I guess those could be forward defined, no? But how would you
> forward define gimple type?

The idea was this:

* all functions from gimplify.c => prototype in gimplify.h
* all gimple_build_* functions => gimplify.h


So I had a patch for this. Then what happened is this:

* gimplify.h must include predict.h for gimple_build_predict
* gimplify.h must define gimple_code because some front ends look at
it (See the OpenMP and gimplification code. I don't think they have to
because it's just lowering. But currently they do.)
* gimplify.h must include enum gimple_try_flags for gimple_build_try

This is all still sort-of OK, but not what I had hoped for.


But then, I ran into things where you also have to expose the
underlying gimple data structures:

* there are many "static inline" functions in gimple.h that are used
in the front ends, e.g. gimple_set_location, gimple_label_label
* some of the predicates are used in the gimplification code, and some
of these predicates are also static inline functions


At this point I gave up.

Caio!
Steven
Joseph Myers June 27, 2010, 6:03 p.m. UTC | #13
On Sun, 27 Jun 2010, Manuel López-Ibáñez wrote:

> I really appreciate what you are trying to do but I insist once more
> that the ideal interface should be first defined somewhere for people
> to read, and then start moving things around to match it. The other

I would strongly advocate incremental development in this area as in 
others.  Small patches can be made that improve modularity in one area 
without needing a much larger design for what the interfaces will end up 
looking like.  It is not necessary to work out the issues with a later 
interface change in order to implement an earlier change.  I didn't need 
to work out how toplev.h and toplev.c should end up getting split up to 
move diagnostics functions to diagnostic-core.h, nor does anyone need such 
a design to change individual files including toplev.h to include 
diagnostic-core.h instead if that's all they needed from toplev.h (or more 
generally to remove unnecessary #includes and fix cases where header A is 
only included for declarations in header B that is included by A).

Working out the ideal interface means understanding what all the 
unexpected interactions are between different parts of the compiler.  I 
claim that to discover such an interaction in practice you generally need 
to try to remove what looks like an inappropriate include and thereby find 
why it is still needed, and that a design not produced in the course of 
incrementally cleaning things up is going to miss many of these issues 
that need resolving in practice.
Manuel López-Ibáñez June 27, 2010, 6:52 p.m. UTC | #14
On 27 June 2010 20:03, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Sun, 27 Jun 2010, Manuel López-Ibáñez wrote:
>
>> I really appreciate what you are trying to do but I insist once more
>> that the ideal interface should be first defined somewhere for people
>> to read, and then start moving things around to match it. The other
>
> I would strongly advocate incremental development in this area as in
> others.  Small patches can be made that improve modularity in one area

You do not need any patch to define what and what not should be
included by other parts of the compiler. I am asking for that. A
simple question: what headers should and should not be included
(ideally) by FEs?

> without needing a much larger design for what the interfaces will end up
> looking like.  It is not necessary to work out the issues with a later
> interface change in order to implement an earlier change.  I didn't need
> to work out how toplev.h and toplev.c should end up getting split up to
> move diagnostics functions to diagnostic-core.h, nor does anyone need such
> a design to change individual files including toplev.h to include
> diagnostic-core.h instead if that's all they needed from toplev.h (or more
> generally to remove unnecessary #includes and fix cases where header A is
> only included for declarations in header B that is included by A).

I think the point is not that a header file is necessary or
unnecessary. If the function that you need is declared in that header,
then one can always make the case that the header *is* necessary. The
point is that it is the wrong header to include. Now, where is the
definition of wrong? That is a design.

I think your example is too simple: It is clear that including
toplev.h to get diagnostics functions is something odd. There is a
loosely defined diagnostic "module" and toplev.h would be a weird
interface for it.

In this case, is tree-low.h the right interface of gimple-low.c for
FEs? Well, it seems it is not clear at all.

In any case, since toplev.h still includes diagnostic-core.h, nothing
prevents anyone to add toplev.h just to get error().

> Working out the ideal interface means understanding what all the
> unexpected interactions are between different parts of the compiler.  I
> claim that to discover such an interaction in practice you generally need
> to try to remove what looks like an inappropriate include and thereby find

First you need to define what is an inappropriate include. I don't
think this is generally known. And, if it is known, it is ignored in
favor of existing practice, which ends up undoing the modularization
work.

Cheers,

Manuel.
Steven Bosscher June 27, 2010, 7:14 p.m. UTC | #15
On Sun, Jun 27, 2010 at 8:52 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 27 June 2010 20:03, Joseph S. Myers <joseph@codesourcery.com> wrote:
>> On Sun, 27 Jun 2010, Manuel López-Ibáñez wrote:
>>
>>> I really appreciate what you are trying to do but I insist once more
>>> that the ideal interface should be first defined somewhere for people
>>> to read, and then start moving things around to match it. The other
>>
>> I would strongly advocate incremental development in this area as in
>> others.  Small patches can be made that improve modularity in one area
>
> You do not need any patch to define what and what not should be
> included by other parts of the compiler. I am asking for that. A
> simple question: what headers should and should not be included
> (ideally) by FEs?

The following is the minimum set I am aiming for:
config.h
convert.h
coretypes.h
ggc.h
gimple.h (until there is a gimplify.h)
langhooks.h
langhooks-def.h
system.h
tm.h
target.h
tree.h
tree-inline.h

And of course any plain data type header (hashtab.h, bitmap.h,
pointer-set.h, etc.) and front-end specific headers.

Ciao!
Steven
Joseph Myers June 27, 2010, 7:16 p.m. UTC | #16
On Sun, 27 Jun 2010, Manuel López-Ibáñez wrote:

> On 27 June 2010 20:03, Joseph S. Myers <joseph@codesourcery.com> wrote:
> > On Sun, 27 Jun 2010, Manuel López-Ibáñez wrote:
> >
> >> I really appreciate what you are trying to do but I insist once more
> >> that the ideal interface should be first defined somewhere for people
> >> to read, and then start moving things around to match it. The other
> >
> > I would strongly advocate incremental development in this area as in
> > others.  Small patches can be made that improve modularity in one area
> 
> You do not need any patch to define what and what not should be
> included by other parts of the compiler. I am asking for that. A
> simple question: what headers should and should not be included
> (ideally) by FEs?

My claim is that there are two answers.  One is so high-level and general 
as to be almost useless: only those headers relating to general purpose 
interfaces shared by all of GCC, or to the interfaces for lowering from 
front-end IR to GIMPLE, or to the interfaces for giving information about 
the target required by the front end.  The other, a precise list of 
headers, can only be derived through a series of incremental patches that 
create the headers in question, separate interfaces at different levels 
and result in front ends including exactly the headers they should 
include; this process will converge on the answer at the same time as the 
front ends conform to the answer, and the headers in question may not 
exist until that process of convergence is finished.  The way to get the 
precise answer is to use the general answer as a guide to the *general 
direction* of patches towards that convergence.

If someone claims they can look 20 steps ahead and see exactly what new 
headers will be needed at each stage and what declarations should go in 
what header, I don't believe them.  I don't think it's useful to look more 
than two or three steps ahead in that level of detail (say, decide you 
want to create gimplify.h, discover a problem in the course of 
implementation and so instead do a smaller patch to fix that problem 
before seeing if there are other obstacles to creating gimplify.h).

Ask not what the overall ideal interface is.  Pick a single modularity 
issue you don't see how to fix and open a discussion of possible 
approaches for that issue on its own, or pick one you do see how to fix 
and fix it.
diff mbox

Patch

Index: ada/gcc-interface/trans.c
===================================================================
--- ada/gcc-interface/trans.c   (revision 161470)
+++ ada/gcc-interface/trans.c   (working copy)
@@ -33,7 +33,7 @@ 
 #include "output.h"
 #include "libfuncs.h"  /* For set_stack_check_libfunc.  */
 #include "tree-iterator.h"
-#include "tree-flow.h"
+#include "tree-flow.h" /* For block_may_fallthru.  */
 #include "gimple.h"

 #include "ada.h"