================
Comment at: lib/Format/Format.cpp:210
@@ +209,3 @@
+
+ bool safeToSplitLine(StringRef Line) {
+ return Line.find_first_of("/\\:<>()[]{}*&%") == StringRef::npos;
----------------
Daniel Jasper wrote:
> nit: I'd prefer isLineSafeToSplit
Done.
================
Comment at: lib/Format/Format.cpp:254
@@ +253,3 @@
+ if (Prefix.endswith("*")) {
+ NewLine += " ";
+ --ColumnLimit;
----------------
Daniel Jasper wrote:
> I presume, that the " " is not part of the prefix because findCommonPrefix
> also looks at the last line of a comment which can be "*/". Why can't
> findCommonPrefix not just ignore that line instead?
Because then we'd have to ignore other lines missing space after *, e.g.
"empty" lines:
/*
* This is a cool comment.
*
* This is another paragraph.
* ...
================
Comment at: unittests/Format/FormatTest.cpp:661
@@ +660,3 @@
+ format("/*\n"
+ "This is a long "
+ "comment that doesn't "
----------------
Daniel Jasper wrote:
> nit: Is there a specific reason to split here and in the next line? I think
> not, but it makes you wonder a little.
With this layout it was easier for me to see how this should be split with
regards to whitespace etc.
================
Comment at: lib/Format/Format.cpp:274
@@ +273,3 @@
+ SourceMgr,
+ TokenLoc.getLocWithOffset(Line.data() + BreakPos - Text.data()),
+ SpacePos - BreakPos + 1, NewLine));
----------------
Daniel Jasper wrote:
> I am a bit worried that this seems to rely on implementation details of
> StringRef (with data() always returning the exact pointer offset after calls
> to split, substr and ltrim).
There are no guarantees in the StringRef interface documentation, but a quick
look at StringRef::data() usages revealed a lot of places that rely not only on
the pointed data, but on the pointer value itself. Among these are object file
format implementations, serialization etc. I would assume, it's rather unlikely
that StringRef implementation will change and break all of these.
================
Comment at: lib/Format/Format.cpp:196
@@ -195,2 +195,3 @@
private:
- void indentBlockComment(const FormatToken &Tok, int Indent) {
+ StringRef findCommonPrefix(ArrayRef<StringRef> Lines) {
+ if (Lines.size() < 3)
----------------
Daniel Jasper wrote:
> This needs a comment describing the logic (or input->output examples).
Done.
================
Comment at: lib/Format/Format.cpp:201
@@ +200,3 @@
+ for (size_t i = 2; i < Lines.size(); ++i) {
+ for (size_t j = 0; j < Prefix.size() && j < Lines[i].size(); ++j)
+ if (Prefix[j] != Lines[i][j]) {
----------------
Daniel Jasper wrote:
> Nit: Add {}
Done.
================
Comment at: lib/Format/Format.cpp:246
@@ +245,3 @@
+ StringRef Prefix = findCommonPrefix(Lines);
+ size_t ColumnLimit = Style.ColumnLimit - Prefix.size() - IndentDelta;
+ std::string NewLine("\n");
----------------
Daniel Jasper wrote:
> Try to reuse more stuff from breakProtrudingToken(), which does something
> similar for String literals. Especially, reuse
> WhitespaceManager::breakToken(), which already does some of this logic and
> also works in multi-line defines.
It turned out to be not useful for this specific case.
http://llvm-reviews.chandlerc.com/D547
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
