Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cil.typsig conflates distinct function types #8

Closed
stephenrkell opened this issue Jan 22, 2014 · 6 comments
Closed

Cil.typsig conflates distinct function types #8

stephenrkell opened this issue Jan 22, 2014 · 6 comments

Comments

@stephenrkell
Copy link
Contributor

CIL typsigs do not distinguish between the types

typedef int f();

and

typedef int f(void);

I will call the first case "no arguments specified" and the second "specified as no-arguments". They both yield the typesigs

TSFun(TSBase(void ), [],false, [])

Note that Cil.typ does encode this distinction, since the argument list can be Some([]) or None.

A possible fix which doesn't break absolutely everybody's code might be to take the C++ approach: the "no arguments specified" case is encoded as a zero-arguments varargs function, i.e.

TSFun(TSBase(void ), [], true, [])

whereas the "specified as no-arguments" case remains as-is.

I could be persuaded to work on a patch to fix this, although I am just working around it for now. :-)

@kerneis
Copy link
Contributor

kerneis commented Jan 23, 2014

On Wed, Jan 22, 2014 at 04:30:33AM -0800, Stephen Kell wrote:

Note that Cil.typ does encode this distinction, since the argument
list can be Some([]) or None.

Off the top of my head and without checking the code, isn't it precisely
the whole point of typsig ("normalising" equivalent types)?

I'll have a more detailed look at it next week.

@stephenrkell
Copy link
Contributor Author

On Thu, 23 Jan 2014 13:32:15 -0800, Gabriel Kerneis wrote:

Off the top of my head and without checking the code, isn't it
precisely the whole point of typsig ("normalising" equivalent types)?

I'll have a more detailed look at it next week.

The point is that

int f(void);

and

int f();

are not equivalent in C. If you read the standard, you can find
several places where the treatment of a function type without an
argument list differs from that with an argument list (e.g. 6.2.7
"Compatible type and composite type", or 6.7.6.3 "Function declarators
(including prototypes)").

Next week (or later still) is fine... my workaround is working-around
nicely. :-)

Stephen

@kerneis
Copy link
Contributor

kerneis commented Mar 10, 2014

Next release will be 2.0 with other incompatible changes. I don't care to break everybody's code if it does the right thing — that's why major releases are for. I suspect few people pattern-match on typsig anyway, they are intended for comparison in the first place; also, this is a trivial fix for affected users to do.

@kerneis kerneis closed this as completed Mar 10, 2014
@stephenrkell
Copy link
Contributor Author

It's definitely not trivial to work around in client code. The client asks for a typsig and gets one back which is subtly inaccurate, perhaps arbitrarily far down in the nest. Fixing it in the client means implementing your own typsig. (Also, I pattern-match on typsig an awful lot.)

My workaround happens far away, outside my CIL code, on detecting a failure which "shouldn't happen", and is an outrageous hack.

I'll produce a patch in (hopefully) a few weeks' time. In the meantime it would be more appropriate to leave this issue open, although as you wish.

@kerneis
Copy link
Contributor

kerneis commented Mar 10, 2014

It's definitely not trivial to work around in client code.

There is a misunderstanding. I'll introduce an option in TSFun to
mimick the option in TFun. What I meant is that it will be trivial
for anybody relying on the old, option-less API to update their code.
And for you, with the new, accurate API, it's now trivial to take the
right decision.

@stephenrkell
Copy link
Contributor Author

Ah, okay, cool. :-)

kerneis added a commit that referenced this issue Mar 10, 2014
This is an incompatible API change.
It allows to distinguish the types of f() and
f(void), which are treated differently in several
places of the C standard.

Closes #8, thanks to Stephen Kell.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants