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

Add -restrict option; restrict symbol generation to JSON file #43

Merged
merged 4 commits into from
Aug 18, 2014
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,6 @@ A few notes about the flags to `generate`:
- `version`: The API version to generate. The `all` pseudo-version includes all functions and enumerations for the specified API.
- `profile`: For `gl` packages with version 3.2 or higher, `core` or `compatibility` ([explanation](http://www.opengl.org/wiki/Core_And_Compatibility_in_Contexts)).
- `addext`: A regular expression describing which extensions to include. `.*` by default, including everything.
- `restrict`: A JSON file that explicitly lists what enumerations / functions that Glow should generate (see example.json).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this control obsoletes the addext and remext flags. Perhaps we could consider pulling out the functionality in this change? Otherwise we can take a separate clean-up pass.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not so certain. Other people may be using addext/remext today and want it to keep working. Also it allows for a regular expression which could be more useful in certain situations -- don't think these options overlap each-other but instead compliment one another.

If you want, I can remove them though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the parameters for now. Over the long term perhaps the right answer is to allow the JSON file to specify direct and regex inclusion parameters. Until then I agree we should keep a consistent API.

- `remext`: A regular expression describing which extensions to exclude. Empty by default, excluding nothing. Takes precedence over explicitly added regular expressions.
- `lenientInit`: Flag to disable strict function availability checks at `Init` time. By default if any non-extension function pointer cannot be loaded then initialization fails; when this flag is set initialization will succeed with missing functions. Note that on some platforms unavailable functions will load successfully even but fail upon invocation so check against the OpenGL context what is supported.
11 changes: 11 additions & 0 deletions example.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"Enums": [
"GL_CCW",
"GL_COLOR_ATTACHMENT0",
"GL_COMPRESSED_RGB"
],
"Functions": [
"glBindTextures",
"glBindVertexArray"
]
}
34 changes: 34 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package main

import (
"encoding/json"
"flag"
"fmt"
"io/ioutil"
Expand Down Expand Up @@ -61,6 +62,7 @@ func generate(name string, args []string) {
profile := flags.String("profile", "", "API profile to generate (e.g., core)")
addext := flags.String("addext", ".*", "Regular expression of extensions to include (e.g., .*)")
remext := flags.String("remext", "$^", "Regular expression of extensions to exclude (e.g., .*)")
restrict := flags.String("restrict", "", "JSON file of symbols to restrict symbol generation")
lenientInit := flags.Bool("lenientInit", false, "When true missing functions do not fail Init")
flags.Parse(args)

Expand Down Expand Up @@ -97,6 +99,9 @@ func generate(name string, args []string) {
pkg = spec.ToPackage(packageSpec)
pkg.SpecRev = rev
docs.AddDocs(pkg)
if len(*restrict) > 0 {
performRestriction(pkg, *restrict)
}
if err := pkg.GeneratePackage(); err != nil {
log.Fatal("error generating package:", err)
}
Expand All @@ -109,6 +114,35 @@ func generate(name string, args []string) {
log.Println("generated package in", pkg.Dir())
}

// Converts a slice string into a simple lookup map.
func lookupMap(s []string) map[string]struct{} {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: prefer map[string]bool 'cause I think that is a more idiomatic way of representing a set

lookup := make(map[string]struct{}, len(s))
for _, str := range s {
lookup[str] = struct{}{}
}
return lookup
}

type jsonRestriction struct {
Enums []string
Functions []string
}

// Reads the given JSON file path into jsonRestriction and filters the package
// accordingly.
func performRestriction(pkg *Package, jsonPath string) {
data, err := ioutil.ReadFile(jsonPath)
if err != nil {
log.Fatal("error reading JSON restriction file:", err)
}
var r jsonRestriction
err = json.Unmarshal(data, &r)
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: prefer if err := json.Unmarshal(data &r); err != nil { for consistency

log.Fatal("error parsing JSON restriction file:", err)
}
pkg.Filter(lookupMap(r.Enums), lookupMap(r.Functions))
}

func parseSpecifications(xmlDir string) ([]*Specification, string) {
specDir := filepath.Join(xmlDir, "spec")
specFiles, err := ioutil.ReadDir(specDir)
Expand Down
25 changes: 25 additions & 0 deletions package.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,31 @@ func (pkg *Package) HasRequiredFunctions() bool {
return false
}

// Filter removes any enums, or functions found in this package that are not
// listed in the given lookup maps. If either of the maps has a length of zero,
// filtering does not occur for that type (e.g. all functions are left intact).
func (pkg *Package) Filter(enums, functions map[string]struct{}) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a thought, how do you feel about post-generation filtering vs. building filtering into package generation? I'm wondering if these restrictions should be factored into PackageSpec and respected in ToPackage. It might end up being more code however... In any case this implementation is fine, I'm just thinking out loud.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I certainly did think about it. My thought was that it would end up being significantly more code and I wasn't sure if such a chance would be desirable. At any rate I don't think generation speed is particularly bad even with this method -- could we settle on a:

// TODO(slimsag): move post-generation filtering into pre-generation filtering

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can omit the TODO for now; I'm not yet convinced I'm suggesting a superior implementation. It was more a thought to bear in mind for future refactoring passes.

if len(enums) > 0 {
// Remove any enum not listed in the enums lookup map.
for name := range pkg.Enums {
_, valid := enums[name]
if !valid {
delete(pkg.Enums, name)
}
}
}

if len(functions) > 0 {
// Remove any function not listed in the functions lookup map.
for name := range pkg.Functions {
_, valid := functions[name]
if !valid {
delete(pkg.Functions, name)
}
}
}
}

// This package's directory (relative to $GOPATH).
const pkgDir = "src/github.com/go-gl/glow"

Expand Down