Skip to content

Commit

Permalink
Relax procptr type checks
Browse files Browse the repository at this point in the history
This implements new type checking for pointer assignments, especially
procedure pointers. The idea is to allow assignments even between different
signatures, as long as they are compatible (e.g. Integer and Long are
different types, but compatible on 32bit).

This was already done for normal pointers, e.g. Integer Ptr and Long Ptr
are treated as compatible on 32bit, even though they're different types.

I think it'd be useful to have this for procedure pointers (i.e. function
result and parameter types) too. For example:
	declare function f() as integer
	dim p as function() as long = @f
Previously, this triggered a "suspicious pointer assignment" warning, but
now it will be allowed (on 32bit), because the signatures are compatible.
(on 64bit it will still result in a warning)

There are several interesting cases where compatibility is possible
despite the signature being different:

* Different integer types, but same size (Integer vs Long/LongInt, signed
  vs. unsigned), see the example above.

* Byref + UDT up-casting:

	type A extends object
	end type

	type B extends A
	end type

	declare	sub f( byref as A )
	dim p as sub( byref as B ) = @f

  This pointer assignment can be allowed, because in a p() call, a B object
  has to be passed. It will be passed to f() which expects	an A. It's ok
  because a B is an A.
  (of course, the opposite direction would not be safe)

  This isn't even entirely new - the old symbIsEqual() already had an
  inheritance-related check that almost made this work for function
  results, no matter whether BYVAL or BYREF.

  In case of BYVAL paramaters/function results it is generally not safe to
  allow this though (because in this case caller and callee have to agree
  on the size of the passed object). The code to handle this is currently
  commented out in the new typeCalcMatch() function, and I'm going to add
  that fix as a separate commit, because it was a problem before this one
  already, and isn't really related to the purpose of this commit.

* CONSTness:

  Just like the following is allowed:

	dim i as integer
	dim p as const integer ptr = @i

  we can allow this:

	declare sub f( byref as const integer )
	dim p as sub( byref as integer ) = @f

  f promises to never modify the given integer, so it doesn't matter
  whether a pointer to f uses CONST or not. (here, too, the opposite
  direction would not be safe)

In general, this patch removes symbIsEqual() and adds some new functions,
affecting the following areas in the compiler:
* astPtrCheck() type checks, which determine when "suspicious pointer
  assignment" or "passing different pointer type" warnings are shown.
  This is now more permissive, i.e. less warnings are shown than before.
* Overload resolution, which has to do very similar checks anyways and now
  re-uses the new functions, which at the very least makes it more
  consistent with normal assignment/parameter passing checks.
* Virtual method overriding, whose existing method signature comparison
  function is the base for the new more generic procedure signature
  compatibility checking function, also re-uses the new functions.
  Overriding virtuals is basically just assigning function pointers (in the
  vtable) anyways, and the same rules can be applied.

This change is useful for new 64bit-compatible Windows API headers.
The Windows API has a lot of callback functions returning a 32bit integer.
In fbc's old headers, that was translated as Integer, but in the new
64bit-compatible headers it has to be translated as Long. This caused
warnings to appear for existing code which implemented such callbacks
using Integer. With this change, the existing code will keep compiling
fine on 32bit, and only cause warnings on 64bit.

Also, this change implements feature request #289 (Covariant parameters and
function results), at least what I had in mind.
  • Loading branch information
dkl committed Jan 9, 2015
1 parent 5661b5c commit 4a68bac
Show file tree
Hide file tree
Showing 70 changed files with 5,160 additions and 2,522 deletions.
2 changes: 2 additions & 0 deletions changelog.txt
Expand Up @@ -2,12 +2,14 @@ Version 1.02.0

[changed]
- Redims with explicit type will now redim existing dynamic array fields, if one of the same name exists, instead of creating a new local array. This makes Redims with explicit type match the behaviour of typeless Redims and Redims for global arrays.
- Relaxed type checking for procedure pointers and virtual method overrides: Procedures with different signatures can now be treated equal if the signatures are compatible. For example: assignments between a function pointer returning an Integer and one returning a Long won't cause a "suspicious pointer assignment" warning on 32bit anymore.

