[cfe-commits] r160319 - in /cfe/trunk: docs/UsersManual.html lib/Frontend/TextDiagnostic.cpp test/FixIt/fixit-unicode.c
Nico Weber
thakis at chromium.org
Thu Jul 19 23:45:36 PDT 2012
I reverted this change for now in r160542.
On Thu, Jul 19, 2012 at 11:08 PM, Nico Weber <thakis at chromium.org> wrote:
> Is it possible that this caused http://llvm.org/bugs/show_bug.cgi?id=13417 ?
>
> On Mon, Jul 16, 2012 at 1:52 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>> Author: jrose
>> Date: Mon Jul 16 15:52:12 2012
>> New Revision: 160319
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=160319&view=rev
>> Log:
>> Don't crash when emitting fixits following Unicode characters.
>>
>> This code is very sensitive to the difference between "columns" as printed
>> and "bytes" (SourceManager columns). All variables are now named explicitly
>> and our assumptions are (hopefully) documented as both comment and assertion.
>>
>> Whether parseable fixits should use byte offsets or Unicode character counts
>> is pending discussion on the mailing list; currently the implementation uses
>> bytes (and has no problems on lines containing multibyte characters).
>> This has been added to the user manual.
>>
>> <rdar://problem/11877454>
>>
>> Modified:
>> cfe/trunk/docs/UsersManual.html
>> cfe/trunk/lib/Frontend/TextDiagnostic.cpp
>> cfe/trunk/test/FixIt/fixit-unicode.c
>>
>> Modified: cfe/trunk/docs/UsersManual.html
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.html?rev=160319&r1=160318&r2=160319&view=diff
>> ==============================================================================
>> --- cfe/trunk/docs/UsersManual.html (original)
>> +++ cfe/trunk/docs/UsersManual.html Mon Jul 16 15:52:12 2012
>> @@ -229,6 +229,9 @@
>>
>> <p>When this is disabled, Clang will print "test.c:28: warning..." with no
>> column number.</p>
>> +
>> +<p>The printed column numbers count bytes from the beginning of the line; take
>> +care if your source contains multibyte characters.</p>
>> </dd>
>>
>> <!-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -->
>> @@ -395,6 +398,9 @@
>> </pre>
>>
>> <p>The {}'s are generated by -fdiagnostics-print-source-range-info.</p>
>> +
>> +<p>The printed column numbers count bytes from the beginning of the line; take
>> +care if your source contains multibyte characters.</p>
>> </dd>
>>
>> <!-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -->
>> @@ -415,6 +421,9 @@
>> "\\"), tabs (as "\t"), newlines (as "\n"), double
>> quotes(as "\"") and non-printable characters (as octal
>> "\xxx").</p>
>> +
>> +<p>The printed column numbers count bytes from the beginning of the line; take
>> +care if your source contains multibyte characters.</p>
>> </dd>
>>
>> <dt id="opt_fno-elide-type">
>>
>> Modified: cfe/trunk/lib/Frontend/TextDiagnostic.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/TextDiagnostic.cpp?rev=160319&r1=160318&r2=160319&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Frontend/TextDiagnostic.cpp (original)
>> +++ cfe/trunk/lib/Frontend/TextDiagnostic.cpp Mon Jul 16 15:52:12 2012
>> @@ -1124,7 +1124,7 @@
>> std::string FixItInsertionLine;
>> if (Hints.empty() || !DiagOpts.ShowFixits)
>> return FixItInsertionLine;
>> - unsigned PrevHintEnd = 0;
>> + unsigned PrevHintEndCol = 0;
>>
>> for (ArrayRef<FixItHint>::iterator I = Hints.begin(), E = Hints.end();
>> I != E; ++I) {
>> @@ -1136,11 +1136,15 @@
>> if (LineNo == SM.getLineNumber(HintLocInfo.first, HintLocInfo.second)) {
>> // Insert the new code into the line just below the code
>> // that the user wrote.
>> - unsigned HintColNo
>> + // Note: When modifying this function, be very careful about what is a
>> + // "column" (printed width, platform-dependent) and what is a
>> + // "byte offset" (SourceManager "column").
>> + unsigned HintByteOffset
>> = SM.getColumnNumber(HintLocInfo.first, HintLocInfo.second) - 1;
>> - // hint must start inside the source or right at the end
>> - assert(HintColNo<static_cast<unsigned>(map.bytes())+1);
>> - HintColNo = map.byteToColumn(HintColNo);
>> +
>> + // The hint must start inside the source or right at the end
>> + assert(HintByteOffset < static_cast<unsigned>(map.bytes())+1);
>> + unsigned HintCol = map.byteToColumn(HintByteOffset);
>>
>> // If we inserted a long previous hint, push this one forwards, and add
>> // an extra space to show that this is not part of the previous
>> @@ -1149,32 +1153,27 @@
>> //
>> // Note that if this hint is located immediately after the previous
>> // hint, no space will be added, since the location is more important.
>> - if (HintColNo < PrevHintEnd)
>> - HintColNo = PrevHintEnd + 1;
>> -
>> - // FIXME: if the fixit includes tabs or other characters that do not
>> - // take up a single column per byte when displayed then
>> - // I->CodeToInsert.size() is not a column number and we're mixing
>> - // units (columns + bytes). We should get printable versions
>> - // of each fixit before using them.
>> - unsigned LastColumnModified
>> - = HintColNo + I->CodeToInsert.size();
>> -
>> - if (LastColumnModified <= static_cast<unsigned>(map.bytes())) {
>> - // If we're right in the middle of a multibyte character skip to
>> - // the end of it.
>> - while (map.byteToColumn(LastColumnModified) == -1)
>> - ++LastColumnModified;
>> - LastColumnModified = map.byteToColumn(LastColumnModified);
>> - }
>> + if (HintCol < PrevHintEndCol)
>> + HintCol = PrevHintEndCol + 1;
>>
>> + // FIXME: This function handles multibyte characters in the source, but
>> + // not in the fixits. This assertion is intended to catch unintended
>> + // use of multibyte characters in fixits. If we decide to do this, we'll
>> + // have to track separate byte widths for the source and fixit lines.
>> + assert((size_t)llvm::sys::locale::columnWidth(I->CodeToInsert) ==
>> + I->CodeToInsert.size());
>> +
>> + // This relies on one byte per column in our fixit hints.
>> + // This should NOT use HintByteOffset, because the source might have
>> + // Unicode characters in earlier columns.
>> + unsigned LastColumnModified = HintCol + I->CodeToInsert.size();
>> if (LastColumnModified > FixItInsertionLine.size())
>> FixItInsertionLine.resize(LastColumnModified, ' ');
>> - assert(HintColNo+I->CodeToInsert.size() <= FixItInsertionLine.size());
>> +
>> std::copy(I->CodeToInsert.begin(), I->CodeToInsert.end(),
>> - FixItInsertionLine.begin() + HintColNo);
>> + FixItInsertionLine.begin() + HintCol);
>>
>> - PrevHintEnd = LastColumnModified;
>> + PrevHintEndCol = LastColumnModified;
>> } else {
>> FixItInsertionLine.clear();
>> break;
>>
>> Modified: cfe/trunk/test/FixIt/fixit-unicode.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit-unicode.c?rev=160319&r1=160318&r2=160319&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/FixIt/fixit-unicode.c (original)
>> +++ cfe/trunk/test/FixIt/fixit-unicode.c Mon Jul 16 15:52:12 2012
>> @@ -1,10 +1,11 @@
>> // RUN: %clang_cc1 -fsyntax-only %s 2>&1 | FileCheck -strict-whitespace %s
>> -// PR13312
>> +// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck -check-prefix=CHECK-MACHINE %s
>>
>> struct Foo {
>> int bar;
>> };
>>
>> +// PR13312
>> void test1() {
>> struct Foo foo;
>> (&foo)☃>bar = 42;
>> @@ -12,4 +13,19 @@
>> // Make sure we emit the fixit right in front of the snowman.
>> // CHECK: {{^ \^}}
>> // CHECK: {{^ ;}}
>> +
>> +// CHECK-MACHINE: fix-it:"{{.*}}fixit-unicode.c":{11:9-11:9}:";"
>> +}
>> +
>> +
>> +int printf(const char *, ...);
>> +void test2() {
>> + printf("∆: %d", 1L);
>> +// CHECK: warning: format specifies type 'int' but the argument has type 'long'
>> +// Don't crash emitting a fixit after the delta.
>> +// CHECK-NEXT: printf("∆: %d", 1L);
>> +// CHECK-NEXT: {{^ ~~ \^~}}
>> +// CHECK-NEXT: {{^ %ld}}
>> +
>> +// CHECK-MACHINE: fix-it:"{{.*}}fixit-unicode.c":{23:16-23:18}:"%ld"
>> }
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list