[PATCH]: wchar_t foo[] = L"bar"

Clang Frontend
#1

hi

the $subj is currently rejected by clang because it does not know
this is allowed by the C standard.. I've come with a simple patch
that lets this happen. It introduces a new isWideCharType() method
to ASTContext class (as suggested by Daniel Dunbar) and uses it
to check if this contstruction is allowed...

the patch: www.vlakno.cz/~rdivacky/wchar.patch

please review/comment and possibly commit :slight_smile:

roman

p.s. the whole issue is tracked at http://llvm.org/bugs/show_bug.cgi?id=3097

#2

Some comments:

+ /// Detects whether a type is of WChar pointer type
+ bool isWideCharType(QualType Ty) const;

With this addition, we now have two isWideCharTypes: one in
ASTContext, and one in Type. The latter only works in C++, while the
one you've added currently works for C and C++. One of them will have
to be removed.

+bool ASTContext::isWideCharType(QualType Ty) const {
+ return Ty->getCanonicalTypeInternal().getUnqualifiedType() ==
getWCharType();
+}

This will always work in C++ (where wchar_t is a distinct type), but
the fact that it works now in C is due to an error in
ASTContext::getWCharType, which isn't doing the right thing for C:

QualType ASTContext::getWCharType() const {
  if (LangOpts.CPlusPlus)
    return WCharTy;

  // FIXME: In C, shouldn't WCharTy just be a typedef of the target's
  // wide-character type?
  return getFromTargetType(Target.getWCharType());
}

If we fix getWCharType, ASTContext::isWideCharType will no longer
work, since getWCharType would then return a typedef. I think we
should either fix both isWideCharType and getWCharType to do the same
thing, or find a different way around the problem. The second one is
easier for now, because Sema::IsStringLiteralInit is more complicated
than it needs to be:

StringLiteral *Sema::IsStringLiteralInit(Expr *Init, QualType DeclType) {
   const ArrayType *AT = Context.getAsArrayType(DeclType);
- if (AT && AT->getElementType()->isCharType()) {
+ if (AT && AT->getElementType()->isCharType() ||
+ Context.isWideCharType(AT->getElementType())) {
     return dyn_cast<StringLiteral>(Init);
   }

Why do we bother checking at the type of a string literal is an array
of characters? We know it's an array of characters, because it's a
string literal. So it seems that the checking of the array type could
be removed, and we could just dyn_cast<StringLiteral>(Init). That also
side-steps the issue of isWideCharType (for now).

Oh, and I know you have a test-case in the bug report, but could you
include the new tests as part of the patch? The tests should go into
test/Sema or test/SemaCXX for this kind of change.

Thanks!

  - Doug

#3

So then, the implementation has to change to "return
Ty->getCanonicalTypeInternal().getUnqualifiedType() ==
getWCharType().getCanonicalType();"? That doesn't seem like a serious
issue.

-Eli