Update for fidlgen_llcpp flags change
This CL fixes fidlbolt's invocation of fidlgen_llcpp now that the flags
have changed in I6ad1235d3259e3874293bb4a66113acd02c49e9f to just pass
--root instead of --header and --source. Since the paths it use include
the library name, this required some refactoring to expose the root
library name from the analysis.
Change-Id: I0f7f1abdd605a388ed1491c94a111b502426824f
Reviewed-on: https://fuchsia-review.googlesource.com/c/fidlbolt/+/587923
Reviewed-by: Yifei Teng <yifeit@google.com>
Reviewed-by: Ian McKellar <ianloic@google.com>
diff --git a/backend/analyze.go b/backend/analyze.go
index 7ccf018..fd2fb6b 100644
--- a/backend/analyze.go
+++ b/backend/analyze.go
@@ -99,29 +99,40 @@
"NOTE: Your file will be compiled with the others in this library. " +
"If you don't want this behavior, choose a different library name."
-// analyze analyzes content, returning a libraryMap that includes content's
-// library together with all its transitive dependencies, and annotations on the
-// "library" and "using" declarations indicating their inferred location. It
-// assumes that filename is a FIDL file containing content. If any library
-// cannot be located in a.paths, it is silently ignored.
-func (a *libraryAnalyzer) analyze(filename, content string) (libraryMap, []annotation) {
- result := make(libraryMap)
+// An analysis is the result of regex-parsing "library" and "using" declarations
+// in a FIDL file and all its transitive dependencies that can be located.
+type analysis struct {
+ // The library name from the input FIDL file, or "" if parsing failed.
+ root library
+ // Map containing the root library (even when it is "") and all transitive
+ // dependencies discovered during analysis.
+ libs libraryMap
+ // Annotations on the "library" and "using" declarations of the input FIDL
+ // file indicating their inferred location in the Fuchsia source tree.
+ annotations []annotation
+}
+
+// analyze analyzes content. It assumes that filename is a FIDL file containing
+// content. If any library cannot be located in a.paths, it is silently ignored.
+func (a *libraryAnalyzer) analyze(filename, content string) analysis {
+ result := analysis{libs: make(libraryMap)}
rootLibraryMatch, ok := parseLibraryMatch(content)
if !ok {
// If we can't parse the library name, there's no point in continuing.
// We could make this function return an error and report that to the
// user. However, we prefer to invoke fidlc anyway and show its error.
- // So, we return a libraryName with an empty library name such that
- // fidlcFileArgs() will produce --files filename.
- result[""] = libraryInfo{files: []string{filename}}
- return result, nil
+ // So, we set the libraryMap such that fidlcFileArgs() will produce
+ // --files filename.
+ result.libs[""] = libraryInfo{files: []string{filename}}
+ return result
}
+ result.root = rootLibraryMatch.lib
// Start with a stack containing the root library (if it's a platform source
// library, meaning we can expect to find it in the tree) and its imports.
var stack []library
- if rootLibraryMatch.lib.isPlatform() {
- stack = append(stack, rootLibraryMatch.lib)
+ if result.root.isPlatform() {
+ stack = append(stack, result.root)
}
rootImports := parsePlatformImportsMatch(content)
for _, m := range rootImports {
@@ -133,13 +144,13 @@
// Pop a library and check if it needs processing.
var lib library
lib, stack = stack[len(stack)-1], stack[:len(stack)-1]
- if _, ok := result[lib]; ok {
+ if _, ok := result.libs[lib]; ok {
continue
}
// Check the analyzer's cache.
if info, ok := a.cache.get(lib); ok {
- result[lib] = info
+ result.libs[lib] = info
stack = append(stack, info.deps...)
continue
}
@@ -187,12 +198,12 @@
}
}
if !info.empty() {
- result[lib] = info
+ result.libs[lib] = info
}
}
// Cache reusable results first, before adding request-specific information.
- a.cache.storeNew(result)
+ a.cache.storeNew(result.libs)
// The following handles two cases:
// 1. (More common) The user typed something like `library foo;`. This
@@ -200,25 +211,23 @@
// 2. The user typed something like `library fuchsia.io;`. This behaves as
// if they were adding a new file to the fuchsia.io library, alongside
// the other ones in the source tree.
- var anns []annotation
- rootLib := rootLibraryMatch.lib
rootInfo := libraryInfo{files: []string{filename}}
for _, m := range rootImports {
rootInfo.deps = append(rootInfo.deps, m.lib)
- if info, ok := result[m.lib]; ok {
+ if info, ok := result.libs[m.lib]; ok {
text := fmt.Sprintf("Found %s in %s", m.lib, info.userVisibleDir())
- anns = append(anns, m.annotation(Info, text))
+ result.annotations = append(result.annotations, m.annotation(Info, text))
}
}
- if info, ok := result[rootLib]; ok {
+ if info, ok := result.libs[result.root]; ok {
rootInfo = mergeInfo(rootInfo, info)
text := fmt.Sprintf("Found %s in %s\n\n%s",
- rootLib, info.userVisibleDir(), existingLibraryNotice)
- anns = append(anns, rootLibraryMatch.annotation(Info, text))
+ result.root, info.userVisibleDir(), existingLibraryNotice)
+ result.annotations = append(result.annotations, rootLibraryMatch.annotation(Info, text))
}
- result[rootLib] = rootInfo
+ result.libs[result.root] = rootInfo
- return result, anns
+ return result
}
// fidlcFileArgs returns a list of arguments to pass to fidlc to compile the
diff --git a/backend/server.go b/backend/server.go
index 1867800..357f010 100644
--- a/backend/server.go
+++ b/backend/server.go
@@ -263,13 +263,14 @@
}
case HLCPP:
switch r.Options.File {
- case "header", "source", "tables":
+ case "header", "source", "test", "tables":
default:
return fmt.Errorf("invalid HLCPP file option: %q", r.Options.File)
}
case LLCPP:
switch r.Options.File {
- case "header", "source", "tables", "test":
+ case "markers.h", "wire.h", "wire_types.h", "wire_types.cc",
+ "wire_messaging.h", "wire_messaging.cc", "wire_test_base.h", "tables":
default:
return fmt.Errorf("invalid LLCPP file option: %q", r.Options.File)
}
@@ -343,12 +344,12 @@
if err != nil {
return response{}, err
}
- libs, anns := s.libraryAnalyzer.analyze(fidl, r.Content)
- res, err := s.handleFIDLWithTemp(ctx, r, temp, fidl, libs)
+ analysis := s.libraryAnalyzer.analyze(fidl, r.Content)
+ res, err := s.handleFIDLWithTemp(ctx, r, temp, fidl, analysis)
if err != nil {
return res, err
}
- res.Annotations = append(res.Annotations, anns...)
+ res.Annotations = append(res.Annotations, analysis.annotations...)
// Remove all occurrences of the temp path to avoid exposing it to the user.
// This is mostly needed for error output, but it is also needed e.g. for
// the JSON IR, which includes source paths.
@@ -359,10 +360,10 @@
const jsonDepIndentSize = 4
-func (s *server) handleFIDLWithTemp(ctx context.Context, r *request, temp tempDir, fidl string, libs libraryMap) (response, error) {
+func (s *server) handleFIDLWithTemp(ctx context.Context, r *request, temp tempDir, fidl string, analysis analysis) (response, error) {
// Run fidlc, automatically appending the correct --files arguments.
fidlc := func(args ...string) (response, error) {
- fileArgs, err := libs.fidlcFileArgs()
+ fileArgs, err := analysis.libs.fidlcFileArgs()
if err != nil {
msg := fmt.Sprintf("%s:1:1: error: %s", fidl, err)
return response{Ok: false, Content: msg}, nil
@@ -380,6 +381,9 @@
} else {
res.Content = run.stderr
}
+ if run.success && analysis.root == "" {
+ return response{}, fmt.Errorf("input has no library declaration, yet fidlc succeeded")
+ }
return res, nil
}
@@ -425,7 +429,7 @@
}
return response{Ok: true, Content: run.stdout}, nil
case "deps":
- graph, err := libs.dependencyGraph()
+ graph, err := analysis.libs.dependencyGraph()
if err != nil {
msg := fmt.Sprintf("%s:1:1: error: %s", fidl, err)
return response{Ok: false, Content: msg}, nil
@@ -460,49 +464,48 @@
if !res.Ok {
return res, nil
}
- res.Content, err = s.handleFIDLWithIR(ctx, r, temp, jsonIR)
+ res.Content, err = s.handleFIDLWithIR(ctx, r, temp, jsonIR, analysis)
if err != nil {
return response{}, err
}
return res, nil
}
-func (s *server) handleFIDLWithIR(ctx context.Context, r *request, temp tempDir, jsonIR string) (string, error) {
+func (s *server) handleFIDLWithIR(ctx context.Context, r *request, temp tempDir, jsonIR string, analysis analysis) (string, error) {
switch r.OutputMode {
case HLCPP:
if _, err := s.fidlgenHlcpp.runInfallible(ctx,
"-json", jsonIR,
- "-include-base", temp.path,
- "-output-base", temp.join("impl"),
+ "-root", temp.path,
"-clang-format-path", s.clangFormat.path,
); err != nil {
return "", err
}
+ // TODO(fxbug.dev/85703): Sanitize paths.
+ libraryDirs := strings.ReplaceAll(string(analysis.root), ".", "/")
switch r.Options.File {
case "header":
- return temp.readFile("impl.h")
+ return temp.readFile(libraryDirs, "cpp", "fidl.h")
case "source":
- return temp.readFile("impl.cc")
+ return temp.readFile(libraryDirs, "cpp", "fidl.cc")
+ case "test":
+ return temp.readFile(libraryDirs, "cpp", "fidl_test_base.h")
default:
return "", fmt.Errorf("invalid file: %q", r.Options.File)
}
case LLCPP:
if _, err := s.fidlgenLlcpp.runInfallible(ctx,
"-json", jsonIR,
- "-header", temp.join("impl.h"),
- "-source", temp.join("impl.cc"),
- "-test-base", temp.join("test.h"),
+ "-root", temp.path,
"-clang-format-path", s.clangFormat.path,
); err != nil {
return "", err
}
switch r.Options.File {
- case "header":
- return temp.readFile("impl.h")
- case "source":
- return temp.readFile("impl.cc")
- case "test":
- return temp.readFile("test.h")
+ case "markers.h", "wire.h", "wire_types.h", "wire_types.cc",
+ "wire_messaging.h", "wire_messaging.cc", "wire_test_base.h":
+ // TODO(fxbug.dev/85703): Sanitize paths.
+ return temp.readFile("fidl", string(analysis.root), "cpp", r.Options.File)
default:
return "", fmt.Errorf("invalid file: %q", r.Options.File)
}
diff --git a/backend/system.go b/backend/system.go
index d887192..0e4ea12 100644
--- a/backend/system.go
+++ b/backend/system.go
@@ -158,9 +158,9 @@
return f.Name(), nil
}
-// readFile creates a response from a file in the temporary directory.
-func (d tempDir) readFile(name string) (string, error) {
- b, err := ioutil.ReadFile(d.join(name))
+// readFile creates a response from a file path in the temporary directory.
+func (d tempDir) readFile(elem ...string) (string, error) {
+ b, err := ioutil.ReadFile(d.join(elem...))
if err != nil {
return "", err
}
diff --git a/frontend/src/elm/Mode.elm b/frontend/src/elm/Mode.elm
index 89ee3e1..c5a48a1 100644
--- a/frontend/src/elm/Mode.elm
+++ b/frontend/src/elm/Mode.elm
@@ -183,14 +183,19 @@
( Fidl, Hlcpp ) ->
[ files ( "header", "Header" )
[ ( "source", "Source" )
+ , ( "test", "Test base" )
, ( "tables", "Tables" )
]
]
( Fidl, Llcpp ) ->
- [ files ( "header", "Header" )
- [ ( "source", "Source" )
- , ( "test", "Test base" )
+ [ files ( "markers.h", "Marker header" )
+ [ ( "wire.h", "Combined header" )
+ , ( "wire_types.h", "Domain object header" )
+ , ( "wire_types.cc", "Domain object source" )
+ , ( "wire_messaging.h", "Messaging header" )
+ , ( "wire_messaging.cc", "Messaging source" )
+ , ( "wire_test_base.h", "Test base" )
, ( "tables", "Tables" )
]
]
