[clang-tools-extra] r178485 - Improve loop convert's variable aliasing

[Rafael Espíndola]

Not sure when this started, but it looks like this test fails
intermittently on some bots:

http://bb.pgr.jp/builders/cmake-clang-i686-mingw32

On 1 April 2013 14:15, Edwin Vane <edwin.vane at intel.com> wrote:
> Author: revane
> Date: Mon Apr  1 13:15:06 2013
> New Revision: 178485
>
> URL: http://llvm.org/viewvc/llvm-project?rev=178485&view=rev
> Log:
> Improve loop convert's variable aliasing
>
> Loop convert's variable name aliasing may cause issues if the variable is
> declared as a value (copy). The converted loop will declare the variable as a
> reference which may inadvertently cause modifications to the container if it
> were used and modified as a temporary copy.
>
> This is fixed by preserving the reference or value qualifiers of the aliased
> variable. That is, if the variable was declared as a value the loop variable
> will also be declared as a value and similarly for references.
>
> Fixes: PR15600
> Author: Jack Yang <jack.yang at intel.com>
>
>
> Modified:
>     clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp
>     clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp
>
> Modified: clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp?rev=178485&r1=178484&r2=178485&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp (original)
> +++ clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp Mon Apr  1 13:15:06 2013
> @@ -737,10 +737,14 @@ void LoopFixer::doConversion(ASTContext
>                               bool ContainerNeedsDereference,
>                               bool DerefByValue) {
>    std::string VarName;
> +  bool VarNameFromAlias = Usages.size() == 1 && AliasDecl;
> +  bool AliasVarIsRef = false;
>
> -  if (Usages.size() == 1 && AliasDecl) {
> +  if (VarNameFromAlias) {
>      const VarDecl *AliasVar = cast<VarDecl>(AliasDecl->getSingleDecl());
>      VarName = AliasVar->getName().str();
> +    AliasVarIsRef = AliasVar->getType()->isReferenceType();
> +
>      // We keep along the entire DeclStmt to keep the correct range here.
>      const SourceRange &ReplaceRange = AliasDecl->getSourceRange();
>      Replace->insert(
> @@ -770,13 +774,18 @@ void LoopFixer::doConversion(ASTContext
>
>    QualType AutoRefType = Context->getAutoDeductType();
>
> -  // If an iterator's operator*() returns a 'T&' we can bind that to 'auto&'.
> -  // If operator*() returns 'T' we can bind that to 'auto&&' which will deduce
> -  // to 'T&&'.
> -  if (DerefByValue)
> -    AutoRefType = Context->getRValueReferenceType(AutoRefType);
> -  else
> -    AutoRefType = Context->getLValueReferenceType(AutoRefType);
> +  // If the new variable name is from the aliased variable, then the reference
> +  // type for the new variable should only be used if the aliased variable was
> +  // declared as a reference.
> +  if (!VarNameFromAlias || AliasVarIsRef) {
> +    // If an iterator's operator*() returns a 'T&' we can bind that to 'auto&'.
> +    // If operator*() returns 'T' we can bind that to 'auto&&' which will deduce
> +    // to 'T&&'.
> +    if (DerefByValue)
> +      AutoRefType = Context->getRValueReferenceType(AutoRefType);
> +    else
> +      AutoRefType = Context->getLValueReferenceType(AutoRefType);
> +  }
>
>    std::string MaybeDereference = ContainerNeedsDereference ? "*" : "";
>    std::string TypeString = AutoRefType.getAsString();
>
> Modified: clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp?rev=178485&r1=178484&r2=178485&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp (original)
> +++ clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp Mon Apr  1 13:15:06 2013
> @@ -57,3 +57,40 @@ void aliasing() {
>    // CHECK-NEXT: Val &t = func(elem);
>    // CHECK-NEXT: int y = t.x;
>  }
> +
> +void refs_and_vals() {
> +  // The following tests check that the transform correctly preserves the
> +  // reference or value qualifiers of the aliased variable. That is, if the
> +  // variable was declared as a value, the loop variable will be declared as a
> +  // value and vice versa for references.
> +
> +  S s;
> +  const S s_const = s;
> +
> +  for (S::const_iterator it = s_const.begin(); it != s_const.end(); ++it) {
> +    MutableVal alias = *it; { }
> +    alias.x = 0;
> +  }
> +  // CHECK: for (auto alias : s_const)
> +  // CHECK-NOT: MutableVal {{[a-z_]+}} =
> +  // CHECK-NEXT: { }
> +  // CHECK-NEXT: alias.x = 0;
> +
> +  for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) {
> +    MutableVal alias = *it; { }
> +    alias.x = 0;
> +  }
> +  // CHECK: for (auto alias : s)
> +  // CHECK-NOT: MutableVal {{[a-z_]+}} =
> +  // CHECK-NEXT: { }
> +  // CHECK-NEXT: alias.x = 0;
> +
> +  for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) {
> +    MutableVal &alias = *it; { }
> +    alias.x = 0;
> +  }
> +  // CHECK: for (auto & alias : s)
> +  // CHECK-NOT: MutableVal &{{[a-z_]+}} =
> +  // CHECK-NEXT: { }
> +  // CHECK-NEXT: alias.x = 0;
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits