Skip to content

Nuke DMASTFolder #2390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ike709 wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Nuke DMASTFolder #2390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
243 changes: 0 additions & 243 deletions DMCompiler/DM/Builders/DMASTFolder.cs
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion DMCompiler/DM/Builders/DMExpressionBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ private DMExpression BuildPath(Location location, DreamPath path) {

private DMExpression BuildIdentifier(DMASTIdentifier identifier, DreamPath? inferredPath = null) {
var name = identifier.Identifier;
if (scopeMode == Normal) {
if (scopeMode == Normal || scopeMode == Static) {
Copy link
Member

@wixoaGit wixoaGit Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not valid even in BYOND, local vars/args aren't available to static vars. Making this change will cause the global init proc to attempt to use one of its own locals, which is incorrect.

I think what BYOND is doing here is a first check that the var exists when the expression is created (that is ignoring the static context), and a second check when the bytecode is generated that is being skipped because of constant folding. As far as I can tell, the first step ignoring the static context is a bug. I'm not sure of the best way to replicate this with the way OD is structured.

Copy link
Collaborator Author

@ike709 ike709 Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not know how to fix this.

// ReSharper disable once ConditionalAccessQualifierIsNonNullableAccordingToAPIContract
var localVar = ctx.Proc?.GetLocalVariable(name);
if (localVar is not null) {
Expand Down
5 changes: 1 addition & 4 deletions DMCompiler/DMCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@
result.Append(t.Text);
}

string outputDir = Path.GetDirectoryName(Settings.Files[0]);

Check warning on line 162 in DMCompiler/DMCompiler.cs

View workflow job for this annotation

GitHub Actions / build (ubuntu-latest) 

Converting null literal or possible null value to non-nullable type.
string outputPath = Path.Combine(outputDir, "preprocessor_dump.dm");

File.WriteAllText(outputPath, result.ToString());
Expand All @@ -177,10 +177,7 @@
VerbosePrint("Parsing");
DMASTFile astFile = dmParser.File();

DMASTFolder astSimplifier = new DMASTFolder();
VerbosePrint("Constant folding");
astSimplifier.FoldAst(astFile);

VerbosePrint("Building code tree");
DMCodeTreeBuilder dmCodeTreeBuilder = new(this);
dmCodeTreeBuilder.BuildCodeTree(astFile);

Expand Down
Loading