Skip to content

Commit

Permalink
refactor static compile error detect; update static depend logic
Browse files Browse the repository at this point in the history
  • Loading branch information
@gewang
gewang committed Dec 12, 2024
1 parent a4c4716 commit 0e83241
Show file tree
Hide file tree
Showing 23 changed files with 221 additions and 66 deletions.
2 changes: 1 addition & 1 deletion src/core/chuck_dl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1935,7 +1935,7 @@ static t_CKBOOL CK_DLL_CALL ck_get_mvar(Chuck_DL_Api::Object o, const char * nam
// see if name is correct type
if (value->type->xid != basic_type) continue;
// see if var is a member var
if (!value->is_member) continue;
if (!value->is_instance_member) continue;

// ladies and gentlemen, we've got it
var = value;
Expand Down
40 changes: 14 additions & 26 deletions src/core/chuck_emit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ t_CKBOOL emit_engine_emit_stmt( Chuck_Emitter * emit, a_Stmt stmt, t_CKBOOL pop

// get associated class
Chuck_Type * type = emit->env->class_def;
// push the current code sequender
// push the current code sequencer
emit->code_stack.push_back( emit->code );
// set the static code sequencer as the current
emit->code = type->static_code_emit;
Expand Down Expand Up @@ -4443,6 +4443,12 @@ t_CKBOOL emit_engine_emit_exp_func_call( Chuck_Emitter * emit,
// should help prevent confusing crashes; removing check for spork
if( !emit->env->func /* && !spork */ )
{
// check if we should check for dependency | 1.5.4.4 (ge) added #2024-static-init
// can skip check if 1) we are in a class def 2) not in a stmt marked for static init 3) the function we are calling is static
// actually, this doesn't work for cases where static functions actually depend (implicitly) on instance members / pre-ctor
// for example through the use of new; see test/06-Errors/error-depend-class-extend.ck
// t_CKBOOL skip = ( emit->env->class_def && !emit->env->in_static_stmt() && func->is_static );

// 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
// NOTE passing in emit->env->class_def, to differentiate dependencies across class definitions | 1.5.2.0 (ge) fixed
Expand Down Expand Up @@ -4790,16 +4796,16 @@ t_CKBOOL emit_engine_emit_exp_dot_member( Chuck_Emitter * emit,

// get the value to test | 1.5.4.3 (ge) added as part of #2024-static-init
// NOTE: this type of check usually resides pre-emisssion, in the type-checker
// but in this case, the hasStaticDecl check happens in the type-checker
// and this verification needs to be come after that
// but in this case, dot_member() emission also implicitly covers symbol X
// if X is a member of a class (type-checker does not do this implicit conversion at the moment)
Chuck_Value * v = type_engine_find_value( t_base, member->xid );
if( v && (!base_type_static && !base_exp_static)
&& emit->env->class_def && !emit->env->func
&& (emit->env->stmt_stack.size() && emit->env->stmt_stack.back()->hasStaticDecl) )
&& emit->env->in_static_stmt() )
{
if( v->func_ref )
{
if( v->is_member )
if( v->is_instance_member )
{
EM_error2( member->where,
"cannot call non-static function '%s' to initialize a static variable", v->func_ref->signature(FALSE,FALSE).c_str() );
Expand Down Expand Up @@ -4871,7 +4877,7 @@ t_CKBOOL emit_engine_emit_exp_dot_member( Chuck_Emitter * emit,
offset = value->offset;

// is the value static?
if( value->is_member )
if( value->is_instance_member )
{
// emit the base (TODO: return on error?)
emit_engine_emit_exp( emit, member->base );
Expand Down Expand Up @@ -5406,7 +5412,7 @@ t_CKBOOL emit_engine_emit_exp_decl( Chuck_Emitter * emit, a_Exp_Decl decl,
// put in the value

// member
if( value->is_member )
if( value->is_instance_member )
{
// zero out location in object, and leave addr on operand stack
// added 1.3.1.0: iskindofint -- on some 64-bit systems, sz_int == sz_FLOAT
Expand Down Expand Up @@ -6242,7 +6248,7 @@ t_CKBOOL emit_engine_emit_symbol( Chuck_Emitter * emit, S_Symbol symbol,
}

// if part of class - this only works because x.y is handled separately
if( v->owner_class && (v->is_member || v->is_static) )
if( v->owner_class && (v->is_instance_member || v->is_static) )
{
// make sure talking about the same class
// this doesn't work since the owner class could be a super class
Expand Down Expand Up @@ -6292,24 +6298,6 @@ t_CKBOOL emit_engine_emit_symbol( Chuck_Emitter * emit, S_Symbol symbol,
return TRUE;
}

// 1.5.4.3 (ge) added this check #2024-static-init
// static variable declarations cannot access values that
if( v && v->is_context_global && !v->is_global
&& emit->env->class_def && !emit->env->func && (emit->env->stmt_stack.size() && emit->env->stmt_stack.back()->hasStaticDecl) )
{
if( v->func_ref )
{
EM_error2( exp->where,
"cannot call local function '%s' to initialize a static variable", v->func_ref->signature(FALSE,FALSE).c_str() );
}
else
{
EM_error2( exp->where,
"cannot access local variable '%s' to initialize a static variable", S_name( exp->var ) );
}
return FALSE;
}

// var or value
if( emit_var )
{
Expand Down
35 changes: 29 additions & 6 deletions src/core/chuck_scan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ t_CKBOOL type_engine_scan0_class_def( Chuck_Env * env, a_Class_Def class_def )
value = env->context->new_Chuck_Value( type, the_class->base_name );
value->owner = env->curr; CK_SAFE_ADD_REF(value->owner);
value->is_const = TRUE;
value->is_member = FALSE;
value->is_instance_member = FALSE;
// add to env
env->curr->add_value( the_class->base_name, value );

Expand Down Expand Up @@ -527,6 +527,9 @@ t_CKBOOL type_engine_scan1_stmt( Chuck_Env * env, a_Stmt stmt )
if( !stmt )
return TRUE;

// push stmt | 1.5.4.4 (ge) added to scanner
env->stmt_stack.push_back( stmt );

// the type of stmt
switch( stmt->s_type )
{
Expand Down Expand Up @@ -657,6 +660,9 @@ t_CKBOOL type_engine_scan1_stmt( Chuck_Env * env, a_Stmt stmt )
// set whether all control paths associated with this stmt return | 1.5.1.0
stmt->allControlPathsReturn = allControlPathsReturn;

// pop stmt | 1.5.4.4 (ge) added to scanner
env->stmt_stack.pop_back();

return ret;
}

Expand Down Expand Up @@ -1785,6 +1791,9 @@ t_CKBOOL type_engine_scan2_stmt( Chuck_Env * env, a_Stmt stmt )
if( !stmt )
return TRUE;

// push stmt | 1.5.4.4 (ge) added to scanner
env->stmt_stack.push_back( stmt );

// the type of stmt
switch( stmt->s_type )
{
Expand Down Expand Up @@ -1890,6 +1899,9 @@ t_CKBOOL type_engine_scan2_stmt( Chuck_Env * env, a_Stmt stmt )
break;
}

// pop stmt | 1.5.4.4 (ge) added to scanner
env->stmt_stack.pop_back();

return ret;
}

Expand Down Expand Up @@ -2684,9 +2696,12 @@ t_CKBOOL type_engine_scan2_exp_decl_create( Chuck_Env * env, a_Exp_Decl decl )
// remember the owner
value->owner = env->curr; CK_SAFE_ADD_REF( value->owner );
value->owner_class = env->func ? NULL : env->class_def;
value->is_member = ( env->class_def != NULL &&
// set if is instanced member
value->is_instance_member = ( env->class_def != NULL &&
env->class_scope == 0 &&
env->func == NULL && !decl->is_static );
// 1.5.4.4 (ge) moved is_static set here from type_engine_check_exp_decl_part2()
value->is_static = (decl->is_static != 0);
value->is_context_global = ( env->class_def == NULL && env->func == NULL );
value->addr = var_decl->addr;
// flag it until the decl is checked
Expand All @@ -2696,9 +2711,17 @@ t_CKBOOL type_engine_scan2_exp_decl_create( Chuck_Env * env, a_Exp_Decl decl )
// flag as const | 1.5.1.3 (ge) added
value->is_const = decl->is_const;

// if we are located inside a statement
if( decl->is_static && env->stmt_stack.size() )
{
// mark containing statement as having static decl | 1.5.4.3 (ge) added as part of #2024-static-init
// 1.5.4.4 (ge) moved this logic from type_engine_check_exp_decl_part2()
env->stmt_stack.back()->hasStaticDecl = TRUE;
}

// dependency tracking: remember the code position of the DECL | 1.5.0.8
// NOTE track if context-global and not global, or class-member
if( (value->is_context_global && !value->is_global ) || value->is_member )
// NOTE track if context-global and not global, or class-member, or static (1.5.4.4 added static check)
if( (value->is_context_global && !value->is_global ) || value->is_instance_member || value->is_static )
value->depend_init_where = var_decl->where;

// remember the value
Expand Down Expand Up @@ -3120,7 +3143,7 @@ t_CKBOOL type_engine_scan2_func_def( Chuck_Env * env, a_Func_Def f )
// remember the owner
value->owner = env->curr; CK_SAFE_ADD_REF( value->owner );
value->owner_class = env->class_def; CK_SAFE_ADD_REF( value->owner_class );
value->is_member = func->is_member;
value->is_instance_member = func->is_member;
// is global context
value->is_context_global = env->class_def == NULL;
// remember the func
Expand Down Expand Up @@ -3251,7 +3274,7 @@ t_CKBOOL type_engine_scan2_func_def( Chuck_Env * env, a_Func_Def f )
v->owner = env->curr; CK_SAFE_ADD_REF( v->owner );
// function args not owned
v->owner_class = NULL; CK_SAFE_ADD_REF( v->owner_class );
v->is_member = FALSE;
v->is_instance_member = FALSE;
v->is_const = FALSE; // 1.5.1.3 (ge) added explicitly for future reference
// later: add as value
// symbols.push_back( arg_list );
Expand Down
70 changes: 47 additions & 23 deletions src/core/chuck_type.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3664,7 +3664,7 @@ t_CKTYPE type_engine_check_exp_primary( Chuck_Env * env, a_Exp_Primary exp )
if( env->func )
{
// if func static, v not
if( env->func->is_static && v->is_member && !v->is_static )
if( env->func->is_static && v->is_instance_member && !v->is_static )
{
EM_error2( exp->where,
"non-static member '%s' used from static function", S_name( exp->var ) );
Expand Down Expand Up @@ -3715,7 +3715,7 @@ t_CKTYPE type_engine_check_exp_primary( Chuck_Env * env, a_Exp_Primary exp )
S_name(exp->var) );
return NULL;
}
else if( v->is_member )
else if( v->is_instance_member )
{
EM_error2( exp->where,
"class member '%s' is used before declaration",
Expand All @@ -3724,6 +3724,24 @@ t_CKTYPE type_engine_check_exp_primary( Chuck_Env * env, a_Exp_Primary exp )
}
}

// 1.5.4.3 (ge) added this check #2024-static-init
// static variable declarations cannot access values that
if( v && v->is_context_global && !v->is_global
&& env->class_def && !env->func && env->in_static_stmt() )
{
if( v->func_ref )
{
EM_error2( exp->where,
"cannot call local function '%s' to initialize a static variable", v->func_ref->signature(FALSE,FALSE).c_str() );
}
else
{
EM_error2( exp->where,
"cannot access local variable '%s' to initialize a static variable", S_name( exp->var ) );
}
return FALSE;
}

// dependency tracking
if( v->depend_init_where > 0 )
{
Expand Down Expand Up @@ -4459,7 +4477,7 @@ t_CKTYPE type_engine_check_exp_decl_part2( Chuck_Env * env, a_Exp_Decl decl )
}

// member?
if( value->is_member )
if( value->is_instance_member )
{
// offset
value->offset = env->curr->offset;
Expand All @@ -4480,7 +4498,7 @@ t_CKTYPE type_engine_check_exp_decl_part2( Chuck_Env * env, a_Exp_Decl decl )
env->curr->offset = type_engine_next_offset( env->curr->offset, type );
// env->curr->offset += type->size;
}
else if( decl->is_static ) // static
else if( value->is_static ) // static
{
// base scope
if( env->class_def == NULL || env->class_scope > 0 )
Expand All @@ -4490,20 +4508,11 @@ t_CKTYPE type_engine_check_exp_decl_part2( Chuck_Env * env, a_Exp_Decl decl )
return NULL;
}

// flag
value->is_static = TRUE;
// offset
value->offset = env->class_def->nspc->static_data_size;
// move the size
env->class_def->nspc->static_data_size += type->size;

// if we are located inside a statement
if( env->stmt_stack.size() )
{
// mark containing statement as having static decl | 1.5.4.3 (ge) added as part of #2024-static-init
env->stmt_stack.back()->hasStaticDecl = TRUE;
}

// // if this is an object
// if( is_obj && !is_ref )
// {
Expand Down Expand Up @@ -4983,6 +4992,10 @@ t_CKTYPE type_engine_check_exp_func_call( Chuck_Env * env, a_Exp exp_func, a_Exp
}
else if( env->class_def ) // in a class definition
{
// check if a non-static stmt content is calling a static function | 1.5.4.4 (ge) added
// if so, no dependency since static is handled out-of-band
// t_CKBOOL skip = env->stmt_stack.size() && !env->stmt_stack.back()->hasStaticDecl && ck_func->is_static;

// dependency tracking: add the callee's dependencies
env->class_def->depends.add( &ck_func->depends );
}
Expand Down Expand Up @@ -5194,7 +5207,8 @@ t_CKTYPE type_engine_check_exp_dot_member( Chuck_Env * env, a_Exp_Dot_Member mem
{
Chuck_Value * value = NULL;
Chuck_Type * the_base = NULL;
t_CKBOOL base_static = FALSE;
t_CKBOOL base_type_static = FALSE;
t_CKBOOL base_exp_static = FALSE;
string str;

// type check the base
Expand All @@ -5215,10 +5229,10 @@ t_CKTYPE type_engine_check_exp_dot_member( Chuck_Env * env, a_Exp_Dot_Member mem
}

// is the base a class/namespace or a variable
base_static = type_engine_is_base_type_static( env, member->t_base );
// base_static = isa( member->t_base, env->ckt_class )
base_type_static = type_engine_is_base_type_static( env, member->t_base );

// actual type
the_base = base_static ? member->t_base->actual_type : member->t_base;
the_base = base_type_static ? member->t_base->actual_type : member->t_base;

// have members?
if( !the_base->nspc )
Expand All @@ -5235,7 +5249,7 @@ t_CKTYPE type_engine_check_exp_dot_member( Chuck_Env * env, a_Exp_Dot_Member mem
if( str == "this" )
{
// uh
if( base_static )
if( base_type_static )
{
EM_error2( member->where,
"keyword 'this' must be associated with object instance" );
Expand Down Expand Up @@ -5264,7 +5278,7 @@ t_CKTYPE type_engine_check_exp_dot_member( Chuck_Env * env, a_Exp_Dot_Member mem
}

// make sure
if( base_static && value->is_member )
if( base_type_static && value->is_instance_member )
{
// this won't work
EM_error2( member->where,
Expand All @@ -5273,6 +5287,15 @@ t_CKTYPE type_engine_check_exp_dot_member( Chuck_Env * env, a_Exp_Dot_Member mem
return NULL;
}

// FYI verification of static initialization rules reside in the emitter
// specifically in emit_engine_emit_exp_dot_member() -- this is due to
// the emitter implicitly handling both X.Y and Y (where Y is used inside
// X's definition), but the type-checker currently does not do this
// FYI this is for detecting things like:
// ""cannot call non-static function '%s' to initialize a static variable"
// 1.5.4.4 (ge) commented after trying to move the logic to this point
// and seeing incorrect unit test behavior | #2024-static-init

return value->type;
}

Expand Down Expand Up @@ -7096,7 +7119,7 @@ Chuck_Type * type_engine_import_class_begin( Chuck_Env * env, Chuck_Type * type,
value->owner = where; CK_SAFE_ADD_REF( value->owner );
// CK_SAFE_REF_ASSIGN( value->owner, where );
value->is_const = TRUE;
value->is_member = FALSE;
value->is_instance_member = FALSE;

// add to env
where->add_value( value->name, value );
Expand Down Expand Up @@ -9613,7 +9636,7 @@ Chuck_Value::Chuck_Value( Chuck_Type * t, const std::string & n, void * a,
is_const = c; access = acc;
owner = o; CK_SAFE_ADD_REF( owner ); // add reference
owner_class = oc; CK_SAFE_ADD_REF( owner_class ); // add reference
addr = a; is_member = FALSE;
addr = a; is_instance_member = FALSE;
is_static = FALSE; is_context_global = FALSE;
is_decl_checked = TRUE; // only set to false in certain cases
is_global = FALSE;
Expand Down Expand Up @@ -10020,8 +10043,9 @@ const Chuck_Value_Dependency * Chuck_Value_Dependency_Graph::locateLocal(
{
// 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)) )
// 1.5.4.4 (ge) add is_static to the logic (otherwise this will not work for static variables) #2024-static-init
if( (fromClassDef == NULL && !v->is_instance_member && !v->is_static) ||
(fromClassDef && equals(v->owner_class, fromClassDef) && (v->is_instance_member|| v->is_static)) )
{
// return dependency
return &directs[i];
Expand Down
Loading

0 comments on commit 0e83241

Please sign in to comment.