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

Raise error for package referecne before defnition #1170

Merged
Show file tree
Hide file tree
Changes from all commits
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
27 changes: 27 additions & 0 deletions crates/analyzer/src/analyzer_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,21 @@ pub enum AnalyzerError {
error_location: SourceSpan,
},

#[diagnostic(
severity(Error),
code(referring_package_before_definition),
help("change order of package definitions"),
url("")
)]
#[error("pakcakge {identifier} is referred before it is defined.")]
ReferringPackageBeforeDefinition {
identifier: String,
#[source_code]
input: NamedSource<String>,
#[label("Error location")]
error_location: SourceSpan,
},

#[diagnostic(
severity(Error),
code(unresolvable_generic_argument),
Expand Down Expand Up @@ -1569,6 +1584,18 @@ impl AnalyzerError {
}
}

pub fn referring_package_before_definition(
identifier: &str,
source: &str,
token: &TokenRange,
) -> Self {
AnalyzerError::ReferringPackageBeforeDefinition {
identifier: identifier.to_string(),
input: AnalyzerError::named_source(source, token),
error_location: token.into(),
}
}

pub fn unresolvable_generic_argument(
identifier: &str,
source: &str,
Expand Down
28 changes: 27 additions & 1 deletion crates/analyzer/src/handlers/create_reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::type_dag::{self, Context, DagError};
use std::collections::HashMap;
use veryl_parser::resource_table;
use veryl_parser::veryl_grammar_trait::*;
use veryl_parser::veryl_token::{is_anonymous_text, Token, TokenRange};
use veryl_parser::veryl_token::{is_anonymous_text, Token, TokenRange, TokenSource};
use veryl_parser::veryl_walker::{Handler, HandlerPoint};
use veryl_parser::ParolError;

Expand Down Expand Up @@ -107,6 +107,7 @@ impl<'a> CreateReference<'a> {

match symbol_table::resolve((&base_path, namespace)) {
Ok(symbol) => {
self.check_pacakge_reference(&symbol.found, &path.range);
symbol_table::add_reference(symbol.found.id, &path.paths[0].base);

// Check number of arguments
Expand Down Expand Up @@ -181,6 +182,31 @@ impl<'a> CreateReference<'a> {
}
}

fn check_pacakge_reference(&mut self, symbol: &Symbol, token_range: &TokenRange) {
if !matches!(symbol.kind, SymbolKind::Package(_)) {
return;
}

let base_token = token_range.end;
let package_token = symbol.token;
if let (TokenSource::File(package_file), TokenSource::File(base_file)) =
(package_token.source, base_token.source)
{
let referecne_before_definition = package_file == base_file
&& (package_token.line > base_token.line
|| package_token.line == base_token.line
&& package_token.column > base_token.column);
if referecne_before_definition {
self.errors
.push(AnalyzerError::referring_package_before_definition(
&package_token.to_string(),
self.text,
token_range,
));
}
}
}

fn insert_declaration_dag_node(&mut self, symbol: &Symbol) -> Option<u32> {
if let Some(child) = self.insert_dag_node(symbol) {
if let Some(parent) = self.dag_scope_parent.last().cloned() {
Expand Down
61 changes: 61 additions & 0 deletions crates/analyzer/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1758,6 +1758,67 @@ fn undefined_identifier() {
));
}

#[test]
fn referring_package_before_definition() {
let code = r#"
module ModuleA {
const A: u32 = PakcageB::B;
}
package PakcageB {
const B: u32 = 0;
}
"#;

let errors = analyze(code);
assert!(matches!(
errors[0],
AnalyzerError::ReferringPackageBeforeDefinition { .. }
));

let code = r#"
interface InterfaceA {
const A: u32 = PakcageB::B;
}
package PakcageB {
const B: u32 = 0;
}
"#;

let errors = analyze(code);
assert!(matches!(
errors[0],
AnalyzerError::ReferringPackageBeforeDefinition { .. }
));

let code = r#"
package PackageA {
const A: u32 = PakcageB::B;
}
package PakcageB {
const B: u32 = 0;
}
"#;

let errors = analyze(code);
assert!(matches!(
errors[0],
AnalyzerError::ReferringPackageBeforeDefinition { .. }
));

let code = r#"
import PakcageB::B;
package PakcageB {
const B: u32 = 0;
}
"#;

let errors = analyze(code);
assert!(matches!(
errors[0],
AnalyzerError::ReferringPackageBeforeDefinition { .. }
));
}

#[test]
fn unknown_attribute() {
let code = r#"
Expand Down
2 changes: 1 addition & 1 deletion testcases/map/testcases/sv/19_import_export.sv.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion testcases/map/testcases/sv/37_package_ref.sv.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion testcases/map/testcases/sv/44_import_resolve.sv.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion testcases/map/testcases/sv/56_generic_interface.sv.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion testcases/map/testcases/sv/57_generic_package.sv.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion testcases/map/testcases/sv/58_generic_struct.sv.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions testcases/sv/19_import_export.sv
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@



package veryl_testcase_PackageA;
import PackageA::A;
import PackageA::*;
localparam int unsigned A = 0;
endpackage

module veryl_testcase_Module19
import PackageA::A;
import PackageA::*;
Expand All @@ -25,10 +31,4 @@ package veryl_testcase_Package19;
export A;
export *::*;
endpackage

package veryl_testcase_PackageA;
import PackageA::A;
import PackageA::*;
localparam int unsigned A = 0;
endpackage
//# sourceMappingURL=../map/testcases/sv/19_import_export.sv.map
17 changes: 9 additions & 8 deletions testcases/sv/37_package_ref.sv
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
module veryl_testcase_Module37;
int unsigned _a;
always_comb _a = veryl_testcase_Package37::A;
int unsigned _b;
always_comb _b = veryl_testcase_Package37::B_C;
int unsigned _c;
always_comb _c = veryl_testcase_Package37::X();
endmodule
package veryl_testcase_Package37;
localparam int unsigned A = 1;

Expand All @@ -17,4 +9,13 @@ package veryl_testcase_Package37;
return 0;
endfunction
endpackage

module veryl_testcase_Module37;
int unsigned _a;
always_comb _a = veryl_testcase_Package37::A;
int unsigned _b;
always_comb _b = veryl_testcase_Package37::B_C;
int unsigned _c;
always_comb _c = veryl_testcase_Package37::X();
endmodule
//# sourceMappingURL=../map/testcases/sv/37_package_ref.sv.map
16 changes: 8 additions & 8 deletions testcases/sv/44_import_resolve.sv
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
package veryl_testcase_Package44A;
localparam int unsigned z = 0;
endpackage

package veryl_testcase_Package44B;
localparam int unsigned y = 0;
endpackage

module veryl_testcase_Module44;
logic [10-1:0] a;
logic [10-1:0] b;
Expand All @@ -10,12 +18,4 @@ module veryl_testcase_Module44;
always_comb b = z;
always_comb c = y;
endmodule

package veryl_testcase_Package44A;
localparam int unsigned z = 0;
endpackage

package veryl_testcase_Package44B;
localparam int unsigned y = 0;
endpackage
//# sourceMappingURL=../map/testcases/sv/44_import_resolve.sv.map
Loading
Loading