From 2c55d3736a3ee933adf684a146135d9086b8b94f Mon Sep 17 00:00:00 2001
From: jsmall-nvidia <jsmall@nvidia.com>
Date: Tue, 21 Apr 2020 17:16:22 -0400
Subject: Fix for a generic definition, followed by a declaration with target
 intrisic causing a crash (#1329)

* * Make a 'definition' if a function has a body or a target intrinsic defined
* Added test for this situation.

* Fix tab.

* Fix single-target-intrisic.slang expected output.

Co-authored-by: Tim Foley <tfoleyNV@users.noreply.github.com>
---
 source/slang/slang-check-decl.cpp                     | 19 ++++++++++++++++++-
 tests/diagnostics/single-target-intrinsic.slang       | 16 ++++++++++++++++
 .../single-target-intrinsic.slang.expected            |  7 +++++++
 3 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 tests/diagnostics/single-target-intrinsic.slang
 create mode 100644 tests/diagnostics/single-target-intrinsic.slang.expected

diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp
index d028ebfe4..d6d115859 100644
--- a/source/slang/slang-check-decl.cpp
+++ b/source/slang/slang-check-decl.cpp
@@ -2382,6 +2382,23 @@ namespace Slang
         return subst;
     }
 
+    // For simplicity we will make having a definition of a function include having a body or a target intrinsics defined.
+    // It may be useful to add other modifiers to mark as having body - for example perhaps
+    // any target intrinsic modifier (like SPIR-V version) should be included.
+    //
+    // Note that not having this check around TargetIntrinsicModifier can lead to a crash in the compiler
+    // with a definition, followed by a declaration with a target intrinsic.
+    // That this doesn't appear to be the case with other modifiers.
+    // TODO: 
+    // We may want to be able to add target intrinsics with other declarations, that being the case this logic
+    // would need to change.
+    // We might also want are more precise error that pointed out the actually problem - because strictly speaking
+    // having a target intrinsic isn't a 'body'.
+    bool _isDefinition(FuncDecl* decl)
+    {
+        return decl->Body || decl->HasModifier<TargetIntrinsicModifier>();
+    }
+
     Result SemanticsVisitor::checkFuncRedeclaration(
         FuncDecl* newDecl,
         FuncDecl* oldDecl)
@@ -2555,7 +2572,7 @@ namespace Slang
         // If both of the declarations have a body, then there
         // is trouble, because we wouldn't know which one to
         // use during code generation.
-        if (newDecl->Body && oldDecl->Body)
+        if (_isDefinition(newDecl) && _isDefinition(oldDecl))
         {
             // Redefinition
             getSink()->diagnose(newDecl, Diagnostics::functionRedefinition, newDecl->getName());
diff --git a/tests/diagnostics/single-target-intrinsic.slang b/tests/diagnostics/single-target-intrinsic.slang
new file mode 100644
index 000000000..e8ff0ed1a
--- /dev/null
+++ b/tests/diagnostics/single-target-intrinsic.slang
@@ -0,0 +1,16 @@
+//DIAGNOSTIC_TEST(windows):SIMPLE:
+
+
+T doThing<T>(T in)
+{
+    return in;
+}
+
+__target_intrinsic(hlsl, "doSomethingIntrinsically($0)")
+T doThing<T>(T in);
+
+void test()
+{
+    int a = 5;
+    int b = doThing(a);
+}
\ No newline at end of file
diff --git a/tests/diagnostics/single-target-intrinsic.slang.expected b/tests/diagnostics/single-target-intrinsic.slang.expected
new file mode 100644
index 000000000..82068f17d
--- /dev/null
+++ b/tests/diagnostics/single-target-intrinsic.slang.expected
@@ -0,0 +1,7 @@
+result code = -1
+standard error = {
+tests/diagnostics/single-target-intrinsic.slang(10): error 30201: function 'doThing' already has a body
+tests/diagnostics/single-target-intrinsic.slang(4): note: see previous definition of 'doThing'
+}
+standard output = {
+}
-- 
cgit v1.2.3