Skip to content

Commit

Permalink
Use dmd in AsmStyleCheck
Browse files Browse the repository at this point in the history
  • Loading branch information
Vladiwostok committed Oct 29, 2023
1 parent 197a389 commit 6edc7eb
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 43 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
# Sublime Text 2
*.sublime-workspace

# Idea stuff
/.idea

# Subversion
.svn/

Expand Down
51 changes: 36 additions & 15 deletions src/dscanner/analysis/asm_style.d
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,58 @@
module dscanner.analysis.asm_style;

import std.stdio;
import dparse.ast;
import dparse.lexer;
import dscanner.analysis.base;
import dscanner.analysis.helpers;
import dsymbol.scope_ : Scope;
import dmd.tokens;

/**
* Checks for confusing asm expressions.
* See_also: $(LINK https://issues.dlang.org/show_bug.cgi?id=9738)
*/
final class AsmStyleCheck : BaseAnalyzer
extern(C++) class AsmStyleCheck(AST) : BaseAnalyzerDmd
{
alias visit = BaseAnalyzer.visit;

alias visit = BaseAnalyzerDmd.visit;
mixin AnalyzerInfo!"asm_style_check";

this(string fileName, const(Scope)* sc, bool skipTests = false)
extern(D) this(string fileName, bool skipTests = false)
{
super(fileName, sc, skipTests);
super(fileName, skipTests);
}

override void visit(const AsmBrExp brExp)
override void visit(AST.AsmStatement asmStatement)
{
if (brExp.asmBrExp !is null && brExp.asmBrExp.asmUnaExp !is null
&& brExp.asmBrExp.asmUnaExp.asmPrimaryExp !is null)
for (Token* token = asmStatement.tokens; token !is null; token = token.next)
{
addErrorMessage(brExp.line, brExp.column, "dscanner.confusing.brexp",
"This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.");
if (isConfusingStatement(token))
{
auto lineNum = cast(ulong) token.loc.linnum;
auto charNum = cast(ulong) token.loc.charnum;
addErrorMessage(lineNum, charNum, KEY, MESSAGE);
}
}
brExp.accept(this);
}

private bool isConfusingStatement(Token* token)
{
if (token.next is null)
return false;

if (token.next.next is null)
return false;

TOK tok1 = token.value;
TOK tok2 = token.next.value;
TOK tok3 = token.next.next.value;

if (tok1 == TOK.leftBracket && tok2 == TOK.int32Literal && tok3 == TOK.rightBracket)
return true;

return false;
}

private:
enum string KEY = "dscanner.confusing.brexp";
enum string MESSAGE = "This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.";
}

unittest
Expand All @@ -45,7 +66,7 @@ unittest

