From 980fc63b74e3e0ec95b7ea17fa7b6ce17f73f561 Mon Sep 17 00:00:00 2001
From: Brian Fiete <bfiete@gmail.com>
Date: Fri, 25 Sep 2020 05:36:58 -0700
Subject: [PATCH] Additional `if` variable restrictions

---
 IDEHelper/Compiler/BfAst.h             |  5 +++
 IDEHelper/Compiler/BfExprEvaluator.cpp | 57 ++++++++++++--------------
 2 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/IDEHelper/Compiler/BfAst.h b/IDEHelper/Compiler/BfAst.h
index 02c460db..945df44f 100644
--- a/IDEHelper/Compiler/BfAst.h
+++ b/IDEHelper/Compiler/BfAst.h
@@ -1409,6 +1409,11 @@ public:
 	{
 		return (srcPos >= mSrcStart + startAdd) && (srcPos < mSrcEnd + lenAdd);
 	}
+
+	bool Contains(BfAstNode* node)
+	{
+		return (node->mSrcStart >= mSrcStart) && (node->mSrcEnd <= mSrcEnd);
+	}
 #endif
 
 
diff --git a/IDEHelper/Compiler/BfExprEvaluator.cpp b/IDEHelper/Compiler/BfExprEvaluator.cpp
index f82321b2..aca7ad57 100644
--- a/IDEHelper/Compiler/BfExprEvaluator.cpp
+++ b/IDEHelper/Compiler/BfExprEvaluator.cpp
@@ -2702,6 +2702,7 @@ void BfExprEvaluator::Visit(BfBlock* blockExpr)
 bool BfExprEvaluator::CheckVariableDeclaration(BfAstNode* checkNode, bool requireSimpleIfExpr, bool exprMustBeTrue, bool silentFail)
 {
 	BfAstNode* checkChild = checkNode;
+	bool childWasAndRHS = false;
 	bool foundIf = false;
 
 	auto parentNodeEntry = mModule->mParentNodeEntry;
@@ -2712,7 +2713,20 @@ bool BfExprEvaluator::CheckVariableDeclaration(BfAstNode* checkNode, bool requir
 			checkChild = parentNodeEntry->mNode;
 			parentNodeEntry = parentNodeEntry->mPrev;
 		}
-	}
+	}	
+
+	auto _Fail = [&](const StringImpl& errorStr, BfAstNode* node)
+	{
+		if (!silentFail)
+		{
+			auto error = mModule->Fail(errorStr, node);
+			if ((error != NULL) && (node != checkNode))
+			{
+				mModule->mCompiler->mPassInstance->MoreInfo("See variable declaration", checkNode);
+			}
+		}
+		return false;
+	};
 
 	while (parentNodeEntry != NULL)
 	{		
@@ -2723,41 +2737,20 @@ bool BfExprEvaluator::CheckVariableDeclaration(BfAstNode* checkNode, bool requir
 			if (binOpExpr->mOp == BfBinaryOp_ConditionalAnd)
 			{
 				// This is always okay
+				childWasAndRHS = (binOpExpr->mRight != NULL) && (binOpExpr->mRight->Contains(checkChild));
 			}
 			else if ((binOpExpr->mOp == BfBinaryOp_ConditionalOr) && (!exprMustBeTrue))
 			{
-				bool matches = false;
-				auto checkRight = binOpExpr->mRight;
-				while (checkRight != NULL)
+				if ((binOpExpr->mRight != NULL) & (binOpExpr->mRight->Contains(checkChild)))				
 				{
-					if (checkRight == checkChild)
-					{
-						matches = true;
-						break;
-					}
-
-					if (auto parenExpr = BfNodeDynCast<BfParenthesizedExpression>(checkRight))
-						checkRight = parenExpr->mExpression;
-					else
-					{
-						break;
-					}
-				}
-
-				if (matches)
-				{
-					if (!silentFail)
-						mModule->Fail("Conditional short-circuiting may skip variable initialization", binOpExpr->mOpToken);
-					return false;
+					return _Fail("Conditional short-circuiting may skip variable initialization", binOpExpr->mOpToken);					
 				}
 			}
 			else
 			{
 				if (exprMustBeTrue)
 				{
-					if (!silentFail)
-						mModule->Fail("Operator cannot be used with variable initialization", binOpExpr->mOpToken);
-					return false;
+					return _Fail("Operator cannot be used with variable initialization", binOpExpr->mOpToken);					
 				}
 			}
 		}
@@ -2769,10 +2762,14 @@ bool BfExprEvaluator::CheckVariableDeclaration(BfAstNode* checkNode, bool requir
 		{			
 			if (exprMustBeTrue)
 			{
-				if (!silentFail)
-					mModule->Fail("Operator cannot be used with variable initialization", unaryOp->mOpToken);
+				return _Fail("Operator cannot be used with variable initialization", unaryOp->mOpToken);
 				return false;
 			}
+
+			if (childWasAndRHS)
+			{		
+				return _Fail("Operator may allow conditional short-circuiting to skip variable initialization", unaryOp->mOpToken);				
+			}
 		}
 		else if (auto ifStmt = BfNodeDynCast<BfIfStatement>(checkParent))
 		{
@@ -2784,9 +2781,7 @@ bool BfExprEvaluator::CheckVariableDeclaration(BfAstNode* checkNode, bool requir
 		{
 			if (requireSimpleIfExpr)
 			{
-				if (!silentFail)
-					mModule->Fail("Variable declaration expression can only be contained in simple 'if' expressions", checkNode);
-				return false;
+				return _Fail("Variable declaration expression can only be contained in simple 'if' expressions", checkNode);				
 			}
 			break;
 		}