Duplicate diagnostic attribute same location error
Bug:351370529
Change-Id: I3857c4df23d1f61afc2c31c7981af15db297a338
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/200916
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Peter McNeeley <petermcneeley@google.com>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/.gitignore b/.gitignore
index 14ddf65..6d88e40 100644
--- a/.gitignore
+++ b/.gitignore
@@ -156,3 +156,9 @@
### pyenv files
/.python-version
+
+### Clangd cached index files
+/.cache
+
+### The 'compile_commands' file can be generated at root
+compile_commands.json
\ No newline at end of file
diff --git a/src/tint/lang/wgsl/resolver/diagnostic_control_test.cc b/src/tint/lang/wgsl/resolver/diagnostic_control_test.cc
index 7e8f2ae..72c5afd 100644
--- a/src/tint/lang/wgsl/resolver/diagnostic_control_test.cc
+++ b/src/tint/lang/wgsl/resolver/diagnostic_control_test.cc
@@ -362,7 +362,10 @@
DiagnosticAttribute(wgsl::DiagnosticSeverity::kError,
DiagnosticRuleName(Source{{56, 78}}, "chromium", "unreachable_code"));
Func("foo", {}, ty.void_(), {}, Vector{attr1, attr2});
- EXPECT_TRUE(r()->Resolve()) << r()->error();
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ R"(56:78 error: duplicate diagnostic attribute
+12:34 note: first attribute declared here)");
}
TEST_F(ResolverDiagnosticControlTest, Conflict_SameNameDifferentSeverity_Attribute) {
@@ -408,5 +411,19 @@
EXPECT_TRUE(r()->Resolve()) << r()->error();
}
+TEST_F(ResolverDiagnosticControlTest, Conflict_SameNameSameInfoSeverity_Attribute) {
+ auto* attr1 =
+ DiagnosticAttribute(wgsl::DiagnosticSeverity::kInfo,
+ DiagnosticRuleName(Source{{12, 34}}, "chromium", "unreachable_code"));
+ auto* attr2 =
+ DiagnosticAttribute(wgsl::DiagnosticSeverity::kInfo,
+ DiagnosticRuleName(Source{{56, 78}}, "chromium", "unreachable_code"));
+ Func("foo", {}, ty.void_(), {}, Vector{attr1, attr2});
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ R"(56:78 error: duplicate diagnostic attribute
+12:34 note: first attribute declared here)");
+}
+
} // namespace
} // namespace tint::resolver
diff --git a/src/tint/lang/wgsl/resolver/resolver.cc b/src/tint/lang/wgsl/resolver/resolver.cc
index a5e89d8..6945b28 100644
--- a/src/tint/lang/wgsl/resolver/resolver.cc
+++ b/src/tint/lang/wgsl/resolver/resolver.cc
@@ -222,7 +222,8 @@
SetShadows();
- if (!validator_.DiagnosticControls(diagnostic_controls, "directive")) {
+ if (!validator_.DiagnosticControls(diagnostic_controls, "directive",
+ DiagnosticDuplicates::kAllowed)) {
return false;
}
diff --git a/src/tint/lang/wgsl/resolver/validator.cc b/src/tint/lang/wgsl/resolver/validator.cc
index c7fc01f..199cf92 100644
--- a/src/tint/lang/wgsl/resolver/validator.cc
+++ b/src/tint/lang/wgsl/resolver/validator.cc
@@ -2921,11 +2921,12 @@
}
}
}
- return DiagnosticControls(diagnostic_controls, "attribute");
+ return DiagnosticControls(diagnostic_controls, "attribute", DiagnosticDuplicates::kDenied);
}
bool Validator::DiagnosticControls(VectorRef<const ast::DiagnosticControl*> controls,
- const char* use) const {
+ const char* use,
+ DiagnosticDuplicates allow_duplicates) const {
// Make sure that no two diagnostic controls conflict.
// They conflict if the rule name is the same and the severity is different.
Hashmap<std::pair<Symbol, Symbol>, const ast::DiagnosticControl*, 8> diagnostics;
@@ -2934,12 +2935,18 @@
auto name = dc->rule_name->name->symbol;
auto diag_added = diagnostics.Add(std::make_pair(category, name), dc);
- if (!diag_added && diag_added.value->severity != dc->severity) {
- AddError(dc->rule_name->source) << "conflicting diagnostic " << use;
- AddNote(diag_added.value->rule_name->source)
- << "severity of " << style::Code(dc->rule_name->String()) << " set to "
- << style::Code(dc->severity) << " here";
- return false;
+ if (!diag_added) {
+ if (diag_added.value->severity != dc->severity) {
+ AddError(dc->rule_name->source) << "conflicting diagnostic " << use;
+ AddNote(diag_added.value->rule_name->source)
+ << "severity of " << style::Code(dc->rule_name->String()) << " set to "
+ << style::Code(dc->severity) << " here";
+ return false;
+ } else if (allow_duplicates == DiagnosticDuplicates::kDenied) {
+ AddError(dc->rule_name->source) << "duplicate diagnostic " << use;
+ AddNote(diag_added.value->rule_name->source) << "first " << use << " declared here";
+ return false;
+ }
}
}
return true;
diff --git a/src/tint/lang/wgsl/resolver/validator.h b/src/tint/lang/wgsl/resolver/validator.h
index 463f593..4843d3d 100644
--- a/src/tint/lang/wgsl/resolver/validator.h
+++ b/src/tint/lang/wgsl/resolver/validator.h
@@ -28,6 +28,7 @@
#ifndef SRC_TINT_LANG_WGSL_RESOLVER_VALIDATOR_H_
#define SRC_TINT_LANG_WGSL_RESOLVER_VALIDATOR_H_
+#include <cstdint>
#include <set>
#include <string>
#include <utility>
@@ -108,6 +109,14 @@
/// DiagnosticFilterStack is a scoped stack of diagnostic filters.
using DiagnosticFilterStack = ScopeStack<wgsl::DiagnosticRule, wgsl::DiagnosticSeverity>;
+/// Enumerator of duplication behavior for diagnostics.
+enum class DiagnosticDuplicates : uint8_t {
+ // Diagnostic duplicates are allowed.
+ kAllowed,
+ // Diagnostic duplicates are not allowed.
+ kDenied,
+};
+
/// Validation logic for various ast nodes. The validations in general should
/// be shallow and depend on the resolver to call on children. The validations
/// also assume that sem changes have already been made. The validation checks
@@ -553,9 +562,11 @@
/// Validates a set of diagnostic controls.
/// @param controls the diagnostic controls to validate
/// @param use the place where the controls are being used ("directive" or "attribute")
+ /// @param allow_duplicates if same name same severity diagnostics are allowed
/// @returns true on success, false otherwise.
bool DiagnosticControls(VectorRef<const ast::DiagnosticControl*> controls,
- const char* use) const;
+ const char* use,
+ DiagnosticDuplicates allow_duplicates) const;
/// Validates a address space layout
/// @param type the type to validate
diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt
index 43bdd95..37f3233f 100644
--- a/webgpu-cts/expectations.txt
+++ b/webgpu-cts/expectations.txt
@@ -1392,24 +1392,6 @@
crbug.com/351372334 webgpu:shader,validation,expression,call,builtin,insertBits:count_offset:offsetStage="override";countStage="constant" [ Failure ]
crbug.com/351372334 webgpu:shader,validation,expression,call,builtin,insertBits:count_offset:offsetStage="override";countStage="override" [ Failure ]
-# validation of diagnostic duplicate name at same location
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="compound";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="for_body";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="for_stmt";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="function";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="if_else";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="if_stmt";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="if_then";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="loop_body";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="loop_continuing";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="loop_stmt";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="switch_body";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="switch_case";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="switch_default";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="switch_stmt";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="while_body";same_rule=true [ Failure ]
-crbug.com/351370529 webgpu:shader,validation,parse,diagnostic:duplicate_attribute_same_location:loc="while_stmt";same_rule=true [ Failure ]
-
# some missing tint validation errors
crbug.com/350775869 webgpu:shader,validation,expression,call,builtin,ldexp:partial_values:stage="constant";* [ Failure ]
crbug.com/350775869 webgpu:shader,validation,expression,call,builtin,ldexp:partial_values:stage="override";* [ Failure ]
