[clang] 1b8125b - Don't assert if we find a dependently-typed variable in the

[David Blaikie via cfe-commits]

Could/should this test be testing something more than "does not
crash"? I'd expect here's some specific behavior that would be
tested/desired rather than "anything other than crashing" - but I
understand if it's not practical to test, or perhap insufficiently
interesting with the new rephrased/fixed code (though if it's tested
at all, I'd have thought it'd be good to validate the behavior beyond
"does not crash").

On Tue, Jun 16, 2020 at 7:41 PM Richard Smith via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
>
> Author: Richard Smith
> Date: 2020-06-16T19:41:13-07:00
> New Revision: 1b8125b041e28a315e5c5fe64441a2fb07a2f5ea
>
> URL: https://github.com/llvm/llvm-project/commit/1b8125b041e28a315e5c5fe64441a2fb07a2f5ea
> DIFF: https://github.com/llvm/llvm-project/commit/1b8125b041e28a315e5c5fe64441a2fb07a2f5ea.diff
>
> LOG: Don't assert if we find a dependently-typed variable in the
> redeclaration chain for an array.
>
> A prior attempt to fix this in r280330 didn't handle the case where the
> old variable is dependent and the new one is not.
>
> It is notable and worrying that the test case in this example forms a
> redeclaration chain for a non-dependent variable that includes a
> declaration with a dependent type. We should probably fix that too.
>
> Added:
>
>
> Modified:
>     clang/lib/Sema/SemaDecl.cpp
>     clang/test/SemaTemplate/array-redeclaration.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
> index d5d946429e7d..2bf16d138d5a 100644
> --- a/clang/lib/Sema/SemaDecl.cpp
> +++ b/clang/lib/Sema/SemaDecl.cpp
> @@ -3910,11 +3910,11 @@ void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old,
>        if (!NewArray->isIncompleteArrayType() && !NewArray->isDependentType()) {
>          for (VarDecl *PrevVD = Old->getMostRecentDecl(); PrevVD;
>               PrevVD = PrevVD->getPreviousDecl()) {
> -          const ArrayType *PrevVDTy = Context.getAsArrayType(PrevVD->getType());
> +          QualType PrevVDTy = PrevVD->getType();
>            if (PrevVDTy->isIncompleteArrayType() || PrevVDTy->isDependentType())
>              continue;
>
> -          if (!Context.hasSameType(NewArray, PrevVDTy))
> +          if (!Context.hasSameType(New->getType(), PrevVDTy))
>              return diagnoseVarDeclTypeMismatch(*this, New, PrevVD);
>          }
>        }
>
> diff  --git a/clang/test/SemaTemplate/array-redeclaration.cpp b/clang/test/SemaTemplate/array-redeclaration.cpp
> index 4edee701cfc4..af0a2770291c 100644
> --- a/clang/test/SemaTemplate/array-redeclaration.cpp
> +++ b/clang/test/SemaTemplate/array-redeclaration.cpp
> @@ -31,3 +31,9 @@ void foo3() {
>  void foo4() {
>    foo3<5>();
>  }
> +
> +namespace NS {
> +  int f() { extern int arr[3]; { extern int arr[]; } return 0; }
> +  template<typename T> void g() { extern int arr[3]; extern T arr; }
> +  template void g<int[]>();
> +}
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits