From d046082c501d3501a24c143dff2cfa42939549b9 Mon Sep 17 00:00:00 2001
From: Yong He <yonghe@outlook.com>
Date: Fri, 17 Jan 2025 14:52:28 -0800
Subject: Add diagnostic for function body followed by a `;`;. (#6122)

---
 source/slang/slang-diagnostic-defs.h             |  6 +++-
 source/slang/slang-parser.cpp                    | 17 ++++++++++-
 tests/language-feature/empty-struct-method.slang | 37 ++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100644 tests/language-feature/empty-struct-method.slang

diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
index 7eddc16a9..915ed9b42 100644
--- a/source/slang/slang-diagnostic-defs.h
+++ b/source/slang/slang-diagnostic-defs.h
@@ -487,7 +487,11 @@ DIAGNOSTIC(
     Warning,
     unintendedEmptyStatement,
     "potentially unintended empty statement at this location; use {} instead.")
-
+DIAGNOSTIC(
+    20102,
+    Error,
+    unexpectedBodyAfterSemicolon,
+    "unexpected function body after signature declaration, is this ';' a typo?")
 DIAGNOSTIC(30102, Error, declNotAllowed, "$0 is not allowed here.")
 
 // 29xxx - Snippet parsing and inline asm
diff --git a/source/slang/slang-parser.cpp b/source/slang/slang-parser.cpp
index 98cfd121c..81619b700 100644
--- a/source/slang/slang-parser.cpp
+++ b/source/slang/slang-parser.cpp
@@ -1802,9 +1802,17 @@ public:
 /// Parse an optional body statement for a declaration that can have a body.
 static Stmt* parseOptBody(Parser* parser)
 {
-    if (AdvanceIf(parser, TokenType::Semicolon))
+    Token semiColonToken;
+    if (AdvanceIf(parser, TokenType::Semicolon, &semiColonToken))
     {
         // empty body
+        // if we see a `{` after a `;`, it is very likely an user error to
+        // have the `;`, so we will provide a better diagnostic for it.
+        if (peekTokenType(parser) == TokenType::LBrace)
+        {
+            parser->sink->diagnose(semiColonToken.loc, Diagnostics::unexpectedBodyAfterSemicolon);
+            return parser->parseBlockStatement();
+        }
         return nullptr;
     }
     else
@@ -4841,6 +4849,13 @@ static DeclBase* ParseDeclWithModifiers(
             // We shouldn't be seeing an LBrace or an LParent when expecting a decl.
             // However recovery logic may lead us here. In this case we just
             // skip the whole `{}` block and return an empty decl.
+            if (!parser->isRecovering)
+            {
+                parser->sink->diagnose(
+                    loc,
+                    Diagnostics::unexpectedToken,
+                    parser->tokenReader.peekToken());
+            }
             SkipBalancedToken(&parser->tokenReader);
             decl = parser->astBuilder->create<EmptyDecl>();
             decl->loc = loc;
diff --git a/tests/language-feature/empty-struct-method.slang b/tests/language-feature/empty-struct-method.slang
new file mode 100644
index 000000000..2467013ac
--- /dev/null
+++ b/tests/language-feature/empty-struct-method.slang
@@ -0,0 +1,37 @@
+//DIAGNOSTIC_TEST:SIMPLE(filecheck=CHECK):-target spirv
+
+struct Light
+{
+    float3 position;
+    float radius;
+
+    float3 color;
+    float intensity;
+};
+
+[vk::binding(0, 0)]
+StructuredBuffer<Light> globalLightList;
+
+struct Lighting
+{
+    //CHECK: ([[# @LINE+1]]): error 20102
+    float3 DoLighting(Light light);
+    {
+        // Not emitted
+        return float3(1.0, 1.0, 1.0);
+    }
+};
+
+[shader("fragment")]
+float4 fragment(float4 color: COLOR0)
+{
+    float4 albedo = color;
+
+    if (albedo.a < 0.025)
+        discard;
+
+    Lighting light = Lighting();
+    albedo.xyz = light.DoLighting(globalLightList[0]);
+
+    return albedo;
+}
\ No newline at end of file
-- 
cgit v1.2.3