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