[cfe-commits] Requesting review of MS compatibility patch (fixes bug 11789)

[Richard Smith]

You need a triple for the wide_predefined_expr test; wchar_t is 16
bits on some platforms. Looks fine otherwise.

On Fri, Jun 22, 2012 at 6:17 PM, Nico Weber <thakis at chromium.org> wrote:
> Now with getArrayElementTypeNoTypeQual(), which I missed before.
>
> On Fri, Jun 22, 2012 at 6:07 PM, Nico Weber <thakis at chromium.org> wrote:
>> Thanks! All comments addressed.
>>
>> On Fri, Jun 22, 2012 at 5:52 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>> On Fri, Jun 22, 2012 at 5:41 PM, João Matos <ripzonetriton at gmail.com> wrote:
>>>> +  case PredefinedExpr::LFunction:       OS <<  " L__FUNCTION__"; break;
>>>>
>>>> Minor nitpick: On the diff this seems one space misaligned from the
>>>> other statements.
>>>>
>>>> Can't review the code gen part since I'm not too familiar with that
>>>> part of Clang yet. Apart from that it looks OK to me (with Aaron's
>>>> fixes).
>>>
>>> For the CodeGen part:
>>>
>>> +    const ConstantArrayType *CAT =
>>> +      getContext().getAsConstantArrayType(E->getType());
>>> +    QualType ElemType = CAT->getElementType();
>>> +    llvm::Constant *C;
>>> +    if (ElemType == getContext().WCharTy.withConst()) {
>>>
>>> CAT->getElementType()->isWideCharType(). You can also use
>>> Type::getArrayElementTypeNoTypeQual here, since you don't care about
>>> cv-qualifiers.
>>>
>>> +      GlobalVarName += ".WChar";
>>>
>>> This seems unnecessary, since GlobalVarName contains the L already.
>>>
>>> A test for the CodeGen part would be great.