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@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@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits