[cfe-commits] r160319 - in /cfe/trunk: docs/UsersManual.html lib/Frontend/TextDiagnostic.cpp test/FixIt/fixit-unicode.c
Jordan Rose
jordan_rose at apple.com
Fri Jul 20 09:39:46 PDT 2012
Ouch, since the original bug is a crash as well. I'll take a look at this today (and PR13418 too).
On Jul 19, 2012, at 11:45 PM, Nico Weber wrote:
> 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