fix_includes incorrectly modifies files when two forward declares already exist

[GoogleCodeExporter]

clang/llvm/iwyu all on trunk (3.7)

fix_includes incorrectly modifies files when two forward declares already exist.

output of iwyu:
components/esm/objectstate.hpp should add these lines:
#include "components/esm/defs.hpp"      // for Position

components/esm/objectstate.hpp should remove these lines:
- #include <string>  // lines 4-4
- #include <vector>  // lines 5-5

The full include-list for components/esm/objectstate.hpp:
#include "cellref.hpp"                  // for CellRef
#include "components/esm/defs.hpp"      // for Position
#include "locals.hpp"                   // for Locals
namespace ESM { class ESMReader; }  // lines 12-12
namespace ESM { class ESMWriter; }  // lines 13-13

when fix_includes applies the changes:
--- a/components/esm/objectstate.hpp
+++ b/components/esm/objectstate.hpp
@@ -1,15 +1,15 @@
 #ifndef OPENMW_ESM_OBJECTSTATE_H
 #define OPENMW_ESM_OBJECTSTATE_H

-#include <string>
-#include <vector>
-
 #include "cellref.hpp"
+#include "components/esm/defs.hpp"
+#include "components/esm/defs.hpp"
 #include "locals.hpp"

 namespace ESM
-{
     class ESMReader;
+{
+
     class ESMWriter;

     // format 0, saved games only

As you can see, it moved the { to after the class definition when it shouldn't 
have done anything.

Using "$ fix_includes -m" works properly and doesn't touch the forward 
declarations.

--- a/components/esm/objectstate.hpp
+++ b/components/esm/objectstate.hpp
@@ -1,10 +1,8 @@
 #ifndef OPENMW_ESM_OBJECTSTATE_H
 #define OPENMW_ESM_OBJECTSTATE_H

-#include <string>
-#include <vector>
-
 #include "cellref.hpp"
+#include "components/esm/defs.hpp"
 #include "locals.hpp"

 namespace ESM

Original issue reported on code.google.com by showard...@gmail.com on 7 Feb 2015 at 3:07

[GoogleCodeExporter]

Thanks for report, showard314. Strangely, I wasn't able to reproduce the issue 
with the following test case

#include "tests/cxx/direct.h"
#include <vector>

namespace NS
{
    class Bar;
    class Foo;
}

void foo() {
    NS::Foo *f = 0;
    NS::Bar *b = 0;
    IndirectClass ic;
}

So I'm not sure if the issue is caused by existing forward declarations. For me

    +#include "components/esm/defs.hpp"
    +#include "components/esm/defs.hpp"

looks suspicious. Maybe fixes were applied to the file multiple times? 
Especially that it is .hpp and included from multiple files. Do you have any 
ideas?

Original comment by vsap...@gmail.com on 10 Feb 2015 at 3:04

[GoogleCodeExporter]

Ah, I wasn't using trunk fix_includes (it defaulted to system) trunk works, 
thanks - please close!

Original comment by showard...@gmail.com on 11 Feb 2015 at 2:59

[GoogleCodeExporter]

OK, thanks!

Original comment by kim.gras...@gmail.com on 11 Feb 2015 at 5:40

• Changed state: NotABug