On Feb 5, 2014, at 11:18 AM, Richard Smith <[email protected]> wrote:

> I'm not especially fond of treating annot_module_begin and 
> annot_module_include differently. That'll mean we get completely different 
> diagnostics for a '#include' in a bad place depending on whether it's 
> including something in the same top-level module or something in a different 
> top-level module.
> 

I approached this from the other side and didn't like that annot_module_include 
is handled so differently from @import.

> I'm also not sure that the diagnostic change here is an improvement. 
> Previously:
> 
> #include "not_module.h"
> #include "module.h"
> 
> ... where not_module.h contains:
> 
> class X {
>   // ...
> // whoops, forgot the '};'
> 
> would give a "missing '}'" diagnostic, pointing at the '{' that is left 
> un-closed. It now gives a mysterious 'must be at the top level' diagnostic, 
> not pointing at the '}' any more, followed by more errors (because 'module.h' 
> didn't get imported and because the code after the #include is still within 
> the class). The error here was almost certainly that a '}' was missing, so 
> this seems like a regression.

Ick, I’ll take a look at this case.

> 
> In what cases does this change allow us to recover from errors better?


Whenever an implicit import is not at top level, we currently give errors like 
‘expected expression’, or ‘expected }’, pointing at the location on 
#include/#import, and give no hint as to how a preprocessor directive 
(#include) could be the source of the error.  To make matters worse, once you 
break out of whatever parsing context you were in due to the module_include, 
any following code is likely to become errors.

int foo() {
  int x = 5;
  #import “Bar.h” // expected }
  if (x == 6) {// expected identifier or (
    ++x; // expected identifier or (
  }
  return 1; // expected identifier of (
} // extraneous closing brace

Ben
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to