Commit 8c37a1a1 authored by Jesse McGrew's avatar Jesse McGrew
Browse files

Added compiler warnings for unused object flags (ZIL0211) and properties (ZIL0212).

These are likely to be typos, or dead code at best. [ZILF-157]
parent 57c7247411a3
......@@ -99,6 +99,8 @@ namespace Zilf.Compiler
BuildUserDefinedTables();
BuildHeaderExtensionTable();
WarnAboutUnusedGlobals();
Game.Finish();
}
......
......@@ -142,7 +142,9 @@ namespace Zilf.Compiler
}
// routine calls
var obj = Context.GetZVal(Context.ZEnvironment.InternGlobalName(head));
ZilAtom internedHead = Context.ZEnvironment.InternGlobalName(head);
MarkGlobalAsRead(internedHead);
var obj = Context.GetZVal(internedHead);
while (obj is ZilConstant cnst)
obj = cnst.Value;
......@@ -219,6 +221,7 @@ namespace Zilf.Compiler
src,
CompilerMessages.Bare_Atom_0_Interpreted_As_Global_Variable_Index,
atom));
MarkGlobalAsRead(atom);
return Globals[atom].Indirect;
}
if (SoftGlobals.ContainsKey(atom))
......
......@@ -288,17 +288,27 @@ namespace Zilf.Compiler
if (atom.StdAtom == StdAtom.T)
return Game.One;
if (Routines.TryGetValue(atom, out var routine))
{
MarkGlobalAsRead(atom);
return routine;
}
if (Objects.TryGetValue(atom, out var obj))
{
MarkGlobalAsRead(atom);
return obj;
}
if (Constants.TryGetValue(atom, out var operand))
{
MarkGlobalAsRead(atom);
return operand;
}
if (mode == AmbiguousConstantMode.Optimistic && Globals.TryGetValue(atom, out var global))
{
Context.HandleError(new CompilerError((ISourceLine?)null,
CompilerMessages.Bare_Atom_0_Interpreted_As_Global_Variable_Index,
atom));
MarkGlobalAsRead(atom);
return global;
}
......@@ -316,6 +326,7 @@ namespace Zilf.Compiler
return tb;
case ZilConstant constant:
MarkGlobalAsRead(constant.Name);
return CompileConstant(constant.Value);
case ZilForm form:
......@@ -331,7 +342,10 @@ namespace Zilf.Compiler
case ZilHash hash when hash.StdTypeAtom == StdAtom.VOC && hash.GetPrimitive(Context) is ZilAtom primAtom:
var wordAtom = ZilAtom.Parse("W?" + primAtom.Text, Context);
if (Constants.TryGetValue(wordAtom, out operand))
{
MarkGlobalAsRead(wordAtom);
return operand;
}
return null;
......@@ -344,5 +358,76 @@ namespace Zilf.Compiler
}
}
}
private void MarkGlobalAsRead(ZilAtom name)
{
ReadAccessedGlobalNames.Add(name);
}
private void WarnAboutUnusedGlobals()
{
var alreadyWarned = new HashSet<ZilAtom>();
SuppressImplicitlyUsedNames();
Check(
CompilerMessages.Flag_0_Is_Defined_But_Never_Used,
from flagName in Flags.Keys
select (flagName, flagName, FlagDefinitions.GetValueOrDefault(flagName)));
Check(
CompilerMessages.Property_0_Is_Defined_But_Never_Used,
from propName in Properties.Keys
let propConst = ZilAtom.Parse("P?" + propName.Text, Context)
select (propConst, propName, PropertyDefinitions.GetValueOrDefault(propName)));
void SuppressImplicitlyUsedNames()
{
// TODO(ZILF-239): this works around the bug where PROPDEF DIRECTIONS incorrectly defines a DIRECTIONS property
alreadyWarned.Add(Context.GetStdAtom(StdAtom.DIRECTIONS));
// direction properties are implicitly used (written into the vocab table)
foreach (var dir in Context.ZEnvironment.Directions)
{
var dirPropConst = ZilAtom.Parse("P?" + dir.Text, Context);
alreadyWarned.Add(dirPropConst);
}
// FIND flags from syntax are implicitly used (written into the syntax table)
foreach (var syn in Context.ZEnvironment.Syntaxes)
{
if (syn.FindFlag1 != null)
alreadyWarned.Add(syn.FindFlag1);
if (syn.FindFlag2 != null)
alreadyWarned.Add(syn.FindFlag2);
}
/* If we warn about unused globals and routines in the future, this should be extended
* to suppress:
* GO
* ACTIONS
* PREACTIONS
* PREPOSITIONS
* VERBS
*
* ...and, for V3 only, the first 3 global variables defined.
*/
}
void Check(int diagnosticCode, IEnumerable<(ZilAtom globalAtom, ZilAtom displayAtom, ISourceLine? src)> names)
{
foreach (var (globalAtom, displayAtom, src) in names.OrderBy(n => n.displayAtom.Text))
{
if (!ReadAccessedGlobalNames.Contains(globalAtom) && alreadyWarned.Add(globalAtom))
{
Context.HandleError(new CompilerError(
src,
diagnosticCode,
displayAtom));
}
}
}
}
}
}
......@@ -289,35 +289,41 @@ namespace Zilf.Compiler
}
}
if (!phony && !Properties.ContainsKey(atom))
if (!phony)
{
if (isSynonym == null)
{
synonym = Context.ZEnvironment.Synonyms.FirstOrDefault(s => s.SynonymWord.Atom == atom);
isSynonym = (synonym != null);
}
PropertyDefinitions.TryAdd(atom, prop.SourceLine);
if (isSynonym.Value)
if (!Properties.ContainsKey(atom))
{
Debug.Assert(synonym != null);
var origAtom = synonym.OriginalWord.Atom;
if (!Properties.TryGetValue(origAtom, out var origPb))
if (isSynonym == null)
{
DefineProperty(origAtom);
origPb = Properties[origAtom];
synonym = Context.ZEnvironment.Synonyms.FirstOrDefault(s => s.SynonymWord.Atom == atom);
isSynonym = (synonym != null);
}
Properties.Add(atom, origPb);
var pAtom = ZilAtom.Parse("P?" + atom.Text, Context);
Constants.Add(pAtom, origPb);
if (isSynonym.Value)
{
Debug.Assert(synonym != null);
var origSpec = Context.GetProp(origAtom, Context.GetStdAtom(StdAtom.PROPSPEC));
Context.PutProp(atom, Context.GetStdAtom(StdAtom.PROPSPEC), origSpec);
}
else
{
DefineProperty(atom);
var origAtom = synonym.OriginalWord.Atom;
PropertyDefinitions.TryAdd(origAtom, prop.SourceLine);
if (!Properties.TryGetValue(origAtom, out var origPb))
{
DefineProperty(origAtom);
origPb = Properties[origAtom];
}
Properties.Add(atom, origPb);
var pAtom = ZilAtom.Parse("P?" + atom.Text, Context);
Constants.Add(pAtom, origPb);
var origSpec = Context.GetProp(origAtom, Context.GetStdAtom(StdAtom.PROPSPEC));
Context.PutProp(atom, Context.GetStdAtom(StdAtom.PROPSPEC), origSpec);
}
else
{
DefineProperty(atom);
}
}
}
......@@ -407,6 +413,7 @@ namespace Zilf.Compiler
DefineFlag(Context.ZEnvironment.TryGetBitSynonym(word, out var original)
? original
: word);
FlagDefinitions.TryAdd(word, prop.SourceLine);
}
catch (ZilError ex)
{
......
......@@ -20,6 +20,7 @@ using System.Collections.Generic;
using Zilf.Emit;
using Zilf.Interpreter;
using Zilf.Interpreter.Values;
using Zilf.Language;
using Zilf.ZModel;
using Zilf.ZModel.Values;
using Zilf.ZModel.Vocab;
......@@ -55,7 +56,9 @@ namespace Zilf.Compiler
Routines = new Dictionary<ZilAtom, IRoutineBuilder>(equalizer);
Objects = new Dictionary<ZilAtom, IObjectBuilder>(equalizer);
Properties = new Dictionary<ZilAtom, IPropertyBuilder>(equalizer);
PropertyDefinitions = new Dictionary<ZilAtom, ISourceLine?>(equalizer);
Flags = new Dictionary<ZilAtom, IFlagBuilder>(equalizer);
FlagDefinitions = new Dictionary<ZilAtom, ISourceLine?>(equalizer);
SoftGlobals = new Dictionary<ZilAtom, SoftGlobal>(equalizer);
}
......@@ -74,12 +77,22 @@ namespace Zilf.Compiler
public readonly Dictionary<ZilAtom, IObjectBuilder> Objects;
public readonly Dictionary<ZilTable, ITableBuilder> Tables = new();
public readonly Dictionary<IWord, IWordBuilder> Vocabulary = new();
public readonly Dictionary<ZilAtom, IPropertyBuilder> Properties;
public readonly Dictionary<ZilAtom, ISourceLine?> PropertyDefinitions;
public readonly Dictionary<ZilAtom, IFlagBuilder> Flags;
public readonly Dictionary<ZilAtom, ISourceLine?> FlagDefinitions;
public readonly Dictionary<ZilAtom, SoftGlobal> SoftGlobals;
public IOperand? SoftGlobalsTable;
public int UniqueFlags { get; set; }
/// <summary>
/// A set of atoms identifying the global values (constants, globals, routines, objects, flags, etc.) that are "read",
/// i.e. used anywhere in the story file as an operand or initializer.
/// </summary>
public readonly HashSet<ZilAtom> ReadAccessedGlobalNames = new();
}
}
......@@ -121,6 +121,10 @@ namespace Zilf.Diagnostics
public const int Mentioned_Object_0_Is_Defined_Elsewhere_As_A_1 = 209;
[Warning("local variable '{0}' is never used", Noisy = true)]
public const int Local_Variable_0_Is_Never_Used = 210;
[Warning("flag '{0}' is defined but never used", Noisy = true)]
public const int Flag_0_Is_Defined_But_Never_Used = 211;
[Warning("property '{0}' is defined but never used", Noisy = true)]
public const int Property_0_Is_Defined_But_Never_Used = 212;
// Z-machine Structures - 0300
......
......@@ -558,8 +558,7 @@ namespace Zilf.Tests.Integration
{
await AssertGlobals(
@"<ROOM WEST-SIDE-OF-FISSURE
(DESC ""West Side of Fissure"")
(ACTION WEST-SIDE-OF-FISSURE-F)>",
(DESC ""West Side of Fissure"")>",
@"<ROUTINE WEST-SIDE-OF-FISSURE-F (RARG) <>>",
@"<OBJECT DIAMONDS (DESC ""diamonds"") (IN WEST-SIDE-OF-FISSURE-F)>")
.WithoutWarnings()
......@@ -574,5 +573,57 @@ namespace Zilf.Tests.Integration
.WithGlobal("<OBJECT FOO (DESC \"first\nsecond\r\nthird\")>")
.OutputsAsync("first second third");
}
[TestMethod]
public async Task Unused_Flags_Should_Warn()
{
// only referenced in one object definition - warning
await AssertGlobals("<OBJECT FOO (FLAGS MYBIT)>")
.WithWarnings("ZIL0211")
.CompilesAsync();
// referenced in two object definitions - warning
await AssertGlobals(
"<OBJECT FOO (FLAGS MYBIT)>",
"<OBJECT BAR (FLAGS MYBIT)>")
.WithWarnings("ZIL0211")
.CompilesAsync();
// referenced in a routine - no warning
await AssertRoutine("", "<FCLEAR ,FOO ,MYBIT>")
.WithGlobal("<OBJECT FOO (FLAGS MYBIT)>")
.WithoutWarnings()
.CompilesAsync();
// referenced in syntax - no warning
await AssertGlobals(
"<OBJECT FOO (FLAGS MYBIT MYBIT2)>",
"<SYNTAX BLAH OBJECT (FIND MYBIT) WITH OBJECT (FIND MYBIT2) = V-BLAH>",
"<ROUTINE V-BLAH () <>>")
.WithoutWarnings()
.CompilesAsync();
}
[TestMethod]
public async Task Unused_Properties_Should_Warn()
{
// only referenced in one object definition - warning
await AssertGlobals("<OBJECT FOO (MYPROP 123)>")
.WithWarnings("ZIL0212")
.CompilesAsync();
// referenced in two object definitions - warning
await AssertGlobals(
"<OBJECT FOO (MYPROP 123)>",
"<OBJECT BAR (MYPROP 456)>")
.WithWarnings("ZIL0212")
.CompilesAsync();
// referenced in a routine - no warning
await AssertRoutine("", "<GETP ,FOO ,P?MYPROP>")
.WithGlobal("<OBJECT FOO (MYPROP 123)>")
.WithoutWarnings()
.CompilesAsync();
}
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment