Improve diagnostics for overlapping/conflicting bindings

Closes #38

- Change overlapping bindings case from error to warning (it is *technically* allowed in HLSL/GLSL)

- Make diagnostic messages for these cases include a note to point at the "other" declaration in each case, so that user can more easily isolate the problem

- Unrelated fix: make sure `slangc` sets up its diagnostic callback *before* parsing command-line options so that error messages output during options parsing will be visible

- Unrelated fix: make sure that formatting for diagnostic messages doesn't print diagnostic ID for notes (all have IDs < 0).
  - Note: eventually I'd like to not print diagnostic IDs at all (I think they are cluttering up our output), but doing that requires touching all the test cases...

6 files changed, 47 insertions, 12 deletions

diff --git a/source/slang/diagnostic-defs.h b/source/slang/diagnostic-defs.h
index 17af6cbbb..a748b39bd 100644
--- a/source/slang/diagnostic-defs.h
+++ b/source/slang/diagnostic-defs.h
@@ -39,6 +39,10 @@ DIAGNOSTIC(-1, Note, seePreviousDefinition, "see previous definition")
 DIAGNOSTIC(-1, Note, seePreviousDefinitionOf, "see previous definition of '$0'")
 DIAGNOSTIC(-1, Note, seeRequirementDeclaration, "see requirement declaration")
 DIAGNOSTIC(-1, Note, doYouForgetToMakeComponentAccessible, "do you forget to make component '$0' acessible from '$1' (missing public qualifier)?")
+
+DIAGNOSTIC(-1, Note, seeDeclarationOf, "see declaration of '$0'")
+DIAGNOSTIC(-1, Note, seeOtherDeclarationOf, "see other declaration of '$0'")
+
 //
 // 0xxxx -  Command line and interaction with host platform APIs.
 //
@@ -301,8 +305,8 @@ DIAGNOSTIC(39999, Error, tooManyArguments, "too many arguments to call (got $0,
 DIAGNOSTIC(39999, Error, invalidIntegerLiteralSuffix, "invalid suffix '$0' on integer literal")
 DIAGNOSTIC(39999, Error, invalidFloatingPOintLiteralSuffix, "invalid suffix '$0' on floating-point literal")
 
-DIAGNOSTIC(39999, Error, conflictingExplicitBindingsForParameter, "conflicting explicit bindings for parameter '$0'")
-DIAGNOSTIC(39999, Error, parameterBindingsOverlap, "explicit parameter bindings overlap for parameters '$0' and '$1'")
+DIAGNOSTIC(39999, Error,    conflictingExplicitBindingsForParameter, "conflicting explicit bindings for parameter '$0'")
+DIAGNOSTIC(39999, Warning,  parameterBindingsOverlap, "explicit binding for parameter '$0' overlaps with parameter '$1'")
 
 //
 // 4xxxx - IL code generation.
diff --git a/source/slang/diagnostics.cpp b/source/slang/diagnostics.cpp
index 45adf0ad8..65161db1d 100644
--- a/source/slang/diagnostics.cpp
+++ b/source/slang/diagnostics.cpp
@@ -162,8 +162,13 @@ static void formatDiagnostic(
     sb << humaneLoc.line;
     sb << "): ";
     sb << getSeverityName(diagnostic.severity);
-    sb << " ";
-    sb << diagnostic.ErrorID;
+
+    if( diagnostic.ErrorID >= 0 )
+    {
+        sb << " ";
+        sb << diagnostic.ErrorID;
+    }
+
     sb << ": ";
     sb << diagnostic.Message;
     sb << "\n";
diff --git a/source/slang/parameter-binding.cpp b/source/slang/parameter-binding.cpp
index 757662101..a00520c05 100644
--- a/source/slang/parameter-binding.cpp
+++ b/source/slang/parameter-binding.cpp
@@ -735,6 +735,7 @@ static RefPtr<UsedRangeSet> findUsedRangeSetForTranslationUnit(
 static void addExplicitParameterBinding(
     ParameterBindingContext*    context,
     RefPtr<ParameterInfo>       parameterInfo,
+    VarDeclBase*                varDecl,
     LayoutSemanticInfo const&   semanticInfo,
     UInt                        count,
     RefPtr<UsedRangeSet>        usedRangeSet = nullptr)
@@ -751,8 +752,13 @@ static void addExplicitParameterBinding(
             || bindingInfo.index != semanticInfo.index
             || bindingInfo.space != semanticInfo.space )
         {
+            getSink(context)->diagnose(varDecl, Diagnostics::conflictingExplicitBindingsForParameter, getReflectionName(varDecl));
+
             auto firstVarDecl = parameterInfo->varLayouts[0]->varDecl.getDecl();
-            getSink(context)->diagnose(firstVarDecl, Diagnostics::conflictingExplicitBindingsForParameter, getReflectionName(firstVarDecl));
+            if( firstVarDecl != varDecl )
+            {
+                getSink(context)->diagnose(firstVarDecl, Diagnostics::seeOtherDeclarationOf, getReflectionName(firstVarDecl));
+            }
         }
 
         // TODO(tfoley): `register` semantics can technically be
@@ -781,6 +787,8 @@ static void addExplicitParameterBinding(
             getSink(context)->diagnose(paramA, Diagnostics::parameterBindingsOverlap,
                 getReflectionName(paramA),
                 getReflectionName(paramB));
+
+            getSink(context)->diagnose(paramB, Diagnostics::seeDeclarationOf, getReflectionName(paramB));
         }
     }
 }
@@ -822,7 +830,7 @@ static void addExplicitParameterBindings_HLSL(
             // TODO: warning here!
         }
 
-        addExplicitParameterBinding(context, parameterInfo, semanticInfo, count);
+        addExplicitParameterBinding(context, parameterInfo, varDecl, semanticInfo, count);
     }
 }
 
