From 5a2fde0c2ca7457304e89c67262f3f2c8ccc0428 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Mon, 23 Dec 2013 15:08:47 -0800 Subject: [PATCH] Collapse duplicate constants and check for overflow. --- src/wren_compiler.c | 125 ++++++++++++++++------------- test/constant/many_duplicate.wren | 38 +++++++++ test/constant/many_unique.wren | 31 +++++++ test/constant/too_many_unique.wren | 31 +++++++ 4 files changed, 171 insertions(+), 54 deletions(-) create mode 100644 test/constant/many_duplicate.wren create mode 100644 test/constant/many_unique.wren create mode 100644 test/constant/too_many_unique.wren diff --git a/src/wren_compiler.c b/src/wren_compiler.c index bac8463f..de519bce 100644 --- a/src/wren_compiler.c +++ b/src/wren_compiler.c @@ -29,6 +29,11 @@ // that a function can close over. #define MAX_UPVALUES (256) +// The maximum number of distinct constants that a function can contain. This +// value is explicit in the bytecode since `CODE_CONSTANT` only takes a single +// argument. +#define MAX_CONSTANTS (256) + // The maximum name of a method, not including the signature. This is an // arbitrary but enforced maximum just so we know how long the method name // strings need to be in the parser. @@ -215,13 +220,75 @@ typedef struct sCompiler CompilerUpvalue upvalues[MAX_UPVALUES]; } Compiler; +// Outputs a compile or syntax error. This also marks the compilation as having +// an error, which ensures that the resulting code will be discarded and never +// run. This means that after calling lexError(), it's fine to generate whatever +// invalid bytecode you want since it won't be used. +static void lexError(Parser* parser, const char* format, ...) +{ + parser->hasError = true; + + fprintf(stderr, "[Line %d] Error: ", parser->currentLine); + + va_list args; + va_start(args, format); + vfprintf(stderr, format, args); + va_end(args); + + fprintf(stderr, "\n"); +} + +// Outputs a compile or syntax error. This also marks the compilation as having +// an error, which ensures that the resulting code will be discarded and never +// run. This means that after calling error(), it's fine to generate whatever +// invalid bytecode you want since it won't be used. +// +// You'll note that most places that call error() continue to parse and compile +// after that. That's so that we can try to find as many compilation errors in +// one pass as possible instead of just bailing at the first one. +static void error(Compiler* compiler, const char* format, ...) +{ + compiler->parser->hasError = true; + + Token* token = &compiler->parser->previous; + fprintf(stderr, "[Line %d] Error on ", token->line); + if (token->type == TOKEN_LINE) + { + // Don't print the newline itself since that looks wonky. + fprintf(stderr, "newline: "); + } + else + { + fprintf(stderr, "'%.*s': ", token->length, token->start); + } + + va_list args; + va_start(args, format); + vfprintf(stderr, format, args); + va_end(args); + + fprintf(stderr, "\n"); +} + // Adds [constant] to the constant pool and returns its index. static int addConstant(Compiler* compiler, Value constant) { - // TODO: Look for existing equal constant. Note that we need to *not* do that - // for the placeholder constant created for super calls. - // TODO: Check for overflow. - compiler->fn->constants[compiler->fn->numConstants++] = constant; + // See if an equivalent constant has already been added. + for (int i = 0; i < compiler->fn->numConstants; i++) + { + if (wrenValuesEqual(compiler->fn->constants[i], constant)) return i; + } + + if (compiler->fn->numConstants < MAX_CONSTANTS) + { + compiler->fn->constants[compiler->fn->numConstants++] = constant; + } + else + { + error(compiler, "A function may only contain %d unique constants.", + MAX_CONSTANTS); + } + return compiler->fn->numConstants - 1; } @@ -279,56 +346,6 @@ static int initCompiler(Compiler* compiler, Parser* parser, Compiler* parent, return addConstant(parent, OBJ_VAL(compiler->fn)); } -// Outputs a compile or syntax error. This also marks the compilation as having -// an error, which ensures that the resulting code will be discarded and never -// run. This means that after calling lexError(), it's fine to generate whatever -// invalid bytecode you want since it won't be used. -static void lexError(Parser* parser, const char* format, ...) -{ - parser->hasError = true; - - fprintf(stderr, "[Line %d] Error: ", parser->currentLine); - - va_list args; - va_start(args, format); - vfprintf(stderr, format, args); - va_end(args); - - fprintf(stderr, "\n"); -} - -// Outputs a compile or syntax error. This also marks the compilation as having -// an error, which ensures that the resulting code will be discarded and never -// run. This means that after calling error(), it's fine to generate whatever -// invalid bytecode you want since it won't be used. -// -// You'll note that most places that call error() continue to parse and compile -// after that. That's so that we can try to find as many compilation errors in -// one pass as possible instead of just bailing at the first one. -static void error(Compiler* compiler, const char* format, ...) -{ - compiler->parser->hasError = true; - - Token* token = &compiler->parser->previous; - fprintf(stderr, "[Line %d] Error on ", token->line); - if (token->type == TOKEN_LINE) - { - // Don't print the newline itself since that looks wonky. - fprintf(stderr, "newline: "); - } - else - { - fprintf(stderr, "'%.*s': ", token->length, token->start); - } - - va_list args; - va_start(args, format); - vfprintf(stderr, format, args); - va_end(args); - - fprintf(stderr, "\n"); -} - // Lexing ---------------------------------------------------------------------- // Returns true if [c] is a valid (non-initial) identifier character. diff --git a/test/constant/many_duplicate.wren b/test/constant/many_duplicate.wren new file mode 100644 index 00000000..cc48bd26 --- /dev/null +++ b/test/constant/many_duplicate.wren @@ -0,0 +1,38 @@ +var f = fn { + 1; 2; 3; 4; 5; 6; 7; 8; 9; 10 + 11; 12; 13; 14; 15; 16; 17; 18; 19; 20 + 21; 22; 23; 24; 25; 26; 27; 28; 29; 30 + 31; 32; 33; 34; 35; 36; 37; 38; 39; 40 + 41; 42; 43; 44; 45; 46; 47; 48; 49; 50 + 51; 52; 53; 54; 55; 56; 57; 58; 59; 60 + 61; 62; 63; 64; 65; 66; 67; 68; 69; 70 + 71; 72; 73; 74; 75; 76; 77; 78; 79; 80 + 81; 82; 83; 84; 85; 86; 87; 88; 89; 90 + 91; 92; 93; 94; 95; 96; 97; 98; 99; 100 + 101; 102; 103; 104; 105; 106; 107; 108; 109; 110 + 111; 112; 113; 114; 115; 116; 117; 118; 119; 120 + 121; 122; 123; 124; 125; 126; 127; 128; 129; 130 + 131; 132; 133; 134; 135; 136; 137; 138; 139; 140 + 141; 142; 143; 144; 145; 146; 147; 148; 149; 150 + 151; 152; 153; 154; 155; 156; 157; 158; 159; 160 + 161; 162; 163; 164; 165; 166; 167; 168; 169; 170 + 171; 172; 173; 174; 175; 176; 177; 178; 179; 180 + 181; 182; 183; 184; 185; 186; 187; 188; 189; 190 + 191; 192; 193; 194; 195; 196; 197; 198; 199; 200 + 201; 202; 203; 204; 205; 206; 207; 208; 209; 210 + 211; 212; 213; 214; 215; 216; 217; 218; 219; 220 + 221; 222; 223; 224; 225; 226; 227; 228; 229; 230 + 231; 232; 233; 234; 235; 236; 237; 238; 239; 240 + 241; 242; 243; 244; 245; 246; 247; 248; 249; 250 + 251; 252; 253; 254; 255 + + // Some more constants that are already in use: + 181; 182; 183; 184; 185; 186; 187; 188; 189; 190 + 191; 192; 193; 194; 195; 196; 197; 198; 199; 200 + 201; 202; 203; 204; 205; 206; 207; 208; 209; 210 + 211; 212; 213; 214; 215; 216; 217; 218; 219; 220 + + IO.write(256) +} + +f.call // expect: 256 \ No newline at end of file diff --git a/test/constant/many_unique.wren b/test/constant/many_unique.wren new file mode 100644 index 00000000..70bea39e --- /dev/null +++ b/test/constant/many_unique.wren @@ -0,0 +1,31 @@ +var f = fn { + 1; 2; 3; 4; 5; 6; 7; 8; 9; 10 + 11; 12; 13; 14; 15; 16; 17; 18; 19; 20 + 21; 22; 23; 24; 25; 26; 27; 28; 29; 30 + 31; 32; 33; 34; 35; 36; 37; 38; 39; 40 + 41; 42; 43; 44; 45; 46; 47; 48; 49; 50 + 51; 52; 53; 54; 55; 56; 57; 58; 59; 60 + 61; 62; 63; 64; 65; 66; 67; 68; 69; 70 + 71; 72; 73; 74; 75; 76; 77; 78; 79; 80 + 81; 82; 83; 84; 85; 86; 87; 88; 89; 90 + 91; 92; 93; 94; 95; 96; 97; 98; 99; 100 + 101; 102; 103; 104; 105; 106; 107; 108; 109; 110 + 111; 112; 113; 114; 115; 116; 117; 118; 119; 120 + 121; 122; 123; 124; 125; 126; 127; 128; 129; 130 + 131; 132; 133; 134; 135; 136; 137; 138; 139; 140 + 141; 142; 143; 144; 145; 146; 147; 148; 149; 150 + 151; 152; 153; 154; 155; 156; 157; 158; 159; 160 + 161; 162; 163; 164; 165; 166; 167; 168; 169; 170 + 171; 172; 173; 174; 175; 176; 177; 178; 179; 180 + 181; 182; 183; 184; 185; 186; 187; 188; 189; 190 + 191; 192; 193; 194; 195; 196; 197; 198; 199; 200 + 201; 202; 203; 204; 205; 206; 207; 208; 209; 210 + 211; 212; 213; 214; 215; 216; 217; 218; 219; 220 + 221; 222; 223; 224; 225; 226; 227; 228; 229; 230 + 231; 232; 233; 234; 235; 236; 237; 238; 239; 240 + 241; 242; 243; 244; 245; 246; 247; 248; 249; 250 + 251; 252; 253; 254; 255 + IO.write(256) +} + +f.call // expect: 256 \ No newline at end of file diff --git a/test/constant/too_many_unique.wren b/test/constant/too_many_unique.wren new file mode 100644 index 00000000..d9ec43bf --- /dev/null +++ b/test/constant/too_many_unique.wren @@ -0,0 +1,31 @@ +var f = fn { + 1; 2; 3; 4; 5; 6; 7; 8; 9; 10 + 11; 12; 13; 14; 15; 16; 17; 18; 19; 20 + 21; 22; 23; 24; 25; 26; 27; 28; 29; 30 + 31; 32; 33; 34; 35; 36; 37; 38; 39; 40 + 41; 42; 43; 44; 45; 46; 47; 48; 49; 50 + 51; 52; 53; 54; 55; 56; 57; 58; 59; 60 + 61; 62; 63; 64; 65; 66; 67; 68; 69; 70 + 71; 72; 73; 74; 75; 76; 77; 78; 79; 80 + 81; 82; 83; 84; 85; 86; 87; 88; 89; 90 + 91; 92; 93; 94; 95; 96; 97; 98; 99; 100 + 101; 102; 103; 104; 105; 106; 107; 108; 109; 110 + 111; 112; 113; 114; 115; 116; 117; 118; 119; 120 + 121; 122; 123; 124; 125; 126; 127; 128; 129; 130 + 131; 132; 133; 134; 135; 136; 137; 138; 139; 140 + 141; 142; 143; 144; 145; 146; 147; 148; 149; 150 + 151; 152; 153; 154; 155; 156; 157; 158; 159; 160 + 161; 162; 163; 164; 165; 166; 167; 168; 169; 170 + 171; 172; 173; 174; 175; 176; 177; 178; 179; 180 + 181; 182; 183; 184; 185; 186; 187; 188; 189; 190 + 191; 192; 193; 194; 195; 196; 197; 198; 199; 200 + 201; 202; 203; 204; 205; 206; 207; 208; 209; 210 + 211; 212; 213; 214; 215; 216; 217; 218; 219; 220 + 221; 222; 223; 224; 225; 226; 227; 228; 229; 230 + 231; 232; 233; 234; 235; 236; 237; 238; 239; 240 + 241; 242; 243; 244; 245; 246; 247; 248; 249; 250 + 251; 252; 253; 254; 255; 256 + IO.write(257) // expect error +} + +f.call