abduld / include-what-you-use

Automatically exported from code.google.com/p/include-what-you-use
Other
0 stars 0 forks source link

Extra fwd-declare for C typdedef struct #153

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
For:
==> test.h <==
typedef struct mystruct mystruct;
mystruct* init();
========

IWYU suggests:
test.h should add these lines:
struct mystruct;

It seems unnecessary to me. Is this intentional or indeed a bug? Interestingly 
if one runs IWYU with "-x c++-header" IWYU doesn't think it needs to be added.

Original issue reported on code.google.com by rol...@rschulz.eu on 5 Jul 2014 at 5:39

GoogleCodeExporter commented 9 years ago
Such a behavior was introduced in r530 to fix issue #123.  Clang emits a 
warning in case of such elaboration-caused forward declaration

    void init(struct mystruct* s);

That's why IWYU doesn't treat such elaboration as forward declaration and 
requires explicit forward declaration.

Clang doesn't complain about

    struct mystruct* init();

But I think IWYU should require explicit forward declaration in this case too, 
for symmetry.  I guess, we can allow elaboration-caused forward declaration in 
typedefs in C because explicit forward declaration in

    struct mystruct;
    typedef struct mystruct mystruct;

looks redundant.  It's just my gut feeling, I am open for discussion.

Original comment by vsap...@gmail.com on 7 Jul 2014 at 3:57

GoogleCodeExporter commented 9 years ago
The change in r530 was too heavy-handed; I completely misinterpreted the basis 
and reach for the warning in question.

This SO answer puts the finger on it:
http://stackoverflow.com/a/10604948/96963

The Clang warning _only_ fires if the elaborated type is in a parameter list in 
a function decl, because that's apparently (off the top of my head) the only 
context where such a declaration has constrained visibility (i.e. only within 
the function body.)

Other elaborated type decls will leak out into the declaring context and be 
recognized there, e.g.

  struct mystruct* init();
  struct mystruct* p = 0;
  typedef struct mystruct mystruct;

Another interesting aspect of C, relative to C++, is that all struct references 
are elaborated (prefixed by 'struct'), so I guess forward declarations are 
often unnecessary.

So I think the rule is not: "In C, forward declarations are always required", 
but rather "In C, forward declarations are never required, except if we're in a 
parameter list."

I'll cook up a patch to that effect.

Original comment by kim.gras...@gmail.com on 16 Aug 2014 at 4:55

GoogleCodeExporter commented 9 years ago
Please try the attached patch.

Original comment by kim.gras...@gmail.com on 16 Aug 2014 at 5:52

Attachments:

GoogleCodeExporter commented 9 years ago
Patch looks good to me, doesn't break anything on Mac OS X.

Original comment by vsap...@gmail.com on 17 Aug 2014 at 7:27

GoogleCodeExporter commented 9 years ago
Thanks!

Committed in r563, with a small adjustment: added "template" keyword when 
calling templated member of current_ast_node(), for consistency with other 
calls to ParentIsA, AncestorIsA, GetParentAs, etc.

Roland, could you report back if this solves your problem in context?

Original comment by kim.gras...@gmail.com on 17 Aug 2014 at 8:39

GoogleCodeExporter commented 9 years ago
Thanks! This fixes it for us.

Original comment by rol...@rschulz.eu on 25 Aug 2014 at 3:26

GoogleCodeExporter commented 9 years ago
Wonderful, thanks!

Original comment by kim.gras...@gmail.com on 25 Aug 2014 at 6:48