Skip to content

Commit

Permalink
feat(diagnostics): refactor the diagnostics to be more consistant (PL…
Browse files Browse the repository at this point in the history
…C-lang#1063)

* feat(PLC-lang#826): refactor the diagnostics to be more consistant

Diagnostics are now created using builder methods, and are no longer
enums. A severity field now indicates the default severity of a
diagnostic. This can be overridden in a diagnostian. This is yet to be
implemented.
We now use Anyhow to report some errors to simplify conversions between
our Diagnostic type and some rust types.

* feat: rework the errors and diagnostics

Errors / Diagnostics are now of a single type and can be constructed
using a builder pattern.
The error codes are now a string value instead of an enum, each error
can now have a description.
At a later stage the descriptions would be accessible from the command
line and the book, for now they do nothing.
This now allows the option make erros configurable using the
Diagnostician so that the user can ignore certain warnings or upgrade a
warning to an error.
With this commit all erros will now break compilation. This has not yet
been tested with existing code but I assume in the current state we will
have issues. We should test this with oscat and internal projects before
commiting to decide if the severity we set as default is OK.

* Convert pointer error to warning, fix a false positive

* fix: array bounds are dint not int

The location problems in the validation snap are still open.

* Make sure snapshots are still valid

The snapshots were re-applied after we switched them to buffered
diagnostics. Some issues were fixed as a result
  • Loading branch information
ghaith authored Jan 30, 2024
1 parent baea087 commit 0ae4811
Show file tree
Hide file tree
Showing 314 changed files with 2,673 additions and 2,513 deletions.
4 changes: 3 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
}
},
"args": [
"target/demo.st"
"--check",
"target/demo.st",
"-i","libs/stdlib/iec61131-st/timers.st"
],
"cwd": "${workspaceFolder}"
},
Expand Down
5 changes: 5 additions & 0 deletions Cargo.lock

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

5 changes: 4 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ regex = "1"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1"
toml = "0.5"
lazy_static = "1.4.0"
shell-words = "1.1.0"
plc_derive = { path = "./compiler/plc_derive" }
lld_rs = "140.0.0"
Expand All @@ -40,6 +39,8 @@ log.workspace = true
inkwell.workspace = true
chrono.workspace = true
itertools.workspace = true
anyhow.workspace = true
lazy_static.workspace = true

[dev-dependencies]
num = "0.4"
Expand Down Expand Up @@ -81,3 +82,5 @@ encoding_rs_io = "0.1"
log = "0.4"
chrono = { version = "0.4", default-features = false }
itertools = "0.11"
anyhow = "1.0"
lazy_static = "1.4.0"
39 changes: 24 additions & 15 deletions compiler/plc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,30 @@ impl TypeNature {
}
}

impl Display for TypeNature {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
let name = match self {
TypeNature::Any => "ANY",
TypeNature::Derived => "ANY_DERIVED",
TypeNature::Elementary => "ANY_ELEMENTARY",
TypeNature::Magnitude => "ANY_MAGNITUDE",
TypeNature::Num => "ANY_NUMBER",
TypeNature::Real => "ANY_REAL",
TypeNature::Int => "ANY_INT",
TypeNature::Signed => "ANY_SIGNED",
TypeNature::Unsigned => "ANY_UNSIGNED",
TypeNature::Duration => "ANY_DURATION",
TypeNature::Bit => "ANY_BIT",
TypeNature::Chars => "ANY_CHARS",
TypeNature::String => "ANY_STRING",
TypeNature::Char => "ANY_CHAR",
TypeNature::Date => "ANY_DATE",
TypeNature::__VLA => "__ANY_VLA",
};
write!(f, "{name}")
}
}

impl DirectAccessType {
/// Returns the size of the bitaccess result
pub fn get_bit_width(&self) -> u64 {
Expand Down Expand Up @@ -379,21 +403,6 @@ impl Variable {
}
}

pub trait DiagnosticInfo {
fn get_description(&self) -> String;
fn get_location(&self) -> SourceLocation;
}

impl DiagnosticInfo for AstNode {
fn get_description(&self) -> String {
format!("{self:?}")
}

fn get_location(&self) -> SourceLocation {
self.get_location()
}
}

#[derive(Clone, PartialEq)]
pub enum DataTypeDeclaration {
DataTypeReference { referenced_type: String, location: SourceLocation },
Expand Down
2 changes: 2 additions & 0 deletions compiler/plc_diagnostics/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ codespan-reporting = "0.11.1"
plc_ast = { path = "../plc_ast" }
plc_source = { path = "../plc_source" }
serde_json = "1"
anyhow.workspace = true
lazy_static.workspace = true
34 changes: 7 additions & 27 deletions compiler/plc_diagnostics/src/diagnostician.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use std::collections::HashMap;

use crate::{
diagnostics::Diagnostic,
errno::ErrNo,
diagnostics::{Diagnostic, Severity},
reporter::{
clang::ClangFormatDiagnosticReporter, codespan::CodeSpanDiagnosticReporter,
null::NullDiagnosticReporter, DiagnosticReporter, ResolvedDiagnostics, ResolvedLocation,
Expand Down Expand Up @@ -35,13 +34,10 @@ impl Diagnostician {
pub fn handle(&mut self, diagnostics: &[Diagnostic]) -> Severity {
let resolved_diagnostics = diagnostics
.iter()
.flat_map(|it| match it {
Diagnostic::CombinedDiagnostic { inner_diagnostics, .. } => {
let mut res = vec![it];
res.extend(inner_diagnostics.iter().collect::<Vec<&Diagnostic>>());
res
}
_ => vec![it],
.flat_map(|it| {
let mut res = vec![it];
res.extend(it.get_sub_diagnostics());
res
})
.map(|d| ResolvedDiagnostics {
message: d.get_message().to_string(),
Expand Down Expand Up @@ -148,30 +144,14 @@ pub struct DefaultDiagnosticAssessor;

impl DiagnosticAssessor for DefaultDiagnosticAssessor {
fn assess(&self, d: &Diagnostic) -> Severity {
match d {
// improvements become warnings
Diagnostic::ImprovementSuggestion { .. } => Severity::Warning,
_ if *d.get_type() == ErrNo::reference__unresolved => Severity::Critical,
// everything else becomes an error
_ => Severity::Error,
}
//TODO: Refer to some severity map to reassign severity here.
d.get_severity()
}
}

/// a diagnostics severity
#[derive(Default, Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub enum Severity {
#[default]
Info,
Warning,
Error,
Critical,
}

impl std::fmt::Display for Severity {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let severity = match self {
Severity::Critical => "critical",
Severity::Error => "error",
Severity::Warning => "warning",
Severity::Info => "info",
Expand Down
Loading

0 comments on commit 0ae4811

Please sign in to comment.