[cfe-commits] r146766 - /cfe/trunk/lib/Sema/SemaInit.cpp

[Richard Smith]

Hi Nicola,

I reverted this patch in r146961. It caused a miscompile (see the testcase I
added in that revision). Checking the LastCheckedSubobjectIndex is
insufficient -- the initializer could be a subobject of a different
initializer list, in which case the patch replaces the initializer expression
with an unrelated one.

- Richard

On Fri, December 16, 2011 22:57, Nicola Gigante wrote:
> Author: gigabytes
> Date: Fri Dec 16 16:57:37 2011
> New Revision: 146766
>
>
> URL: http://llvm.org/viewvc/llvm-project?rev=146766&view=rev
> Log:
> Fix an inconsistency in the syntactic form of InitListExpr in case of
> initialization that involves a ConstructorConversion
>
> Modified:
> cfe/trunk/lib/Sema/SemaInit.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=14676
> 6&r1=146765&r2=146766&view=diff
> ==============================================================================
>  --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Dec 16 16:57:37 2011
> @@ -171,6 +171,9 @@
> bool hadError; bool VerifyOnly; // no diagnostics, no structure building bool
> AllowBraceElision;
> +  Expr *LastCheckedSubobject;
> +  unsigned LastCheckedSubobjectIndex;
> +
> std::map<InitListExpr *, InitListExpr *> SyntacticToSemantic;
> InitListExpr *FullyStructuredList;
>
>
> @@ -470,7 +473,8 @@
> InitListChecker::InitListChecker(Sema &S, const InitializedEntity &Entity,
> InitListExpr *IL, QualType &T,
> bool VerifyOnly, bool AllowBraceElision) -  : SemaRef(S),
> VerifyOnly(VerifyOnly), AllowBraceElision(AllowBraceElision) {
> +  : SemaRef(S), VerifyOnly(VerifyOnly), AllowBraceElision(AllowBraceElision),
>  +    LastCheckedSubobject(0), LastCheckedSubobjectIndex(0) {
> hadError = false;
>
> unsigned newIndex = 0; @@ -792,13 +796,40 @@
>
>
> if (Seq) { if (!VerifyOnly) { -        ExprResult Result =
> -          Seq.Perform(SemaRef, Entity, Kind, MultiExprArg(&expr, 1));
> -        if (Result.isInvalid())
> -          hadError = true;
> +        // struct S {
> +        //   S(int);
> +        // };
> +        //
> +        // S s[] = { [0 ... 2] = 3 };
> +        //
> +        // In code like this, we want to perform the initialization and then
> +        // update the syntactic list with the result. However, we reach this
> +        // point once for each subobject, but the update needs
> +        // to be done only once for each syntactic element. For this reason,
> +        // the initialization result and its syntactic Index are cached in
> +        // LastCheckedSubobject and LastCheckedSubobjectIndex and reused
> until +        // we move to the next Index.
> +        Expr *ResultExpr = LastCheckedSubobject;
> +
> +        if (!ResultExpr || Index != LastCheckedSubobjectIndex) {
> +          ExprResult Result = Seq.Perform(SemaRef, Entity, Kind,
> MultiExprArg(&expr, 1));
> +
> +          if (Result.isInvalid()) {
> +            hadError = true;
> +            ResultExpr = 0;
> +          } else {
> +            ResultExpr = Result.takeAs<Expr>();
> +          }
> +
> +          LastCheckedSubobject = ResultExpr;
> +          LastCheckedSubobjectIndex = Index;
> +        }
> +
> +        // Update the syntactic list
> +        IList->setInit(Index, ResultExpr);
>
>
> UpdateStructuredListElement(StructuredList, StructuredIndex,
> -                                    Result.takeAs<Expr>());
> +                                    ResultExpr);
> }
> ++Index;
> return;
>
>
>
>