ilya-biryukov added a comment.
I somehow missed the review email, sorry for the delayed response.
Just one nit and one question from me on changed behavior in the tests (quoted
vs angled #include).
Otherwise LG, just wanted to make sure the change in behavior is intentional.
================
Comment at: lib/Format/Format.cpp:2131
+inline StringRef trimInclude(StringRef IncludeName) {
+ return IncludeName.trim("\"<>");
+}
----------------
NIT: Maybe we could add some asserts to this function that the passed include
name is actually properly quoted.
E.g. starts with '<' or '"', ends with the corresponding char, etc.
So long as this is part of `Format.cpp`, it's not terribly important, since
it's only called on paths that come from regex matches.
If we make it a public later, having asserts would be really useful.
That said, we might address it later when/if we actually make this function
public.
================
Comment at: unittests/Format/CleanupTest.cpp:862
std::string Expected = "#include \"a.h\"\n"
- "#include \"vector\"\n"
- "#include <vector>\n"
- "#include <a.h>\n";
+ "#include <vector>\n";
tooling::Replacements Replaces =
----------------
Are we sure we want this behavior change?
The API seems to allow us keeping the old one.
I know there's a FIXME there, but I just wanted to understand better what's the
right thing to do here and why.
Repository:
rC Clang
https://reviews.llvm.org/D46180
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
