Skip to content

Commit

Permalink
fix(wgsl-in): yield conflict errors for full diagnostic(…) rule dup…
Browse files Browse the repository at this point in the history
…es in attr. pos.
  • Loading branch information
ErichDonGubler committed Nov 14, 2024
1 parent 693eaaf commit 077984c
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 11 deletions.
19 changes: 18 additions & 1 deletion naga/src/diagnostic_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,18 @@ pub struct DiagnosticFilter {
pub triggering_rule: FilterableTriggeringRule,
}

/// Determines whether [`DiagnosticFilterMap::add`] should consider full duplicates a conflict.
///
/// In WGSL, directive position does not consider this case a conflict, while attribute position
/// does.
#[cfg(feature = "wgsl-in")]
pub(crate) enum ShouldConflictOnFullDuplicate {
/// Use this for attributes in WGSL.
Yes,
/// Use this for directives in WGSL.
No,
}

/// A map of diagnostic filters to their severity and first occurrence's span.
///
/// Intended for front ends' first step into storing parsed [`DiagnosticFilter`]s.
Expand All @@ -104,6 +116,7 @@ impl DiagnosticFilterMap {
&mut self,
diagnostic_filter: DiagnosticFilter,
span: Span,
should_conflict_on_full_duplicate: ShouldConflictOnFullDuplicate,
) -> Result<(), ConflictingDiagnosticRuleError> {
use indexmap::map::Entry;

Expand All @@ -119,7 +132,11 @@ impl DiagnosticFilterMap {
}
Entry::Occupied(entry) => {
let &(first_severity, first_span) = entry.get();
if first_severity != new_severity {
let should_conflict_on_full_duplicate = match should_conflict_on_full_duplicate {
ShouldConflictOnFullDuplicate::Yes => true,
ShouldConflictOnFullDuplicate::No => false,
};
if first_severity != new_severity || should_conflict_on_full_duplicate {
return Err(ConflictingDiagnosticRuleError {
triggering_rule,
triggering_rule_spans: [first_span, span],
Expand Down
15 changes: 8 additions & 7 deletions naga/src/front/wgsl/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1047,13 +1047,14 @@ impl<'a> Error<'a> {
(first_span, "first rule".into()),
(second_span, "second rule".into()),
],
notes: vec![concat!(
"multiple `diagnostic(…)` rules with the same rule name ",
"conflict unless the severity is the same; ",
"delete the rule you don't want, or ",
"ensure that all severities with the same rule name match"
)
.into()],
notes: vec![
concat!(
"Multiple `diagnostic(…)` rules with the same rule name ",
"conflict unless it is a directive and the severity is the same.",
)
.into(),
"You should delete the rule you don't want.".into(),
],
}
}
Error::DiagnosticAttributeNotYetImplementedAtParseSite {
Expand Down
11 changes: 8 additions & 3 deletions naga/src/front/wgsl/parse/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::diagnostic_filter::{
self, DiagnosticFilter, DiagnosticFilterMap, DiagnosticFilterNode, FilterableTriggeringRule,
ShouldConflictOnFullDuplicate,
};
use crate::front::wgsl::error::{Error, ExpectedToken};
use crate::front::wgsl::parse::directive::enable_extension::{
Expand Down Expand Up @@ -2167,7 +2168,7 @@ impl Parser {
if let Some(DirectiveKind::Diagnostic) = DirectiveKind::from_ident(name) {
if let Some(filter) = self.diagnostic_filter(lexer)? {
let span = self.peek_rule_span(lexer);
diagnostic_filters.add(filter, span)?;
diagnostic_filters.add(filter, span, ShouldConflictOnFullDuplicate::Yes)?;
}
} else {
return Err(Error::Unexpected(
Expand Down Expand Up @@ -2369,7 +2370,7 @@ impl Parser {
if let Some(DirectiveKind::Diagnostic) = DirectiveKind::from_ident(name) {
if let Some(filter) = self.diagnostic_filter(lexer)? {
let span = self.peek_rule_span(lexer);
diagnostic_filters.add(filter, span)?;
diagnostic_filters.add(filter, span, ShouldConflictOnFullDuplicate::Yes)?;
}
continue;
}
Expand Down Expand Up @@ -2602,7 +2603,11 @@ impl Parser {
DirectiveKind::Diagnostic => {
if let Some(diagnostic_filter) = self.diagnostic_filter(&mut lexer)? {
let span = self.peek_rule_span(&lexer);
diagnostic_filters.add(diagnostic_filter, span)?;
diagnostic_filters.add(
diagnostic_filter,
span,
ShouldConflictOnFullDuplicate::No,
)?;
}
lexer.expect(Token::Separator(';'))?;
}
Expand Down

0 comments on commit 077984c

Please sign in to comment.