Skip to content

Commit

Permalink
Merge pull request #1170 from taichi-ishitani/raise_error_for_referri…
Browse files Browse the repository at this point in the history
…ng_package_before_definition

Raise error for package referecne before defnition
  • Loading branch information
dalance authored Dec 28, 2024
2 parents d50cbfe + 809dce5 commit f2e4226
Show file tree
Hide file tree
Showing 21 changed files with 214 additions and 98 deletions.
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

0 comments on commit f2e4226

Please sign in to comment.