[added]
- __FB_ASM__ intrinsic #define on x86, defined to "intel" or "att" (corresponding to the -asm command line option -- useful for -gen gcc/llvm only)
- fbc -showincludes option (shows #include tree) for debugging of #includes
- Address-of followed by pointer-arithmetic on variables (expressions such as @x+N) will now be turned into offsetted variable accesses, resulting in better generated code
- The results of type-casted function calls can now be ignored, even if it's a non-trivial cast
- Covariant parameters and function results (feature request #289)
- Bindings (new/updated, including 64bit support):
FastCGI 2.4.1-SNAP-0311112127

Expand Down
69 changes: 17 additions & 52 deletions src/compiler/ast-misc.bas
Expand Up @@ -223,7 +223,7 @@ function astIsEqualParamInit _
'' symbols, but if they have the same signature they should still be
'' treated equal here.
if( typeGetDtOnly( l->dtype ) = FB_DATATYPE_FUNCTION ) then
if( symbIsEqual( l->subtype, r->subtype ) = FALSE ) then
if( symbCalcProcMatch( l->subtype, r->subtype, 0 ) = 0 ) then
exit function
end if
else
Expand Down Expand Up @@ -622,20 +622,17 @@ sub astCheckConst _
end if
end sub

'':::::
function astPtrCheck _
( _
byval pdtype as integer, _
byval psubtype as FBSYMBOL ptr, _
byval expr as ASTNODE ptr, _
byval strictcheck as integer _
byval pparammode as integer, _
byval expr as ASTNODE ptr _
) as integer

dim as integer edtype = any

assert( typeIsPtr( pdtype ) )
function = FALSE

edtype = astGetFullType( expr )
var edtype = astGetFullType( expr )

'' expr not a pointer?
if( typeIsPtr( edtype ) = FALSE ) then
Expand All @@ -648,53 +645,21 @@ function astPtrCheck _
exit function
end if

'' different constant masks?
if( strictcheck ) then
if( typeGetPtrConstMask( edtype ) <> _
typeGetPtrConstMask( pdtype ) ) then
exit function
'' Are ANY PTRs involved? Then we don't need to do exact type checks
if( typeGetDtOnly( pdtype ) = FB_DATATYPE_VOID ) then
if( typeGetDtOnly( edtype ) = FB_DATATYPE_VOID ) then
return TRUE
end if
return (typeGetPtrCnt( pdtype ) <= typeGetPtrCnt( edtype ))
elseif( typeGetDtOnly( edtype ) = FB_DATATYPE_VOID ) then
return (typeGetPtrCnt( pdtype ) >= typeGetPtrCnt( edtype ))
end if

'' different types?
if( typeGetDtAndPtrOnly( pdtype ) <> typeGetDtAndPtrOnly( edtype ) ) then

'' remove the pointers
dim as integer pdtype_np = any, edtype_np = any
pdtype_np = typeGetDtOnly( pdtype )
edtype_np = typeGetDtOnly( edtype )

'' 1st) are ANY PTRs involved? Then we don't need to do exact type checks
if( pdtype_np = FB_DATATYPE_VOID ) then
if( edtype_np = FB_DATATYPE_VOID ) then
return TRUE
end if
return (typeGetPtrCnt( pdtype ) <= typeGetPtrCnt( edtype ))
elseif( edtype_np = FB_DATATYPE_VOID ) then
return (typeGetPtrCnt( pdtype ) >= typeGetPtrCnt( edtype ))
end if

'' 2nd) same level of indirection?
if( typeGetPtrCnt( pdtype ) <> typeGetPtrCnt( edtype ) ) then
exit function
end if

'' 4th) same size and class?
if( (pdtype_np <= FB_DATATYPE_DOUBLE) and _
(edtype_np <= FB_DATATYPE_DOUBLE) ) then
if( typeGetSize( pdtype_np ) = typeGetSize( edtype_np ) ) then
if( typeGetClass( pdtype_np ) = typeGetClass( edtype_np ) ) then
return TRUE
end if
end if
end if

exit function
end if

'' check sub types
function = symbIsEqual( astGetSubType( expr ), psubtype )

