This is an archive of the discontinued LLVM Phabricator instance.

[include-fixer] Use YAML format in -output-headers and -insert-header mode.
ClosedPublic

Authored by hokein on May 31 2016, 11:16 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

hokein updated this revision to Diff 59097.May 31 2016, 11:16 AM
hokein updated this revision to Diff 59100.
hokein retitled this revision from to [include-fixer] Use YAML format in -output-headers and -insert-header mode..
hokein updated this object.
hokein added a reviewer: bkramer.
hokein added a subscriber: cfe-commits.

Remove unused code.

bkramer added inline comments.May 31 2016, 11:45 AM
include-fixer/tool/ClangIncludeFixer.cpp
80 ↗(On Diff #59100)

drop 'the'

include-fixer/tool/clang-include-fixer.py
22 ↗(On Diff #59100)

We cannot rely on python-yaml being available, I don't want users to install python modules just to run include-fixer. Can we restrict ourselves to the JSON subset of YAML here?

hokein updated this revision to Diff 59183.Jun 1 2016, 2:03 AM

Get rid of yaml dependency, using json module.

hokein marked 2 inline comments as done.Jun 1 2016, 2:04 AM
hokein updated this revision to Diff 59184.Jun 1 2016, 2:18 AM

Show error message when clang-include-fixer died with a fatal error.

bkramer added inline comments.Jun 1 2016, 2:32 AM
include-fixer/tool/ClangIncludeFixer.cpp
160 ↗(On Diff #59184)

Can you just llvm::yaml::escape the string?

hokein updated this revision to Diff 59185.Jun 1 2016, 2:52 AM

Use llvm::yaml::escape to escape double quote.

hokein marked an inline comment as done.Jun 1 2016, 3:08 AM
bkramer added inline comments.Jun 1 2016, 3:09 AM
include-fixer/tool/ClangIncludeFixer.cpp
160 ↗(On Diff #59185)

I think we should do the escaping always and not just when the first character is a quote.

hokein updated this revision to Diff 59187.Jun 1 2016, 3:13 AM

Always escape headers.

hokein marked an inline comment as done.Jun 1 2016, 3:13 AM
bkramer accepted this revision.Jun 1 2016, 3:40 AM
bkramer edited edge metadata.
This revision is now accepted and ready to land.Jun 1 2016, 3:40 AM
This revision was automatically updated to reflect the committed changes.