Skip to content

<c-void>, warning removal, external linkage for variables #12

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

<c-void>, warning removal, external linkage for variables #12

wants to merge 5 commits into from

Conversation

iglennrogers
Copy link
Contributor

Sorry, this is as one blob - slight issue of messing up the repository.

  1. typedef void FILE now produces a <c-void> rather than <void> which produces an error.
  2. Making various slots constant (removing -setter not used warnings) and commenting out some unused slots, bringing the number of warnings in melange & melange-parser to 1.
  3. Melange now produces code for variables, with an additional external: #t in the variable or include-file clause of the interface definition for external linkage.

@hannesm
Copy link
Contributor

hannesm commented Jul 4, 2012

first of all: big thanks.

it would be even better if you could separate 1, 2 and 3 in separate commits (using git add --interactive).

I actually do not completely understand 3 - the changes are that external is passed around in find-dylan-name and compute-dylan-name - but the only user is write-declaration (which indirectly accesses this argument in define method find-dylan-name (decl :: <variable-declaration>, mapper :: <function>, prefix :: <string>, containers :: <sequence>, rd-only :: <boolean>, sealing :: <string>, external :: <boolean>, #next next-method) => (result :: <string>);

Or do I miss something?

@hannesm
Copy link
Contributor

hannesm commented Jul 4, 2012

(somehow I've the feeling that the complexity external adds is quite high (by adding an argument to every method), but I don't have a good idea for an alternative solution)

Slots with required-init-keywords made constant; some unused slots commented out (may be used in future).
The warning that remains is from parsergen processing of the input files.
@iglennrogers
Copy link
Contributor Author

Splitting in progress, or rather a series of blow-by-blow sessions with diff :( Should get it finished this evening.

You're right that there's a lot of extra work involved with external, having to pass it as a parameter everywhere in sight, just to use it twice. Just about the only way around it is to assemble the mapper ... external parameters into a class and pass that instead, reassembling it as required. Worth thinking about if another parameter needs to be added on for some reason.

The working/real code does the following (from memory):

  • Change the <variable-declaration> to derive from <container-options> so it can include read-only etc members.
  • Change the parser so that it accepts external: phrases in the interface definition.
  • Add a new method to write out a define c-variable clause.
  • Add a line merging the external: setting into the global container options.

Allow variables to be added into the interface definition. Add an external: #t/f
clause for external linkage.
@waywardmonkeys
Copy link
Member

I've hand-merged parts of this in my next branch that is landing ... but not the "Add definition of c-variable ..." part yet.

I don't like adding the external stuff everywhere (and what's that for?).

@waywardmonkeys
Copy link
Member

For reference, my branch is here: https://github.com/waywardmonkeys/melange/commits/misc-improvements

I'm hoping to do more stuff on it and land it within a couple of days.

@iglennrogers
Copy link
Contributor Author

The external stuff is for global variables, whether they need to imported from another library, or whether they are part of a c-file within the dylan library. See "http://opendylan.org/documentation/library-reference/c-ffi/index.html#c-ffi:c-ffi:definec-variable" for reference. It certainly feels like a lot of effort, for something that shouldn't be used very often.

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

Successfully merging this pull request may close these issues.

3 participants