'' Check type compatibility, but without checking CONSTness. Users of astPtrCheck()
'' do this manually already (and if there is a CONSTness mismatch, they show an error,
'' not just a warning, which is astPtrCheck()'s main purpose).
function = (typeCalcMatch( typeGetDtAndPtrOnly( pdtype ), psubtype, pparammode, _
typeGetDtAndPtrOnly( edtype ), expr->subtype ) > 0)
end function

'':::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/ast-node-arg.bas
Expand Up @@ -912,7 +912,7 @@ private function hCheckParam _

'' pointer checking
if( typeIsPtr( param_dtype ) ) then
if( astPtrCheck( symbGetFullType( param ), symbGetSubtype( param ), n->l ) = FALSE ) then
if( astPtrCheck( param->typ, param->subtype, param->param.mode, n->l ) = FALSE ) then
if( typeIsPtr( arg_dtype ) = FALSE ) then
errReportWarn( FB_WARNINGMSG_PASSINGSCALARASPTR )
else
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/ast-node-assign.bas
Expand Up @@ -211,7 +211,7 @@ private function hCheckConstAndPointerOps _
exit function
end if

if( astPtrCheck( ldtype, l->subtype, r ) = FALSE ) then
if( astPtrCheck( ldtype, l->subtype, FB_PARAMMODE_BYVAL, r ) = FALSE ) then
'' if both are UDT, a derived lhs can't be assigned from a base rhs
if( typeGetDtOnly( ldtype ) = FB_DATATYPE_STRUCT and typeGetDtOnly( rdtype ) = FB_DATATYPE_STRUCT ) then
if( symbGetUDTBaseLevel( astGetSubType( l ), astGetSubType( r ) ) > 0 ) then
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/ast-node-call.bas
Expand Up @@ -55,7 +55,7 @@ function astNewCALL _
''
if( ptrexpr ) then
assert( astGetDataType( ptrexpr ) = typeAddrOf( FB_DATATYPE_FUNCTION ) )
assert( (ptrexpr->subtype = sym) or symbIsEqual( ptrexpr->subtype, sym ) )
assert( (ptrexpr->subtype = sym) or (symbCalcProcMatch( ptrexpr->subtype, sym, 0 ) > 0) )
sym = ptrexpr->subtype
end if

Expand Down
4 changes: 2 additions & 2 deletions src/compiler/ast.bi
Expand Up @@ -511,8 +511,8 @@ declare function astPtrCheck _
( _
byval pdtype as integer, _
byval psubtype as FBSYMBOL ptr, _
byval expr as ASTNODE ptr, _
byval strictcheck as integer = FALSE _
byval pparammode as integer, _
byval expr as ASTNODE ptr _
) as integer

declare function astNewNOP _
Expand Down
66 changes: 66 additions & 0 deletions src/compiler/symb-data.bas
Expand Up @@ -490,3 +490,69 @@ function closestType _
return dtype1

end function

function typeCalcMatch _
( _
byval ldtype as integer, _
byval lsubtype as FBSYMBOL ptr, _
byval lparammode as integer, _
byval rdtype as integer, _
byval rsubtype as FBSYMBOL ptr _
) as integer

if( (ldtype = rdtype) and (lsubtype = rsubtype) ) then
return FB_OVLPROC_FULLMATCH
end if

if( typeGetPtrCnt( ldtype ) <> typeGetPtrCnt( rdtype ) ) then
return 0
end if

if( symbCheckConstAssign( ldtype, rdtype, lsubtype, rsubtype, lparammode ) = FALSE ) then
return 0
end if

if( (typeGetDtAndPtrOnly( ldtype ) = typeGetDtAndPtrOnly( rdtype )) and (lsubtype = rsubtype) ) then
return FB_OVLPROC_HALFMATCH
end if

'' We know that they're different (in terms of dtype or subtype or both),
'' but they have the same ptrcount and CONSTs don't disallow the assignment.

var ldt = typeGetDtOnly( ldtype )
var rdt = typeGetDtOnly( rdtype )
if( ldt <> rdt ) then
'' Different base dtype, so probably not compatible.

'' The only exception here are integer types with the same size.
'' This applies to plain integers, and also to integer pointers.
'' For example: Integer [Ptr] will be treated as compatible to Long/LongInt [Ptr],
'' on 32bit/64bit respectively, and vice-versa.
if( (typeGetClass( ldt ) = FB_DATACLASS_INTEGER) and _
(typeGetClass( rdt ) = FB_DATACLASS_INTEGER) and _
(typeGetSize( ldt ) = typeGetSize( rdt )) ) then
return FB_OVLPROC_HALFMATCH - symb_dtypeMatchTB( ldt, rdt )
end if

return 0
end if

select case( ldt )
case FB_DATATYPE_STRUCT
'' Allow up-casting for any pointers, and BYREF parameters/function results,
'' but not for BYVAL parameters/function results. At least for BYVAL function results,
'' it's not safe, as the caller allocates the temp var where the callee will write
'' the result, thus they must use the exact same type or there will be a buffer overflow.
'if( typeIsPtr( ldtype ) or (lparammode = FB_PARAMMODE_BYREF) ) then
'' Check whether r is derived from l.
if( symbGetUDTBaseLevel( rsubtype, lsubtype ) > 0 ) then
return FB_OVLPROC_HALFMATCH
end if
'end if
case FB_DATATYPE_FUNCTION
'' Allow different procptrs as long as the signatures are compatible enough
return symbCalcProcMatch( lsubtype, rsubtype, 0 )
end select

function = 0
end function

0 comments on commit 4a68bac

Please sign in to comment.