Thanks for the review.  I have updated the comments, and the test
cases for ARM.  The new patch file is attached.

Thanks.
Logan

On Fri, Oct 19, 2012 at 7:47 AM, Eli Friedman <[email protected]>wrote:

> On Thu, Oct 18, 2012 at 8:23 AM, Logan Chien <[email protected]>
> wrote:
> >
> >
> > ---------- Forwarded message ----------
> > From: Logan Chien <[email protected]>
> > Date: Thu, Oct 18, 2012 at 11:20 PM
> > Subject: Re: [cfe-commits] Bug 11709 Fix: va_list on ARM is not following
> > AAPCS 7.1.4
> > To: Eli Friedman <[email protected]>
> >
> >
> > Thanks for your review.
> >
> > I have added new test to test/SemaCXX/builtins-arm.cpp to
> > specify the error message when PerformCopyInitialization fails.
> >
> > BTW, is the InitializedEntity related code correct?  I hope I can
> > get a modifiable Lvalue reference after initialization (not a copy
> > of the value in the structure.)  I'm not very confident with this.
>
> The code is doing the right thing.
>
> +    // If va_list is a record type and we are compiling under C++ mode,
> +    // then we should check the argument by copy assignment operator
>
> This comment is wrong; even though you're using
> PerformCopyInitialization, there's no actual copy because the
> InitializedEntity has reference type.  "If va_list is a record type
> and we are compiling in C++ mode, check the argument using reference
> binding."
>
> +// RUN: %clang_cc1 -triple armv7 -fsyntax-only -verify %s
> +
> +#if defined(__ARM_PCS) || defined(__ARM_EABI__)
>
> Please don't use an ifdef here; just use an appropriate triple.
>
> Otherwise, looks fine.
>
> -Eli
>

Attachment: 0001-Fix-__builtin_va_arg-assertion-failure-in-ARM-AAPCS.patch
Description: Binary data

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to