Skip to content

Commit

Permalink
refactor: Remove flux_analyze (#5335)
Browse files Browse the repository at this point in the history 
* refactor: Remove flux_analyze

It can be implemented in terms of `flux_analyze_with`

* chore: make generate
  • Loading branch information
Markus Westerlind authored Nov 9, 2022
1 parent 1cdebda commit 1db3400
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 167 deletions.
89 changes: 2 additions & 87 deletions libflux/c/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ void test_env_stdlib();

int main(int argc, char* argv[]) {
test_ast();
test_semantic();
test_stateful_analyzer();
test_env_stdlib();
return 0;
Expand Down Expand Up @@ -68,91 +67,6 @@ void test_ast() {
}
}

void test_semantic() {
printf("Testing semantic graph functions...\n");

{
printf("Parsing to AST\n");
struct flux_ast_pkg_t *ast_pkg_foo = flux_parse("test", "package foo\nx = 1 + 1");
assert(ast_pkg_foo != NULL);

printf("Analyzing (expect success)\n");
struct flux_semantic_pkg_t* sem_pkg = NULL;
struct flux_error_t* err = flux_analyze(ast_pkg_foo, "", &sem_pkg);
assert(err == NULL);

printf("Marshaling to FlatBuffer\n");
struct flux_buffer_t buf;
err = flux_semantic_marshal_fb(sem_pkg, &buf);
assert(err == NULL);
printf(" FlatBuffer is length %ld\n", buf.len);
flux_free_bytes(buf.data);

flux_free_semantic_pkg(sem_pkg);
}

{
printf("Parsing to AST\n");
struct flux_ast_pkg_t *ast_pkg_foo = flux_parse("test", "package foo\nx = 1 + 1.0");
assert(ast_pkg_foo != NULL);

printf("Analyzing (expect failure)\n");
struct flux_semantic_pkg_t* sem_pkg = NULL;
struct flux_error_t* err = flux_analyze(ast_pkg_foo, "", &sem_pkg);
assert(err != NULL);
assert(sem_pkg != NULL);
const char* err_str = flux_error_str(err);
printf(" error: %s\n", err_str);
flux_free_error(err);
flux_free_semantic_pkg(sem_pkg);
}

{
printf("Parsing to AST\n");
struct flux_ast_pkg_t *ast_pkg_foo = flux_parse("test", "package foo\nx = 1 + 1");
assert(ast_pkg_foo != NULL);

struct flux_semantic_pkg_t* sem_pkg = NULL;
struct flux_error_t* err = flux_analyze(ast_pkg_foo, "", &sem_pkg);
assert(err == NULL);
assert(sem_pkg != NULL);

printf("Find variable type v (expect success)\n");
struct flux_buffer_t buf;
err = flux_find_var_type(sem_pkg, "v", &buf);
// Note that we do not call flux_free_ast_pkg(ast_pkg_foo); here because we will
// consume the AST package during the conversion from the AST package to the semantic package.
assert(err == NULL);
printf(" FlatBuffer is length %ld\n", buf.len);
flux_free_bytes(buf.data);
flux_free_semantic_pkg(sem_pkg);
}

{
printf("Parsing to AST\n");
struct flux_ast_pkg_t *ast_pkg_foo = flux_parse("test", "package foo\nx = 1 + 1.0");
assert(ast_pkg_foo != NULL);

struct flux_semantic_pkg_t* sem_pkg = NULL;
struct flux_error_t* err = flux_analyze(ast_pkg_foo, "", &sem_pkg);
assert(err != NULL);
assert(sem_pkg != NULL);
const char* err_str = flux_error_str(err);
printf(" error: %s\n", err_str);
flux_free_error(err);

printf("Find variable type v (expect failure)\n");
struct flux_buffer_t buf;
err = flux_find_var_type(sem_pkg, "v", &buf);
assert(err == NULL);

flux_free_bytes(buf.data);
flux_free_semantic_pkg(sem_pkg);
}

printf("\n");
}

void test_stateful_analyzer() {
printf("Testing semantic analyzer...\n");

Expand Down Expand Up @@ -186,7 +100,8 @@ void test_stateful_analyzer() {
sem_pkg = NULL;
err = flux_analyze_with(analyzer, NULL, ast_pkg, &sem_pkg);
assert(err != NULL);
assert(sem_pkg == NULL);
assert(sem_pkg != NULL);
flux_free_semantic_pkg(sem_pkg);
const char* err_str = flux_error_str(err);
printf(" error: %s\n", err_str);
flux_error_print(err);
Expand Down
55 changes: 13 additions & 42 deletions libflux/flux/src/cffi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,40 +344,6 @@ pub unsafe extern "C" fn flux_merge_ast_pkgs(
.unwrap_or_else(|err| Some(err.into()))
}

/// flux_analyze is a C-compatible wrapper around the analyze() function below
///
/// Note that Box<T> is used to indicate we are receiving/returning a C pointer and also
/// transferring ownership.
///
/// # Safety
///
/// This function is unsafe because it dereferences a raw pointer.
#[no_mangle]
#[allow(clippy::boxed_local)]
pub unsafe extern "C" fn flux_analyze(
ast_pkg: Box<ast::Package>,
options: *const c_char,
out_sem_pkg: *mut Option<Box<semantic::nodes::Package>>,
) -> Option<Box<ErrorHandle>> {
catch_unwind(|| {
let options = match Options::from_c_str(options) {
Ok(x) => x,
Err(err) => return Some(err.into()),
};
match analyze(&ast_pkg, options) {
Ok(sem_pkg) => {
*out_sem_pkg = Some(Box::new(sem_pkg));
None
}
Err(salvage) => {
*out_sem_pkg = salvage.value.map(Box::new);
Some(salvage.error.into())
}
}
})
.unwrap_or_else(|err| Some(err.into()))
}

/// flux_find_var_type() is a C-compatible wrapper around the find_var_type() function below.
/// Note that Box<T> is used to indicate we are receiving/returning a C pointer and also
/// transferring ownership.
Expand Down Expand Up @@ -455,7 +421,10 @@ pub struct StatefulAnalyzer {
}

impl StatefulAnalyzer {
fn analyze(&mut self, ast_pkg: &ast::Package) -> Result<fluxcore::semantic::nodes::Package> {
fn analyze(
&mut self,
ast_pkg: &ast::Package,
) -> SalvageResult<fluxcore::semantic::nodes::Package, Error> {
let Options { features } = self.options.clone();

let env = Environment::from(&self.env);
Expand All @@ -471,7 +440,7 @@ impl StatefulAnalyzer {
let (mut env, sem_pkg) = match result {
Ok(r) => r,
Err(e) => {
return Err(e.error.into());
return Err(e.err_into().map(|(_, x)| x));
}
};

Expand Down Expand Up @@ -550,19 +519,21 @@ pub unsafe extern "C" fn flux_analyze_with(
Some(std::str::from_utf8(CStr::from_ptr(csrc).to_bytes()).unwrap())
};

let sem_pkg = Box::new(match analyzer.analyze(ast_pkg) {
Ok(sem_pkg) => sem_pkg,
match analyzer.analyze(ast_pkg) {
Ok(sem_pkg) => {
*out_sem_pkg = Some(Box::new(sem_pkg));
}
Err(mut err) => {
*out_sem_pkg = err.value.map(Box::new);
if let Some(src) = src {
if let Error::Semantic(err) = &mut err {
if let Error::Semantic(err) = &mut err.error {
err.source = Some(src.into());
}
}
return Some(err.into());
return Some(err.error.into());
}
});
}

*out_sem_pkg = Some(sem_pkg);
None
})
.unwrap_or_else(|err| Some(err.into()))
Expand Down
44 changes: 19 additions & 25 deletions libflux/go/libflux/analyze.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ func Analyze(astPkg *ASTPkg) (*SemanticPkg, error) {
}

func AnalyzeWithOptions(astPkg *ASTPkg, options Options) (*SemanticPkg, error) {
var semPkg *C.struct_flux_semantic_pkg_t
defer func() {
// This is necessary because the ASTPkg returned from the libflux API calls has its finalizer
// set with the Go runtime. But this API will consume the AST package during
Expand All @@ -136,25 +135,13 @@ func AnalyzeWithOptions(astPkg *ASTPkg, options Options) (*SemanticPkg, error) {
cOptions := C.CString(stringOptions)
defer C.free(unsafe.Pointer(cOptions))

// Analyze may return a semantic package even on errors
fluxErr := C.flux_analyze(astPkg.ptr, cOptions, &semPkg)

runtime.KeepAlive(astPkg)

var p *SemanticPkg
if semPkg != nil {
p = &SemanticPkg{ptr: semPkg}
runtime.SetFinalizer(p, free)
}
analyzer, err := NewAnalyzerWithOptions(options)

semPkg, fluxErr := analyzer.Analyze("", astPkg)
if fluxErr != nil {
defer C.flux_free_error(fluxErr)
cstr := C.flux_error_str(fluxErr)
str := C.GoString(cstr)
err = errors.New(codes.Invalid, str)
err = fluxErr.GoError()
}

return p, err
return semPkg, err
}

func AnalyzeString(script string) (*SemanticPkg, error) {
Expand Down Expand Up @@ -233,7 +220,7 @@ func (p *Analyzer) Analyze(src string, astPkg *ASTPkg) (*SemanticPkg, *FluxError
csrc := C.CString(src)
defer C.free(unsafe.Pointer(csrc))

var semPkg *C.struct_flux_semantic_pkg_t
var cSemPkg *C.struct_flux_semantic_pkg_t
defer func() {
// This is necessary because the ASTPkg returned from the libflux API calls has its finalizer
// set with the Go runtime. But this API will consume the AST package during
Expand All @@ -242,16 +229,23 @@ func (p *Analyzer) Analyze(src string, astPkg *ASTPkg) (*SemanticPkg, *FluxError
astPkg.ptr = nil
}()

if err := C.flux_analyze_with(p.ptr, csrc, astPkg.ptr, &semPkg); err != nil {
err := &FluxError{ptr: err}
fluxErr := C.flux_analyze_with(p.ptr, csrc, astPkg.ptr, &cSemPkg)

runtime.KeepAlive(astPkg)

var semPkg *SemanticPkg
if cSemPkg != nil {
semPkg = &SemanticPkg{ptr: cSemPkg}
runtime.SetFinalizer(semPkg, free)
}

var err *FluxError
if fluxErr != nil {
err = &FluxError{ptr: fluxErr}
runtime.SetFinalizer(err, free)
return nil, err
}
runtime.KeepAlive(p)

pkg := &SemanticPkg{ptr: semPkg}
runtime.SetFinalizer(pkg, free)
return pkg, nil
return semPkg, err
}

// Free frees the memory allocated by Rust for the semantic graph.
Expand Down
4 changes: 2 additions & 2 deletions libflux/go/libflux/buildinfo.gen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ var sourceHashes = map[string]string{
"libflux/flux/Cargo.toml": "308c541b31f6ef25094ed4a4c2fddaf38244b0632b93c1f44b2ee4b4209d479c",
"libflux/flux/FLUXDOC.md": "92e6dd8043bd87b4924e09aa28fb5346630aee1214de28ea2c8fc0687cad0785",
"libflux/flux/build.rs": "31dcb1e825555e56b4d959244c4ea630b1d32ccddc1f8615620e0c23552d914f",
"libflux/flux/src/cffi.rs": "de2fc6176cc106b41826f37b2538f8617bc9485eafe098fa2e8008e0ddddb4e3",
"libflux/flux/src/cffi.rs": "27c789e117b7c0d298217444129b5d999edd6e6e6f2c7d51e5f2e698b85712fe",
"libflux/flux/src/lib.rs": "af2f62a02ec08ee476121e035c6587e31c1b6d5d4912ccace1e2e9ae019ad9c1",
"libflux/flux/templates/base.html": "a818747b9621828bb96b94291c60922db54052bbe35d5e354f8e589d2a4ebd02",
"libflux/flux/templates/home.html": "f9927514dd42ca7271b4817ad1ca33ec79c03a77a783581b4dcafabd246ebf3f",
"libflux/flux/templates/package.html": "635e1d638ad1a430f3894ff0a458572ca3c4dc6b9cec2c57fe771443849e1402",
"libflux/flux/templates/value.html": "2af699af2535f83635acbc75c92d2ee941c2335350fc7e82ceb2dccf479360bf",
"libflux/include/influxdata/flux.h": "35f4bfcf252329f6f941c75096488d7b5a902f05a09a41db76649c3632d85fb7",
"libflux/include/influxdata/flux.h": "c97e8a157a7867f5fd14053b45f3668e3ba26df38cae786f7631e8d13ec2eeb8",
"stdlib/array/array.flux": "d0737bb4ee1cea99eb82d1fd6150f5686edd58d1c36a676593e623933dbcc424",
"stdlib/array/array_test.flux": "c4281ef1128ba6164c16f1dcb1b051d2231fe39617f23a070ae1942b5ddb70d5",
"stdlib/array/from_test.flux": "688a3593d27aed7110d016c0b7634808dee533311d4349669a1104bc50a9f3e7",
Expand Down
11 changes: 0 additions & 11 deletions libflux/include/influxdata/flux.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,6 @@ void flux_free_stateful_analyzer(struct flux_stateful_analyzer_t *);
// a semantic graph for that snippet.
struct flux_error_t *flux_analyze_with(struct flux_stateful_analyzer_t *, const char * src, struct flux_ast_pkg_t *, struct flux_semantic_pkg_t **);

// flux_analyze analyzes the given AST and will populate the second pointer argument with
// a pointer to the resulting semantic graph.
// It is the caller's responsibility to free the resulting semantic graph with a call to flux_free_semantic_pkg().
// Note that the caller should free the pointer to the semantic graph, not the pointer to the pointer
// to the semantic graph. It is the former that references memory allocated by Rust.
// If analysis fails, the second pointer argument wil be pointed at 0, and an error will be returned.
// Any non-null error must be freed by calling flux_free_error.
// Regardless of whether an error is returned, this function will consume and free its
// flux_ast_pkg_t* argument, so it should not be reused after calling this function.
struct flux_error_t *flux_analyze(struct flux_ast_pkg_t *, const char * options, struct flux_semantic_pkg_t **);

// Find out the type of a variable referenced in the given Flux AST and return a MonoType flat buffer for it.
// The Flux AST should not contain any definition for the referenced variable.
// A type variable for the designated variable will be automatically injected into the type environment
Expand Down

0 comments on commit 1db3400

Please sign in to comment.