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

[Nico Weber]

On Fri, Jun 22, 2012 at 6:25 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> You need a triple for the wide_predefined_expr test; wchar_t is 16
> bits on some platforms. Looks fine otherwise.

Done. r159060, thanks!

>
> 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.