Skip to content

Commit

Permalink
update var dependecy validation; fix overzealous validation
Browse files Browse the repository at this point in the history
  • Loading branch information
gewang committed Dec 3, 2023
1 parent bb768fa commit dbcec84
Show file tree
Hide file tree
Showing 12 changed files with 176 additions and 24 deletions.
3 changes: 2 additions & 1 deletion VERSIONS
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ new Foo(10,11,12) @=> Foo @ f4;
- (added) Shred.parent() and Shred.ancestor()
- (added) overloaded functions can have different return types
- (fixed) Type.of("int[][]").isArray() now correctly returns true
- (fixed) overzealous dependency validation for member variable usage from other classes
- (fixed) disambiguating overloaded functions to choose the "closest" match
by function call argument type inheritance "distance" to function argument type
- (added) compiler error for ambiguous function call where function call is ambiguous.
- (added) compiler error for ambiguous function calls when there is no "closest" match
- (updated) ChucK API doucmentation for constructors
- (added) overloaded constructors added (more to come)
==================
Expand Down
5 changes: 3 additions & 2 deletions src/core/chuck_emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4247,12 +4247,13 @@ t_CKBOOL emit_engine_emit_exp_func_call( Chuck_Emitter * emit,

// only check dependency violations if we are at a context-top-level
// or class-top-level scope, i.e., not in a function definition
// also, once sporked, it will be up the programmer to ensure intention
// also, once sporked, it will be up the programmer to ensure intention
if( !emit->env->func && !spork )
{
// dependency tracking: check if we invoke func before all its deps are initialized | 1.5.0.8 (ge) added
// NOTE if func originates from another file, this should behave correctly and return NULL | 1.5.1.1 (ge) fixed
const Chuck_Value_Dependency * unfulfilled = func->depends.locate( where, emit->env->class_def != NULL );
// NOTE passing in emit->env->class_def, to differentiate dependencies across class definitions | 1.5.2.0 (ge) fixed
const Chuck_Value_Dependency * unfulfilled = func->depends.locate( where, emit->env->class_def );
// at least one unfulfilled dependency
if( unfulfilled )
{
Expand Down
4 changes: 2 additions & 2 deletions src/core/chuck_scan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2521,8 +2521,8 @@ t_CKBOOL type_engine_scan2_exp_decl_create( Chuck_Env * env, a_Exp_Decl decl )
value->is_const = decl->is_const;

// dependency tracking: remember the code position of the DECL | 1.5.0.8
// do only if file-top-level or class-top-level, but not global
if( (value->is_member || value->is_context_global) && !value->is_global )
// NOTE track if context-global and not global, or class-member
if( (value->is_context_global && !value->is_global ) || value->is_member )
value->depend_init_where = var_decl->where;

// remember the value
Expand Down
66 changes: 51 additions & 15 deletions src/core/chuck_type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3420,12 +3420,25 @@ t_CKTYPE type_engine_check_exp_primary( Chuck_Env * env, a_Exp_Primary exp )
}

// make sure v is legit as this point
// but only check under certain conditions
// 1) file-level variable, invoked from anywhere
// 2) class-level member, invoked from pre-ctor only
if( !v->is_decl_checked )
{
EM_error2( exp->where,
"variable/member '%s' is used before declaration",
S_name(exp->var) );
return NULL;
if( v->is_context_global )
{
EM_error2( exp->where,
"variable '%s' is used before declaration",
S_name(exp->var) );
return NULL;
}
else if( v->is_member && !env->func )
{
EM_error2( exp->where,
"class member '%s' is used before declaration",
S_name(exp->var) );
return NULL;
}
}

// dependency tracking
Expand Down Expand Up @@ -9045,9 +9058,9 @@ string Chuck_Func::signature( t_CKBOOL incFuncDef, t_CKBOOL incRetType ) const
string className = value_ref->owner_class ? value_ref->owner_class->name() + "." : "";
// make signature so far
string signature = className + base_name + "(";
// the function keyword
if( incRetType ) signature = def()->ret_type->name() + " " + signature;
// the return type
if( incRetType ) signature = def()->ret_type->name() + " " + signature;
// the function keyword
if( incFuncDef ) signature = string("fun") + " " + signature;

// loop over arguments
Expand Down Expand Up @@ -9082,6 +9095,21 @@ string Chuck_Func::signature( t_CKBOOL incFuncDef, t_CKBOOL incRetType ) const



//-----------------------------------------------------------------------------
// name: ownerType()
// desc: get owner type: if func part of a class
//-----------------------------------------------------------------------------
Chuck_Type * Chuck_Func::ownerType() const
{
// check we have the necessary info
if( !value_ref ) return NULL;
// return value's owner type
return value_ref->owner_class;
}




//-----------------------------------------------------------------------------
// name: funcdef_connect()
// desc: connect a Func_Def (called when this func is type checked)
Expand Down Expand Up @@ -9324,7 +9352,7 @@ void Chuck_Value_Dependency_Graph::add( Chuck_Value_Dependency_Graph * graph )
// desc: locate dependency non-recursive
//-----------------------------------------------------------------------------
const Chuck_Value_Dependency * Chuck_Value_Dependency_Graph::locateLocal(
t_CKUINT pos, t_CKBOOL isMember )
t_CKUINT pos, Chuck_Type * fromClassDef )
{
// don't worry it if pos == 0 (assume omni-present, which is all good)
if( !pos ) return NULL;
Expand All @@ -9337,13 +9365,21 @@ const Chuck_Value_Dependency * Chuck_Value_Dependency_Graph::locateLocal(
{
// get value
v = directs[i].value;

// check
if( !v ) continue;

// look for any dependencies whose location is after pos
if( v->is_member == isMember && pos < v->depend_init_where )
if( pos < v->depend_init_where )
{
// return it
return &directs[i];
// usage NOT from within a class def; value in question NOT a class member OR
// usage from within a class def; value in question is a class member of the same class
if( (fromClassDef==NULL && v->is_member==FALSE) ||
(fromClassDef && v->is_member && equals(v->owner_class, fromClassDef)) )
{
// return dependency
return &directs[i];
}
}
}

Expand Down Expand Up @@ -9384,12 +9420,12 @@ void Chuck_Value_Dependency_Graph::resetRecursive( t_CKUINT value )
// desc: crawl the remote graph, taking care to handle cycle
//-----------------------------------------------------------------------------
const Chuck_Value_Dependency * Chuck_Value_Dependency_Graph::locateRecursive(
t_CKUINT pos, t_CKBOOL isMember, t_CKUINT searchToken )
t_CKUINT pos, Chuck_Type * fromClassDef, t_CKUINT searchToken )
{
// pointer to hold dep
const Chuck_Value_Dependency * dep = NULL;
// first search locally
dep = locateLocal( pos, isMember );
dep = locateLocal( pos, fromClassDef );
// if found, done
if( dep ) return dep;

Expand All @@ -9404,7 +9440,7 @@ const Chuck_Value_Dependency * Chuck_Value_Dependency_Graph::locateRecursive(
graph = remotes[i];
// if not already visited, visit
if( graph->token != searchToken )
dep = graph->locateRecursive( pos, isMember, searchToken );
dep = graph->locateRecursive( pos, fromClassDef, searchToken );
// if found, done
if( dep ) return dep;
}
Expand All @@ -9420,12 +9456,12 @@ const Chuck_Value_Dependency * Chuck_Value_Dependency_Graph::locateRecursive(
// desc: look for a dependency that occurs AFTER a particular code position
//-----------------------------------------------------------------------------
const Chuck_Value_Dependency * Chuck_Value_Dependency_Graph::locate(
t_CKUINT pos, t_CKBOOL isMember )
t_CKUINT pos, Chuck_Type * fromClassDef )
{
// reset search token
resetRecursive();
// recursive search
return locateRecursive( pos, isMember, 1 );
return locateRecursive( pos, fromClassDef, 1 );
}


Expand Down
8 changes: 5 additions & 3 deletions src/core/chuck_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ struct Chuck_Value_Dependency_Graph
void clear();
// look for a dependency that occurs AFTER a particular code position
// this function crawls the graph, taking care in the event of cycles
const Chuck_Value_Dependency * locate( t_CKUINT pos, t_CKBOOL isClassDef = FALSE );
const Chuck_Value_Dependency * locate( t_CKUINT pos, Chuck_Type * fromClassDef = NULL );

public:
// constructor
Expand All @@ -884,11 +884,11 @@ struct Chuck_Value_Dependency_Graph

protected:
// locate non-recursive
const Chuck_Value_Dependency * locateLocal( t_CKUINT pos, t_CKBOOL isClassDef );
const Chuck_Value_Dependency * locateLocal( t_CKUINT pos, Chuck_Type * fromClassDef );
// reset search tokens
void resetRecursive( t_CKUINT value = 0 );
// locate recursive
const Chuck_Value_Dependency * locateRecursive( t_CKUINT pos, t_CKBOOL isClassDef, t_CKUINT searchToken = 1 );
const Chuck_Value_Dependency * locateRecursive( t_CKUINT pos, Chuck_Type * fromClassDef, t_CKUINT searchToken = 1 );
};


Expand Down Expand Up @@ -1112,6 +1112,8 @@ struct Chuck_Func : public Chuck_VM_Object
std::string signature( t_CKBOOL incFunDef = TRUE, t_CKBOOL incRetType = TRUE ) const;
// code (included imported)
Chuck_VM_Code * code;
// get owner type: if func part of a class
Chuck_Type * ownerType() const;
// member (inside class)
t_CKBOOL is_member;
// static (inside class)
Expand Down
20 changes: 20 additions & 0 deletions src/test/01-Basic/244-depend-member-var.ck
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// test declaring class members in a file below access

class Foo
{
// member variable, declare above access in ctors
int m_a;

// a constructor
fun @construct( int n ) { n => m_a => m_b; }
// another constructor
fun @construct( int a, int b ) { a => m_a; b => m_b; }

// member variable, declare below access in ctrs
int m_b;
}

// instantiate with
Foo foo( 1, 2 );

if( foo.m_a + foo.m_b == 3 ) <<< "success" >>>;
37 changes: 37 additions & 0 deletions src/test/01-Basic/245-depend-class-extend.ck
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// test declaring/instantation classes with inheritance, involving
// a member variable, declared in the file below its access
// was issue #376

public class Foo
{
// this implicitly uses Bar.name, but okay since .name gets
// initialized in pre-ctor before all this runs
BarFactory.make() @=> Bar bar;
// test new
new BarChild @=> BarChild @ bc;
// test decl
BarChild bc2;
}

class Bar
{
"i am bar" => string name;
}

class BarChild extends Bar
{
"success" => name;
}

class BarFactory
{
fun static BarChild make()
{
return new BarChild;
}
}

// make a Foo
Foo foo;
// print
<<< foo.bar.name >>>;
27 changes: 27 additions & 0 deletions src/test/06-Errors/error-depend-class-extend.ck
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// test declaring/instantation classes with inheritance, involving
// a member variable, declared in the file below its access

class Bar
{
// this implicitly uses Bar.name...
// also would cause a infinite loop, but won't get that far
BarFactory.make();
// the var
"i am bar" => string name;
}

class BarChild extends Bar
{
"success" => name;
}

class BarFactory
{
fun static BarChild make()
{
return new BarChild;
}
}

// shouldn't quite get to here
BarFactory.make();
10 changes: 10 additions & 0 deletions src/test/06-Errors/error-depend-class-extend.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error-depend-class-extend.ck:8:16: error: calling 'make()' at this point skips initialization of a needed variable:
[8] BarFactory.make();
^
error-depend-class-extend.ck:10:26: error: ...(note: this skipped variable initialization is needed by 'fun BarChild BarFactory.make()')
[10] "i am bar" => string name;
^
error-depend-class-extend.ck:15:18: error: ...(note: this is where the variable is used within 'fun BarChild BarFactory.make()' or its subsidiaries)
[15] "success" => name;
^
error-depend-class-extend.ck: ...(hint: try calling 'make()' after the variable initialization)
15 changes: 15 additions & 0 deletions src/test/06-Errors/error-depend-class-var.ck
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// within class definition of Foo, class member 'myVal' is used before declaration

// class definition
class Foo
{
// no work -- depends on myVal being initialized
<<< myVal >>>;

// the dependency
5 => int myVal;
}

// should have error'ed out by this point
Foo foo;

3 changes: 3 additions & 0 deletions src/test/06-Errors/error-depend-class-var.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
error-depend-class-var.ck:7:9: error: class member 'myVal' is used before declaration
[7] <<< myVal >>>;
^
2 changes: 1 addition & 1 deletion src/test/06-Errors/error-depend-var2.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
error-depend-var2.ck:4:5: error: variable/member 'a' is used before declaration
error-depend-var2.ck:4:5: error: variable 'a' is used before declaration
[4] <<< a >>>;
^

0 comments on commit dbcec84

Please sign in to comment.