klimek added a subscriber: klimek.
================
Comment at: include-fixer/IncludeFixer.cpp:397
@@ +396,3 @@
+ clang::tooling::Replacements Insertions;
+ if (FirstIncludeOffset == -1U) {
+ // FIXME: skip header guards.
----------------
Do we want UINT_MAX? I find the wording in the standard to be too involved for
me to easily understand that this is both well-defined and what we want and
portable.
================
Comment at: include-fixer/IncludeFixer.h:62-72
@@ -64,1 +61,13 @@
+/// Insert a header before the first #include in \p Code and run
+/// clang-format to sort all headers.
+/// \param Code The source code.
+/// \param FilePath The source file path.
+/// \param StyleName Fallback style for reformatting.
+/// \param FirstIncludeOffset The insertion point for new include directives.
+/// \param Header The header being inserted.
+/// \return Replacements for inserting and sorting headers.
+std::vector<clang::tooling::Replacement> createReplacementsForHeader(
+ StringRef Code, StringRef FilePath, StringRef FallbackStyle,
+ unsigned FirstIncludeOffset, StringRef Header);
+
----------------
1. This only needs one style, so I think we should pass it in instead of
FilePath and FallbackStyle
2. From the docs it seems like FirstIncludeOffset should always be INT_MAX? Can
we just leave out that parameter and make the function do the right thing?
3. Generally, I'd order parameters by "importance", that is, the ones that are
core to the functionality go first (C++ kinda implies this by only allowing
later parameters being defaultable); so basically, I'd make this:
.. insertHeader(StringRef Code, StringRef Header, const FormatStyle& Style);
http://reviews.llvm.org/D20621
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
