Skip to content

Commit

Permalink
Replace libdparse in AsmStyleCheck (#75)
Browse files Browse the repository at this point in the history
  • Loading branch information
Vladiwostok committed Nov 7, 2023
1 parent 8c50b17 commit 2b7d5ec
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 20 deletions.
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
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 2b7d5ec

Please sign in to comment.