Page MenuHomePhabricator

[clang-move] The new.cc file should include new_header.h instead of old_header.h
ClosedPublic

Authored by hokein on Sep 22 2016, 7:07 AM.

Details

Summary

Previously, all #includes (includeing old_header.h) in old.cc will be copied to new.cc,
however, the new.cc should include new_header.h instead of the old_header.h

Before applying the patch, the new.cc looks like:

#include "old_header.h"
...

The new.cc looks like with this patch:

#include "new_header"
...

Diff Detail

Repository
rL LLVM

Event Timeline

hokein updated this revision to Diff 72165.Sep 22 2016, 7:07 AM
hokein retitled this revision from to [clang-move] Don't add old_header to the new files..
hokein updated this object.
hokein added a reviewer: ioeric.
hokein added a subscriber: cfe-commits.
ioeric edited edge metadata.Sep 22 2016, 7:09 AM

Could you elaborate on what this CL does in the description?

hokein retitled this revision from [clang-move] Don't add old_header to the new files. to [clang-move] The new.cc file should include new_header.h instead of old_header.h.Sep 22 2016, 7:27 AM
hokein updated this object.
hokein edited edge metadata.
hokein updated this object.
ioeric accepted this revision.Sep 22 2016, 8:03 AM
ioeric edited edge metadata.

Lg with nits.

clang-move/ClangMove.cpp
290 ↗(On Diff #72165)

I think we also need a FIXME here. There could be interesting dependency: if the new target depends on the old target, then it makes sense to add the header.

295 ↗(On Diff #72165)

I think ("#include <" + IncludeHeader + ">\n").str() is more efficient since it only creates one string instance.

This revision is now accepted and ready to land.Sep 22 2016, 8:03 AM
hokein updated this revision to Diff 72265.Sep 23 2016, 6:26 AM
hokein marked 2 inline comments as done.
hokein edited edge metadata.

Add comments.

This revision was automatically updated to reflect the committed changes.