From d3e6879223c5b3e9c84b52e783422f2e88e0ab0d Mon Sep 17 00:00:00 2001 From: Matt Pharr Date: Mon, 21 Nov 2011 09:16:29 -0800 Subject: [PATCH] Improve error checking for unsized arrays. Added support for resolving dimensions of multi-dimensional unsized arrays from their initializer exprerssions (previously, only the first dimension would be resolved.) Added checks to make sure that no unsized array dimensions remain after doing this (except for the first dimensision of array parameters to functions.) --- decl.cpp | 19 +++++++-- module.cpp | 12 ++++++ stmt.cpp | 61 +++++++++++++++++------------ tests/unsized-array.ispc | 11 ++++++ tests_errors/decl-4.ispc | 2 +- tests_errors/initexpr-2.ispc | 3 ++ tests_errors/initexpr-3.ispc | 6 +++ tests_errors/initexpr-4.ispc | 4 ++ tests_errors/initexpr-6.ispc | 4 ++ tests_errors/initexpr.ispc | 3 ++ tests_errors/unsized-funcparam.ispc | 4 ++ type.cpp | 38 ++++++++++++++++++ type.h | 2 + 13 files changed, 140 insertions(+), 29 deletions(-) create mode 100644 tests/unsized-array.ispc create mode 100644 tests_errors/initexpr-2.ispc create mode 100644 tests_errors/initexpr-3.ispc create mode 100644 tests_errors/initexpr-4.ispc create mode 100644 tests_errors/initexpr-6.ispc create mode 100644 tests_errors/initexpr.ispc create mode 100644 tests_errors/unsized-funcparam.ispc diff --git a/decl.cpp b/decl.cpp index 2404e1eb..1dad7571 100644 --- a/decl.cpp +++ b/decl.cpp @@ -272,11 +272,24 @@ Declarator::GetType(const Type *base, DeclSpecs *ds) const { } } - // Arrays are passed by reference, so convert array - // parameters to be references here. - if (dynamic_cast(sym->type) != NULL) + const ArrayType *at = dynamic_cast(sym->type); + if (at != NULL) { + // Arrays are passed by reference, so convert array + // parameters to be references here. sym->type = new ReferenceType(sym->type, sym->type->IsConstType()); + // Make sure there are no unsized arrays (other than the + // first dimension) in function parameter lists. + at = dynamic_cast(at->GetElementType()); + while (at != NULL) { + if (at->GetElementCount() == 0) + Error(sym->pos, "Arrays with unsized dimensions in " + "dimensions after the first one are illegal in " + "function parameter lists."); + at = dynamic_cast(at->GetElementType()); + } + } + args.push_back(sym->type); argNames.push_back(sym->name); argPos.push_back(sym->pos); diff --git a/module.cpp b/module.cpp index 7d713085..05810080 100644 --- a/module.cpp +++ b/module.cpp @@ -237,6 +237,18 @@ Module::AddGlobalVariable(Symbol *sym, Expr *initExpr, bool isConst) { return; } + sym->type = ArrayType::SizeUnsizedArrays(sym->type, initExpr); + if (sym->type == NULL) + return; + + const ArrayType *at = dynamic_cast(sym->type); + if (at != NULL && at->TotalElementCount() == 0) { + Error(sym->pos, "Illegal to declare a global variable with unsized " + "array dimensions that aren't set with an initializer " + "expression."); + return; + } + LLVM_TYPE_CONST llvm::Type *llvmType = sym->type->LLVMType(g->ctx); // See if we have an initializer expression for the global. If so, diff --git a/stmt.cpp b/stmt.cpp index 978e40c8..4c934982 100644 --- a/stmt.cpp +++ b/stmt.cpp @@ -255,6 +255,19 @@ lInitSymbol(llvm::Value *lvalue, const char *symName, const Type *symType, } +static bool +lHasUnsizedArrays(const Type *type) { + const ArrayType *at = dynamic_cast(type); + if (at == NULL) + return false; + + if (at->GetElementCount() == 0) + return true; + else + return lHasUnsizedArrays(at->GetElementType()); +} + + void DeclStmt::EmitCode(FunctionEmitContext *ctx) const { if (!ctx->GetCurrentBasicBlock()) @@ -263,8 +276,7 @@ DeclStmt::EmitCode(FunctionEmitContext *ctx) const { for (unsigned int i = 0; i < vars.size(); ++i) { Symbol *sym = vars[i].sym; assert(sym != NULL); - const Type *type = sym->type; - if (type == NULL) + if (sym->type == NULL) continue; Expr *initExpr = vars[i].init; @@ -280,32 +292,29 @@ DeclStmt::EmitCode(FunctionEmitContext *ctx) const { // If it's an array that was declared without a size but has an // initializer list, then use the number of elements in the // initializer list to finally set the array's size. - const ArrayType *at = dynamic_cast(type); - if (at && at->GetElementCount() == 0) { - ExprList *exprList = dynamic_cast(initExpr); - if (exprList) { - ArrayType *t = at->GetSizedArray(exprList->exprs.size()); - assert(t != NULL); - sym->type = type = t; - } - else { - Error(sym->pos, "Can't declare an unsized array as a local " - "variable without providing an initializer expression to " - "set its size."); - continue; - } + sym->type = ArrayType::SizeUnsizedArrays(sym->type, initExpr); + if (sym->type == NULL) + continue; + + if (lHasUnsizedArrays(sym->type)) { + Error(pos, "Illegal to declare an unsized array variable without " + "providing an initializer expression to set its size."); + continue; } // References must have initializer expressions as well. - if (dynamic_cast(type) && initExpr == NULL) { + if (dynamic_cast(sym->type) && initExpr == NULL) { Error(sym->pos, "Must provide initializer for reference-type variable \"%s\".", sym->name.c_str()); continue; } - LLVM_TYPE_CONST llvm::Type *llvmType = type->LLVMType(g->ctx); - assert(llvmType != NULL); + LLVM_TYPE_CONST llvm::Type *llvmType = sym->type->LLVMType(g->ctx); + if (llvmType == NULL) { + assert(m->errorCount > 0); + return; + } if (sym->storageClass == SC_STATIC) { // For static variables, we need a compile-time constant value @@ -313,20 +322,21 @@ DeclStmt::EmitCode(FunctionEmitContext *ctx) const { // zero value. llvm::Constant *cinit = NULL; if (initExpr != NULL) { - if (lPossiblyResolveFunctionOverloads(initExpr, type) == false) + if (lPossiblyResolveFunctionOverloads(initExpr, sym->type) == false) continue; // FIXME: we only need this for function pointers; it was // already done for atomic types and enums in // DeclStmt::TypeCheck()... if (dynamic_cast(initExpr) == NULL) { - initExpr = TypeConvertExpr(initExpr, type, "initializer"); + initExpr = TypeConvertExpr(initExpr, sym->type, + "initializer"); // FIXME: and this is only needed to re-establish // constant-ness so that GetConstant below works for // constant artithmetic expressions... initExpr = initExpr->Optimize(); } - cinit = initExpr->GetConstant(type); + cinit = initExpr->GetConstant(sym->type); if (cinit == NULL) Error(initExpr->pos, "Initializer for static variable " "\"%s\" must be a constant.", sym->name.c_str()); @@ -337,7 +347,8 @@ DeclStmt::EmitCode(FunctionEmitContext *ctx) const { // Allocate space for the static variable in global scope, so // that it persists across function calls sym->storagePtr = - new llvm::GlobalVariable(*m->module, llvmType, type->IsConstType(), + new llvm::GlobalVariable(*m->module, llvmType, + sym->type->IsConstType(), llvm::GlobalValue::InternalLinkage, cinit, llvm::Twine("static.") + llvm::Twine(sym->pos.first_line) + @@ -353,8 +364,8 @@ DeclStmt::EmitCode(FunctionEmitContext *ctx) const { ctx->EmitVariableDebugInfo(sym); // And then get it initialized... sym->parentFunction = ctx->GetFunction(); - lInitSymbol(sym->storagePtr, sym->name.c_str(), type, initExpr, - ctx, sym->pos); + lInitSymbol(sym->storagePtr, sym->name.c_str(), sym->type, + initExpr, ctx, sym->pos); } } } diff --git a/tests/unsized-array.ispc b/tests/unsized-array.ispc new file mode 100644 index 00000000..29c552dd --- /dev/null +++ b/tests/unsized-array.ispc @@ -0,0 +1,11 @@ + +export uniform int width() { return programCount; } + +export void f_f(uniform float RET[], uniform float aFOO[]) { + float a[][2][] = { { { 1 }, { 2 } }, { { 3 }, { 4 } }, { { 5 }, { 6 } } }; + RET[programIndex] = a[1][1][0]; +} + +export void result(uniform float RET[]) { + RET[programIndex] = 4; +} diff --git a/tests_errors/decl-4.ispc b/tests_errors/decl-4.ispc index fe758a61..be1e65b4 100644 --- a/tests_errors/decl-4.ispc +++ b/tests_errors/decl-4.ispc @@ -1,4 +1,4 @@ -// Can't declare an unsized array as a local variable without providing an initializer expression to set its size +// Illegal to declare an unsized array variable without providing an initializer expression to set its size int func() { int a[]; diff --git a/tests_errors/initexpr-2.ispc b/tests_errors/initexpr-2.ispc new file mode 100644 index 00000000..1ee1208f --- /dev/null +++ b/tests_errors/initexpr-2.ispc @@ -0,0 +1,3 @@ +// Initializer list for array "int32[2][4]" must have 2 elements (has 3). + +int a[2][4] = { { 1, 2, 3 }, { 1, 2, 3, 4 }, 1 }; diff --git a/tests_errors/initexpr-3.ispc b/tests_errors/initexpr-3.ispc new file mode 100644 index 00000000..2c2a0c27 --- /dev/null +++ b/tests_errors/initexpr-3.ispc @@ -0,0 +1,6 @@ +// Inconsistent expression list lengths found in initializer list + +void foo() { + int a[2][] = { { 1, 2, 3 }, { 1, 2, 3, 4 } }; +} + diff --git a/tests_errors/initexpr-4.ispc b/tests_errors/initexpr-4.ispc new file mode 100644 index 00000000..be281f7b --- /dev/null +++ b/tests_errors/initexpr-4.ispc @@ -0,0 +1,4 @@ +// Illegal to declare an unsized array variable without providing an initializer expression to set its size + +void foo() { int a[][]; } + diff --git a/tests_errors/initexpr-6.ispc b/tests_errors/initexpr-6.ispc new file mode 100644 index 00000000..f9245c4d --- /dev/null +++ b/tests_errors/initexpr-6.ispc @@ -0,0 +1,4 @@ +// Illegal to declare a global variable with unsized array dimensions that aren't set with an initializer expression + +int a[][]; + diff --git a/tests_errors/initexpr.ispc b/tests_errors/initexpr.ispc new file mode 100644 index 00000000..5c19b6b7 --- /dev/null +++ b/tests_errors/initexpr.ispc @@ -0,0 +1,3 @@ +// Initializer list for array "int32[4]" must have 4 elements + +int a[2][4] = { { 1, 2, 3 }, { 1, 2, 3, 4 } }; diff --git a/tests_errors/unsized-funcparam.ispc b/tests_errors/unsized-funcparam.ispc new file mode 100644 index 00000000..62f9a1c7 --- /dev/null +++ b/tests_errors/unsized-funcparam.ispc @@ -0,0 +1,4 @@ +// Arrays with unsized dimensions in dimensions after the first one are illegal in function parameter lists + +void func(int a[1][]) { +} diff --git a/type.cpp b/type.cpp index 591bac1e..41b1e78a 100644 --- a/type.cpp +++ b/type.cpp @@ -1103,6 +1103,44 @@ ArrayType::GetSizedArray(int sz) const { } +const Type * +ArrayType::SizeUnsizedArrays(const Type *type, Expr *initExpr) { + const ArrayType *at = dynamic_cast(type); + if (at == NULL) + return type; + + ExprList *exprList = dynamic_cast(initExpr); + if (exprList == NULL || exprList->exprs.size() == 0) + return type; + + if (at->GetElementCount() == 0) + type = at->GetSizedArray(exprList->exprs.size()); + + ExprList *nextList = dynamic_cast(exprList->exprs[0]); + if (nextList == NULL) + return type; + + unsigned int nextSize = nextList->exprs.size(); + for (unsigned int i = 1; i < exprList->exprs.size(); ++i) { + if (exprList->exprs[i] == NULL) { + assert(m->errorCount > 0); + continue; + } + + ExprList *el = dynamic_cast(exprList->exprs[i]); + if (el == NULL || el->exprs.size() != nextSize) { + Error(Union(exprList->exprs[0]->pos, exprList->exprs[i]->pos), + "Inconsistent expression list lengths found in initializer " + "list."); + return NULL; + } + } + + return new ArrayType(SizeUnsizedArrays(at->GetElementType(), nextList), + exprList->exprs.size()); +} + + /////////////////////////////////////////////////////////////////////////// // SOAArrayType diff --git a/type.h b/type.h index 7ff579ce..672ae87f 100644 --- a/type.h +++ b/type.h @@ -429,6 +429,8 @@ public: length. */ virtual ArrayType *GetSizedArray(int length) const; + static const Type *SizeUnsizedArrays(const Type *type, Expr *initExpr); + private: friend class SOAArrayType; /** Type of the elements of the array. */