StaticAnalysisConfig sac = disabledConfig();
sac.asm_style_check = Check.enabled;
assertAnalyzerWarnings(q{
assertAnalyzerWarningsDMD(q{
void testAsm()
{
asm
Expand Down
44 changes: 21 additions & 23 deletions src/dscanner/analysis/helpers.d
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ S after(S)(S value, S separator) if (isSomeString!S)
* marked like so: // [warn]: Failed to do somethings.
*/
void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
string file = __FILE__, size_t line = __LINE__)
string file = __FILE__, size_t line = __LINE__)
{
import dscanner.analysis.run : parseModule;
import dparse.lexer : StringCache, Token;
Expand All @@ -75,8 +75,7 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
immutable size_t rawLine = rawWarning.line;
if (rawLine == 0)
{
stderr.writefln("!!! Skipping warning because it is on line zero:\n%s",
rawWarning.message);
stderr.writefln("!!! Skipping warning because it is on line zero:\n%s", rawWarning.message);
continue;
}

Expand Down Expand Up @@ -111,15 +110,15 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
// No warning
if (lineNo !in warnings)
{
immutable string errors = "Expected warning:\n%s\nFrom source code at (%s:?):\n%s".format(messages[lineNo],
lineNo, codeLines[lineNo - line]);
immutable string errors = "Expected warning:\n%s\nFrom source code at (%s:?):\n%s"
.format(messages[lineNo], lineNo, codeLines[lineNo - line]);
throw new AssertError(errors, file, lineNo);
}
// Different warning
else if (warnings[lineNo] != messages[lineNo])
{
immutable string errors = "Expected warning:\n%s\nBut was:\n%s\nFrom source code at (%s:?):\n%s".format(
messages[lineNo], warnings[lineNo], lineNo, codeLines[lineNo - line]);
immutable string errors = "Expected warning:\n%s\nBut was:\n%s\nFrom source code at (%s:?):\n%s"
.format(messages[lineNo], warnings[lineNo], lineNo, codeLines[lineNo - line]);
throw new AssertError(errors, file, lineNo);
}
}
Expand All @@ -131,8 +130,8 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
// Unexpected warning
if (lineNo !in messages)
{
unexpectedWarnings ~= "%s\nFrom source code at (%s:?):\n%s".format(warning,
lineNo, codeLines[lineNo - line]);
unexpectedWarnings ~= "%s\nFrom source code at (%s:?):\n%s"
.format(warning, lineNo, codeLines[lineNo - line]);
}
}
if (unexpectedWarnings.length)
Expand All @@ -143,7 +142,7 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
}

void assertAnalyzerWarningsDMD(string code, const StaticAnalysisConfig config, bool semantic = false,
string file = __FILE__, size_t line = __LINE__)
string file = __FILE__, size_t line = __LINE__)
{
import dmd.globals : global;
import dscanner.utils : getModuleName;
Expand All @@ -160,7 +159,7 @@ void assertAnalyzerWarningsDMD(string code, const StaticAnalysisConfig config, b
scope(exit)
{
assert(exists(deleteme));
remove(deleteme);
remove(deleteme);
}

f.write(code);
Expand All @@ -170,14 +169,14 @@ void assertAnalyzerWarningsDMD(string code, const StaticAnalysisConfig config, b

global.params.useUnitTests = true;
global.path = new Strings();
global.path.push((dmdParentDir ~ "/dmd").ptr);
global.path.push((dmdParentDir ~ "/dmd/druntime/src").ptr);
global.path.push((dmdParentDir ~ "/dmd" ~ "\0").ptr);
global.path.push((dmdParentDir ~ "/dmd/druntime/src" ~ "\0").ptr);

initDMD();

auto input = cast(char[]) code;
input ~= '\0';
auto t = dmd.frontend.parseModule(cast(const(char)[]) file, cast(const (char)[]) input);
auto t = dmd.frontend.parseModule(cast(const(char)[]) file, cast(const (char)[]) input);
if (semantic)
t.module_.fullSemantic();

Expand All @@ -193,8 +192,7 @@ void assertAnalyzerWarningsDMD(string code, const StaticAnalysisConfig config, b
immutable size_t rawLine = rawWarning.line;
if (rawLine == 0)
{
stderr.writefln("!!! Skipping warning because it is on line zero:\n%s",
rawWarning.message);
stderr.writefln("!!! Skipping warning because it is on line zero:\n%s", rawWarning.message);
continue;
}

Expand Down Expand Up @@ -229,15 +227,15 @@ void assertAnalyzerWarningsDMD(string code, const StaticAnalysisConfig config, b
// No warning
if (lineNo !in warnings)
{
immutable string errors = "Expected warning:\n%s\nFrom source code at (%s:?):\n%s".format(messages[lineNo],
lineNo, codeLines[lineNo - line]);
immutable string errors = "Expected warning:\n%s\nFrom source code at (%s:?):\n%s"
.format(messages[lineNo], lineNo, codeLines[lineNo - line]);
throw new AssertError(errors, file, lineNo);
}
// Different warning
else if (warnings[lineNo] != messages[lineNo])
{
immutable string errors = "Expected warning:\n%s\nBut was:\n%s\nFrom source code at (%s:?):\n%s".format(
messages[lineNo], warnings[lineNo], lineNo, codeLines[lineNo - line]);
immutable string errors = "Expected warning:\n%s\nBut was:\n%s\nFrom source code at (%s:?):\n%s"
.format(messages[lineNo], warnings[lineNo], lineNo, codeLines[lineNo - line]);
throw new AssertError(errors, file, lineNo);
}
}
Expand All @@ -249,13 +247,13 @@ void assertAnalyzerWarningsDMD(string code, const StaticAnalysisConfig config, b
// Unexpected warning
if (lineNo !in messages)
{
unexpectedWarnings ~= "%s\nFrom source code at (%s:?):\n%s".format(warning,
lineNo, codeLines[lineNo - line]);
unexpectedWarnings ~= "%s\nFrom source code at (%s:?):\n%s"
.format(warning, lineNo, codeLines[lineNo - line]);
}
}
if (unexpectedWarnings.length)
{
immutable string message = "Unexpected warnings:\n" ~ unexpectedWarnings.join("\n");
throw new AssertError(message, file, line);
}
}
}
12 changes: 7 additions & 5 deletions src/dscanner/analysis/run.d
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ unittest
config.asm_style_check = Check.enabled;
// this is done automatically by inifiled
config.filters.asm_style_check = filters.split(",");
return shouldRun!AsmStyleCheck(moduleName, config);
return moduleName.shouldRunDmd!(AsmStyleCheck!ASTCodegen)(config);
}

// test inclusion
Expand Down Expand Up @@ -464,10 +464,6 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a

GC.disable;

if (moduleName.shouldRun!AsmStyleCheck(analysisConfig))
checks ~= new AsmStyleCheck(fileName, moduleScope,
analysisConfig.asm_style_check == Check.skipTests && !ut);

if (moduleName.shouldRun!CommaExpressionCheck(analysisConfig))
checks ~= new CommaExpressionCheck(fileName, moduleScope,
analysisConfig.comma_expression_check == Check.skipTests && !ut);
Expand Down Expand Up @@ -693,6 +689,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN
config.useless_assert_check == Check.skipTests && !ut
);

if (moduleName.shouldRunDmd!(AsmStyleCheck!ASTCodegen)(config))
visitors ~= new AsmStyleCheck!ASTCodegen(
fileName,
config.asm_style_check == Check.skipTests && !ut
);

foreach (visitor; visitors)
{
m.accept(visitor);
Expand Down

0 comments on commit 6edc7eb

Please sign in to comment.