<div dir="ltr">On Sun, May 5, 2013 at 9:17 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><div class="h5">On Sun, May 5, 2013 at 8:45 AM, Abramo Bagnara <span dir="ltr"><<a href="mailto:abramo.bagnara@bugseng.com" target="_blank">abramo.bagnara@bugseng.com</a>></span> wrote:<br>
</div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div>Il 05/05/2013 17:05, Richard Smith ha scritto:<br>
</div><div>> On Sun, May 5, 2013 at 2:53 AM, Abramo Bagnara<br>
</div><div><div>> <<a href="mailto:abramo.bagnara@bugseng.com" target="_blank">abramo.bagnara@bugseng.com</a> <mailto:<a href="mailto:abramo.bagnara@bugseng.com" target="_blank">abramo.bagnara@bugseng.com</a>>> wrote:<br>
><br>
> Il 26/04/2013 16:36, Richard Smith ha scritto:<br>
> > Modified: cfe/trunk/lib/Sema/SemaInit.cpp<br>
> > URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=180603&r1=180602&r2=180603&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=180603&r1=180602&r2=180603&view=diff</a><br>
> ><br>
> ==============================================================================<br>
> > --- cfe/trunk/lib/Sema/SemaInit.cpp (original)<br>
> > +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Apr 26 09:36:30 2013<br>
> > @@ -97,6 +97,7 @@ static void CheckStringInit(Expr *Str, Q<br>
> > DeclT = S.Context.getConstantArrayType(IAT->getElementType(),<br>
> > ConstVal,<br>
> > ArrayType::Normal, 0);<br>
> > + Str->setType(DeclT);<br>
> > return;<br>
> > }<br>
><br>
> Is this change deliberate? It seems to introduce a regression:<br>
><br>
> $ cat z.c<br>
><br>
> void f() {<br>
> signed char s[] = "a";<br>
> unsigned char u[] = "a";<br>
> }<br>
> $ _clang -cc1 -ast-dump z.c<br>
> TranslationUnitDecl 0x67d96e0 <<invalid sloc>><br>
> |-TypedefDecl 0x67d9bc0 <<invalid sloc>> __int128_t '__int128'<br>
> |-TypedefDecl 0x67d9c20 <<invalid sloc>> __uint128_t 'unsigned __int128'<br>
> |-TypedefDecl 0x67d9f70 <<invalid sloc>> __builtin_va_list<br>
> '__va_list_tag [1]'<br>
> `-FunctionDecl 0x67da010 <z.c:2:1, line:5:1> f 'void ()'<br>
> `-CompoundStmt 0x67da350 <line:2:10, line:5:1><br>
> |-DeclStmt 0x67da208 <line:3:3, col:24><br>
> | `-VarDecl 0x67da100 <col:3, col:21> s 'signed char [2]'<br>
> | `-StringLiteral 0x67da198 <col:21> 'signed char [2]' lvalue "a"<br>
> `-DeclStmt 0x67da338 <line:4:3, col:26><br>
> `-VarDecl 0x67da270 <col:3, col:23> u 'unsigned char [2]'<br>
> `-StringLiteral 0x67da2c8 <col:23> 'unsigned char [2]'<br>
> lvalue "a"<br>
><br>
> Type of string literal should be plain char.<br>
><br>
><br>
> Yes, this is deliberate; we intended to set the string literal's type to<br>
> the type of the initialized variable (otherwise we would be initializing<br>
> an array of 'unsigned char' from an array of 'char'), but accidentally<br>
> only updated it in either the array bound or the type, but not both.<br>
><br>
> unsigned char a[] = "foo", b[4] = "bar";<br>
><br>
> ... used to produce ...<br>
><br>
> |-VarDecl 0x6222ad0 <<stdin>:1:1, col:21> a 'unsigned char [4]'<br>
> | `-StringLiteral 0x6222ba8 <col:21> 'const char [4]' lvalue "foo"<br>
> `-VarDecl 0x6222c60 <col:1, col:35> b 'unsigned char [4]'<br>
> `-StringLiteral 0x6222cb8 <col:35> 'unsigned char [4]' lvalue "bar"<br>
<br>
</div></div>BTW: uniformity is still not there:<br>
<div>$ cat z.c<br>
void f() {<br>
</div> unsigned char q[] = ("a");<br>
<div>}<br>
$ _clang -cc1 -ast-dump z.c<br>
</div>TranslationUnitDecl 0x6f296e0 <<invalid sloc>><br>
|-TypedefDecl 0x6f29bc0 <<invalid sloc>> __int128_t '__int128'<br>
|-TypedefDecl 0x6f29c20 <<invalid sloc>> __uint128_t 'unsigned __int128'<br>
|-TypedefDecl 0x6f29f70 <<invalid sloc>> __builtin_va_list<br>
'__va_list_tag [1]'<br>
`-FunctionDecl 0x6f2a010 <z.c:2:1, line:4:1> f 'void ()'<br>
`-CompoundStmt 0x6f2a240 <line:2:10, line:4:1><br>
`-DeclStmt 0x6f2a228 <line:3:3, col:28><br>
`-VarDecl 0x6f2a100 <col:3, col:27> q 'unsigned char [2]'<br>
`-ParenExpr 0x6f2a1c8 <col:23, col:27> 'unsigned char [2]' lvalue<br>
`-StringLiteral 0x6f2a198 <col:24> 'char [2]' lvalue "a"<br></blockquote><div><br></div></div></div><div>Yes, this is definitely wrong. A ParenExpr shouldn't change the type of its operand.</div>
</div></div></div></blockquote><div><br></div><div style>Fixed in r181159, thanks; this could be coaxed into producing a crash-on-valid.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
IMHO the type of StringLiteral and ParenExpr should be char[] and as the<br>
use of string literal for array initialization is definitely a special<br>
case, nothing is wrong if its type is the one mandated by the standard<br>
and different from initialized decl (i.e. "initialize this unsigned char<br>
array from this byte sequence that is plain char typed")</blockquote><div><br></div></div><div>There are several problems with that approach (or one problem viewed in several different ways):</div><div> - it introduces a change in type with no AST node performing the conversion</div>
<div> - the StringLiteral expression actually *is* creating an object of type unsigned char[2] in this case</div><div> - in C++, the standard-mandated type is *const* char[N], but such a StringLiteral can still be the value of a non-const array</div>
<div><br></div><div>Analogous to how we handle InitListExprs, we could keep separate syntactic and semantic forms for a StringLiteral used as an initializer (or maybe just separate syntactic and semantic types).</div>
</div></div></div>
</blockquote></div><br></div></div>