berkus / include-what-you-use

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

Suggestion for inl files #128

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago

Hello!  have a small suggestion for IWYU.

As you know, one way to structure code is to separate the implementation of 
inline methods into inl files, that way we separate the interface from the 
implementation and we don't bloat the headers.

At the moment IWYU don't realize this, and it suggests to include the inl files 
directly in the cpp.

I attached a small sample with the situation.

What do you think? Would it be feasible to realize that this file can only be 
included from a header, and deal with the header only?

Original issue reported on code.google.com by dpun...@gmail.com on 16 Apr 2014 at 9:41

Attachments:

GoogleCodeExporter commented 9 years ago
IWYU has some logic for this, but I'm not sure what the policy is.

I *suspect* that if you name the .inl the same as your header (i.e. 
mytestclass.inl), it might understand that they belong together. I haven't had 
a chance to test it yet, but I'll get to it eventually.

Original comment by kim.gras...@gmail.com on 16 Apr 2014 at 10:01

GoogleCodeExporter commented 9 years ago

Ah that would explain, and in fact maybe I said too much. It is possible that 
you don't want the inl to be referenced from the header right?

But I wonder if in this case it should be reported, since it already exists in 
the header of the class.

Original comment by dpun...@gmail.com on 16 Apr 2014 at 11:29

GoogleCodeExporter commented 9 years ago
I've tried different variations of this, and I can't find one that behaves the 
way I would expect.

If you name the .inl file mytestclass.inl or mytestclass-inl.h, IWYU correctly 
identifies it as an associated header (e.g. associated headers for foo.cc would 
be foo.h, foo-inl.h and/or foo.inl).

However, it still suggest the .inl include should be moved from mytestclass.h 
to mytestclass.cpp, which makes no sense to me. But it could be a rule that's 
just not implemented...

What would the rule be? If the .inl contains any definitions of declarations in 
the .h, keep it there? Or should we always just leave .inl included from 
headers alone?

Original comment by kim.gras...@gmail.com on 21 Apr 2014 at 7:00

GoogleCodeExporter commented 9 years ago

I think it's a complex case... because some people decide to include the inl 
directly in the header of the class, and others only where the implementation 
is needed (searching 
http://stackoverflow.com/questions/1208028/significance-of-a-inl-file-in-c)

It seems to be a very special case.

Original comment by dpun...@gmail.com on 22 Apr 2014 at 3:21

GoogleCodeExporter commented 9 years ago
As far as I understand, there is no single rule regarding inl files.  Some 
developers prefer to include inl file only from corresponding header, others - 
include inl files from implementation files.  The latter practice is already 
supported by IWYU, it recommends to add #include "inl" to implementation files 
which use methods defined in inl file.  And the former practice can be enforced 
with the help of IWYU mappings and pragmas.  For example, foo.inl can use a 
line // IWYU pragma: private, include "foo.h".

As a summary, I think that IWYU already has enough mechanisms to support 
different policies regarding inline files.  And there is no need to implement 
additional functionality.  Do you agree?

Original comment by vsap...@gmail.com on 28 Apr 2014 at 4:20

GoogleCodeExporter commented 9 years ago

If you ask me, I agree :)

Original comment by dpun...@gmail.com on 28 Apr 2014 at 6:24

GoogleCodeExporter commented 9 years ago
I am interested in every participant's opinion. Kim, do you have something to 
add?

Original comment by vsap...@gmail.com on 29 Apr 2014 at 12:59

GoogleCodeExporter commented 9 years ago
Not really. Of course it's always better for IWYU to just transparently work, 
but if there's no unambiguous rule, I'm not sure what to do. I don't use .inl 
myself, so I don't see any patterns clearly.

The IWYU pragma seems like a good workaround.

Original comment by kim.gras...@gmail.com on 29 Apr 2014 at 4:08

GoogleCodeExporter commented 9 years ago
I'll keep in mind that special handling of inline files can be desirable.  But 
right now I am closing the issue because we haven't come up with any concrete 
IWYU enhancements.

Original comment by vsap...@gmail.com on 29 Apr 2014 at 5:11