On Mar 26, 2012, at 1:56 PM, Steven Bosscher wrote:
> This patch is one way to address PR44982.

That's a great idea, I like it.  There is only one problem that I know of that 
prevents it from going in now.  We emit errors/warnings from below finalize and 
there is a feature in which we emit all the reasonable error and warning 
messages that we can with one compile.  There is a certain class of power user 
that actually wants to see all the errors or warnings in a given compile, for 
them, this is a feature.  If you want to move the analysis for those to before 
finalize, then I think this is a good way to fix the problem, until then I 
think we should fix this in the normal way.  The usual way to fix this would be 
to find the bit that had the error_mark_node in it, and then do as much as you 
can, but, then to bubble up error_mark_node or otherwise end processing.

> I see no good reason to cgraph_finalize_compilation_unit if there were parse 
> errors.

It is actually a feature people use, even if you don't have any users or your 
user base is too small to have ever met one.  I have.  The time the feature 
allows to be saved, can be measured in days and weeks.

> it seems to me that those warnings are not very meaningful after parse errors 
> (-Wuninitialized after a
> parse error??),

But you're wrong.

int f = &sdf;

main() {
  int i;
  printf("%d", i);
}

is a program that has parse errors and certainly, any and all the errors after 
the first line that don't involve f in any meaningful way, are meaningful and 
valuable to some people.  I understand you don't see the value in them, that's 
all right.  They aren't valuable or meaningful to you, I get it.  But, are you 
willing to accept it when I assert that there is a class of people for which 
this is a feature, they are meaningful and valuable?

> and errors from the middle end are mostly for exotic
> code (involving asm()s and the like). Bailing out after parse errors
> is therefore IMHO the right thing to do for the common case.

Some are for very exotic things, yes, but not all of them.  Actually, I started 
reviewing them, none of them seem that exotic to me.  Take for example:

int f() { return this->i; }   // { dg-error "" "no member named i" } 

Really, exotic?

register int y; /* { dg-warning "file-scope declaration of 'y' specifies 
'register'" } */

Again, I don't see the value in not giving the warning.

static int f2(void); /* { dg-error "used but never defined" } */

No exactly unheard of.

int x[]; /* { dg-warning "array 'x' assumed to have one element" } */

Gosh, seems reasonable.

        hash_unique_table (size_t n, hasher, key_equal,
                           value_allocator & a):table (n, a)    // { dg-error 
"is not a direct base" }   

This one I can see being nice to get with certain refactoring operations on 
large code bases.

  static_assert( I % 2 == 1, "I must be an odd number"); // { dg-error "odd 
number" }                    

What, you mean my static_asserts are gonna go away, but we _liked_ them.

    case low ... high : return i + 1;  // { dg-error "previously" }             
                         
    case 5 : return i + 2;             // { dg-error "duplicate" }              
                         

duplicate case statements, awe...  What's next?

  void Look(W& w) { w.x = 3; }          // { dg-error "private within this 
context" }

Bye bye access control, I guess it was overrated.

I was going to go through them all, but I have to stop now, my stomach doesn't 
feel well.  I was only 38% of the way though them.  Did you actually review 
them, all of them?

What is so very wrong with fixing this in the normal way?  I can envision 
beefier analysis and code checking that runs very late in the compilation 
process in order to get the most accurate analysis out of it.  It seems 
reasonable to permit that, even in the presence of parse errors, for all  the 
same reasons we want to see that a case statement was a duplicate, or that 
something was never defined.

Reply via email to