Skip to content

Commit

Permalink
Topologically sort ast.FileSet.Files so imported files come first.
Browse files Browse the repository at this point in the history
This matches protoc behaviour.
  • Loading branch information
dsymonds committed Mar 26, 2016
1 parent 37e69c0 commit 2b8ac83
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 3 deletions.
1 change: 1 addition & 0 deletions .gitignore
@@ -1,5 +1,6 @@
.*.swp
.DS_Store
_mini*
_testmain.go
gotoc
testdata/*.actual
Expand Down
15 changes: 15 additions & 0 deletions Makefile
Expand Up @@ -21,3 +21,18 @@ baseline:
@protoc --descriptor_set_out=$(MINI_TMP) --include_imports testdata/mini.proto
@protoc --decode=google.protobuf.FileDescriptorSet -I $(PROTOBUF) $(PROTOBUF)/src/google/protobuf/descriptor.proto < $(MINI_TMP)
@rm $(MINI_TMP)

MINI_FSET_GOTOC=_mini_fset_gotoc.txt
MINI_FSET_PROTOC=_mini_fset_protoc.txt
baselinediff:
go build
# First, gotoc
@./gotoc --descriptor_only testdata/mini.proto > $(MINI_FSET_GOTOC)
@sed -i '' 's/: </ {/g' $(MINI_FSET_GOTOC)
@sed -i '' 's/>$$/}/g' $(MINI_FSET_GOTOC)
# Next, protoc
@protoc --descriptor_set_out=$(MINI_TMP) --include_imports testdata/mini.proto
@protoc --decode=google.protobuf.FileDescriptorSet -I $(PROTOBUF) $(PROTOBUF)/src/google/protobuf/descriptor.proto < $(MINI_TMP) > $(MINI_FSET_PROTOC)
# Compare
diff -ud $(MINI_FSET_PROTOC) $(MINI_FSET_GOTOC)
@rm $(MINI_TMP) $(MINI_FSET_GOTOC) $(MINI_FSET_PROTOC)
36 changes: 36 additions & 0 deletions ast/ast.go
Expand Up @@ -17,9 +17,45 @@ type Node interface {

// FileSet describes a set of proto files.
type FileSet struct {
// Files is sorted in topological order, bottom up.
// That means that a file X will only import a file Y
// if Y occurs in this slice before X.
Files []*File
}

// Sort sorts fs.Files topologically.
func (fs *FileSet) Sort() {
in := fs.Files // old version of fs.Files; shrinks each loop
out := make([]*File, 0, len(in)) // new version of fs.Files; grows each loop
done := make(map[string]bool) // filenames that we've seen and that don't have un-done imports
for len(in) > 0 {
// Find a file that doesn't have an un-done import.
var next *File
for i, f := range in {
ok := true
for _, imp := range f.Imports {
if !done[imp] {
ok = false
break
}
}
if !ok {
continue
}
next = f
copy(in[i:], in[i+1:])
in = in[:len(in)-1]
break
}
if next == nil {
panic("import loop!") // shouldn't happen
}
out = append(out, next)
done[next.Name] = true
}
fs.Files = out
}

// File represents a single proto file.
type File struct {
Name string // filename
Expand Down
3 changes: 1 addition & 2 deletions main.go
Expand Up @@ -76,8 +76,7 @@ func main() {
// Prepare request.
cgRequest := &plugin.CodeGeneratorRequest{
FileToGenerate: flag.Args(),
// TODO: proto_file should be topologically sorted (bottom-up)
ProtoFile: fds.File,
ProtoFile: fds.File,
}
buf, err := proto.Marshal(cgRequest)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions parser/parser.go
Expand Up @@ -86,6 +86,7 @@ func ParseFiles(filenames []string, importPaths []string) (*ast.FileSet, error)
if err := resolveSymbols(fset); err != nil {
return nil, err
}
fset.Sort()
return fset, nil
}

Expand Down
2 changes: 1 addition & 1 deletion testdata/run.sh
Expand Up @@ -17,7 +17,7 @@ for ((i=1; $i <= $MAX; i=$((i+1)))); do

$GOTOC --descriptor_only $i.proto > $i.actual
$PROTOCMP $i.expected $i.actual || {
echo "==> FAILED" 1>&2
echo "==> FAILED expected vs. actual" 1>&2
failures=$(($failures + 1))
}

Expand Down

0 comments on commit 2b8ac83

Please sign in to comment.