@@ -888,7 +896,7 @@ static void addExplicitParameterBindings_GLSL(
     auto count = resInfo->count;
     semanticInfo.kind = kind;
 
-    addExplicitParameterBinding(context, parameterInfo, semanticInfo, int(count), usedRangeSet);
+    addExplicitParameterBinding(context, parameterInfo, varDecl, semanticInfo, int(count), usedRangeSet);
 }
 
 // Given a single parameter, collect whatever information we have on
diff --git a/source/slangc/main.cpp b/source/slangc/main.cpp
index 6fb394cce..da670909d 100644
--- a/source/slangc/main.cpp
+++ b/source/slangc/main.cpp
@@ -34,6 +34,11 @@ int MAIN(int argc, char** argv)
     SlangSession* session = spCreateSession(nullptr);
     SlangCompileRequest* compileRequest = spCreateCompileRequest(session);
 
+    spSetDiagnosticCallback(
+        compileRequest,
+        &diagnosticCallback,
+        nullptr);
+
     spSetCommandLineCompilerMode(compileRequest);
 
     char const* appName = "slangc";
@@ -46,11 +51,6 @@ int MAIN(int argc, char** argv)
         exit(1);
     }
 
-    spSetDiagnosticCallback(
-        compileRequest,
-        &diagnosticCallback,
-        nullptr);
-
     // Invoke the compiler
 
 #ifndef _DEBUG
diff --git a/tests/diagnostics/gh-38-fs.hlsl b/tests/diagnostics/gh-38-fs.hlsl
new file mode 100644
index 000000000..61c8381e4
--- /dev/null
+++ b/tests/diagnostics/gh-38-fs.hlsl
@@ -0,0 +1,9 @@
+//TEST_IGNORE_FILE:
+
+// Companion file to `gh-38-fs.hlsl`
+
+Texture2D overlappingB : register(t0);
+
+Texture2D conflicting : register(t2);
+
+float4 main() : SV_Target { return 0; }
diff --git a/tests/diagnostics/gh-38-vs.hlsl b/tests/diagnostics/gh-38-vs.hlsl
new file mode 100644
index 000000000..7b23efdea
--- /dev/null
+++ b/tests/diagnostics/gh-38-vs.hlsl
@@ -0,0 +1,9 @@
+//TEST:SIMPLE: -target dxbc-assembly -profile vs_5_0 -entry main tests/diagnostics/gh-38-fs.hlsl -profile ps_5_0 -entry main
+
+// Ensure that we catch errors with overlapping or conflicting parameter bindings.
+
+Texture2D overlappingA : register(t0);
+
+Texture2D conflicting : register(t1);
+
+float4 main() : SV_Position { return 0; }