<html>
<head>
<base href="https://llvm.org/bugs/" />
</head>
<body><table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Bug ID</th>
<td><a class="bz_bug_link
bz_status_NEW "
title="NEW --- - Spacing issue with "UseTab=Always" option"
href="https://llvm.org/bugs/show_bug.cgi?id=24401">24401</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>Spacing issue with "UseTab=Always" option
</td>
</tr>
<tr>
<th>Product</th>
<td>clang
</td>
</tr>
<tr>
<th>Version</th>
<td>trunk
</td>
</tr>
<tr>
<th>Hardware</th>
<td>PC
</td>
</tr>
<tr>
<th>OS</th>
<td>Linux
</td>
</tr>
<tr>
<th>Status</th>
<td>NEW
</td>
</tr>
<tr>
<th>Severity</th>
<td>normal
</td>
</tr>
<tr>
<th>Priority</th>
<td>P
</td>
</tr>
<tr>
<th>Component</th>
<td>Formatter
</td>
</tr>
<tr>
<th>Assignee</th>
<td>unassignedclangbugs@nondot.org
</td>
</tr>
<tr>
<th>Reporter</th>
<td>cbeck@princeton.edu
</td>
</tr>
<tr>
<th>CC</th>
<td>djasper@google.com, klimek@google.com, llvm-bugs@lists.llvm.org
</td>
</tr>
<tr>
<th>Classification</th>
<td>Unclassified
</td>
</tr></table>
<p>
<div>
<pre>Created <span class=""><a href="attachment.cgi?id=14713" name="attach_14713" title="Patch which adds a unit test demonstrating the bug, @git revision 9d19c31c0b500c54d9276d4d8d0748a0820abe68">attachment 14713</a> <a href="attachment.cgi?id=14713&action=edit" title="Patch which adds a unit test demonstrating the bug, @git revision 9d19c31c0b500c54d9276d4d8d0748a0820abe68">[details]</a></span>
Patch which adds a unit test demonstrating the bug, @git revision
9d19c31c0b500c54d9276d4d8d0748a0820abe68
When using "UseTab=Always" together with "AlignTrailingComments", I get
misaligned trailing comments. The issue is fixed by using
"UseTab=ForIndentation".
I took an example from my project and coded it up as a clang unit test, patch
attached.
To make the issue concrete, here's what it looks like with
"UseTab=ForIndentation":
if (it != bytecode_cache_.end()) {
lua_pushstring(L, bytecode_cache_key); // key
lua_rawget(L, LUA_REGISTRYINDEX); // cache_table
lua_rawgeti(L, -1, it->second); // cache_table bytecode
lua_remove(L, -2); // bytecode
lua_insert(L, -1 - nargs); // bytecode [args x nargs]
return protected_call(nargs); //
} else if (load_string(script)) { // bytecode
lua_pushstring(L, bytecode_cache_key); // bytecode key
lua_rawget(L, LUA_REGISTRYINDEX); // bytecode cache_table
lua_pushvalue(L, -2); // bytecode cache_table bytecode
bytecode_cache_[script] = luaL_ref(L, -2); // bytecode cache_table
lua_pop(L, 1); // bytecode
lua_insert(L, -1 - nargs); // bytecode [args x nargs]
return protected_call(nargs); //
} else {
return false;
}
Here's what it looks like with "UseTab=Always"
if (it != bytecode_cache_.end()) {
lua_pushstring(L, bytecode_cache_key); // key
lua_rawget(L, LUA_REGISTRYINDEX); // cache_table
lua_rawgeti(L, -1, it->second); // cache_table bytecode
lua_remove(L, -2); // bytecode
lua_insert(L, -1 - nargs); // bytecode [args x nargs]
return protected_call(nargs); //
} else if (load_string(script)) { // bytecode
lua_pushstring(L, bytecode_cache_key); // bytecode key
lua_rawget(L, LUA_REGISTRYINDEX); // bytecode cache_table
lua_pushvalue(L, -2); // bytecode cache_table bytecode
bytecode_cache_[script] = luaL_ref(L, -2); // bytecode cache_table
lua_pop(L, 1); // bytecode
lua_insert(L, -1 - nargs); // bytecode [args x nargs]
return protected_call(nargs); //
} else {
return false;
}
The patch is a clang unit test (valid against master branch git commit
9d19c31c0b500c54d9276d4d8d0748a0820abe68) which tests that this is indeed the
current (incorrect) behavior, using style with minimal changes from LLVM style
which generates the behavior.
I tried to figure out how to fix it, I think that potentially the issue might
be a logic error here?
<a href="https://github.com/llvm-mirror/clang/blob/45aa4ad6e537866b40fe9574fa992bf3d6593e39/lib/Format/WhitespaceManager.cpp#L422">https://github.com/llvm-mirror/clang/blob/45aa4ad6e537866b40fe9574fa992bf3d6593e39/lib/Format/WhitespaceManager.cpp#L422</a>
The code comment says "Indent with tabs only when there's at least one full
tab", and checks if the desired number of spaces is at least "FirstTabWidth"
(the width the first tab would have) + the width of a full tab. If so it emits
one tab character and subtracts that many spaces. Then we assume we are tab
aligned, and emit Spaces / TabWidth tabs, and Spaces % TabWidth spaces.
However, in the case that the tested condition fails, presumably we aren't
supposed to emit any tabs and should only emit spaces, but we will do so
anyways, at line 427.
I experimented by changing the code, to negate the condition and cause it to
simply emit "Spaces" many spaces and return when the condition has failed,
otherwise it follows the same code path as before. However, this breaks several
other unit tests. So I thought I should just submit what I've done as an issue
to the bugtracker instead.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>