Replace libdparse in AsmStyleCheck (#75)
This commit is contained in:
parent
52b2a16e1a
commit
608e773400
|
|
@ -6,39 +6,58 @@
|
||||||
module dscanner.analysis.asm_style;
|
module dscanner.analysis.asm_style;
|
||||||
|
|
||||||
import std.stdio;
|
import std.stdio;
|
||||||
import dparse.ast;
|
|
||||||
import dparse.lexer;
|
|
||||||
import dscanner.analysis.base;
|
import dscanner.analysis.base;
|
||||||
import dscanner.analysis.helpers;
|
import dscanner.analysis.helpers;
|
||||||
import dsymbol.scope_ : Scope;
|
import dmd.tokens;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Checks for confusing asm expressions.
|
* Checks for confusing asm expressions.
|
||||||
* See_also: $(LINK https://issues.dlang.org/show_bug.cgi?id=9738)
|
* 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";
|
mixin AnalyzerInfo!"asm_style_check";
|
||||||
|
|
||||||
this(BaseAnalyzerArguments args)
|
extern (D) this(string fileName, bool skipTests = false)
|
||||||
{
|
{
|
||||||
super(args);
|
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
|
for (Token* token = asmStatement.tokens; token !is null; token = token.next)
|
||||||
&& brExp.asmBrExp.asmUnaExp.asmPrimaryExp !is null)
|
|
||||||
{
|
{
|
||||||
addErrorMessage(brExp, KEY,
|
if (isConfusingStatement(token))
|
||||||
"This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.");
|
{
|
||||||
|
auto lineNum = cast(ulong) token.loc.linnum;
|
||||||
|
auto charNum = cast(ulong) token.loc.charnum;
|
||||||
|
addErrorMessage(lineNum, charNum, KEY, MESSAGE);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
brExp.accept(this);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private enum string KEY = "dscanner.confusing.brexp";
|
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
|
unittest
|
||||||
|
|
@ -47,13 +66,12 @@ unittest
|
||||||
|
|
||||||
StaticAnalysisConfig sac = disabledConfig();
|
StaticAnalysisConfig sac = disabledConfig();
|
||||||
sac.asm_style_check = Check.enabled;
|
sac.asm_style_check = Check.enabled;
|
||||||
assertAnalyzerWarnings(q{
|
assertAnalyzerWarningsDMD(q{
|
||||||
void testAsm()
|
void testAsm()
|
||||||
{
|
{
|
||||||
asm
|
asm
|
||||||
{
|
{
|
||||||
mov a, someArray[1]; /+
|
mov a, someArray[1]; // [warn]: This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.
|
||||||
^^^^^^^^^^^^ [warn]: This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify. +/
|
|
||||||
add near ptr [EAX], 3;
|
add near ptr [EAX], 3;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -785,7 +785,7 @@ unittest
|
||||||
config.asm_style_check = Check.enabled;
|
config.asm_style_check = Check.enabled;
|
||||||
// this is done automatically by inifiled
|
// this is done automatically by inifiled
|
||||||
config.filters.asm_style_check = filters.split(",");
|
config.filters.asm_style_check = filters.split(",");
|
||||||
return shouldRun!AsmStyleCheck(moduleName, config);
|
return moduleName.shouldRunDmd!(AsmStyleCheck!ASTCodegen)(config);
|
||||||
}
|
}
|
||||||
|
|
||||||
// test inclusion
|
// test inclusion
|
||||||
|
|
@ -828,10 +828,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
|
||||||
moduleScope
|
moduleScope
|
||||||
);
|
);
|
||||||
|
|
||||||
if (moduleName.shouldRun!AsmStyleCheck(analysisConfig))
|
|
||||||
checks ~= new AsmStyleCheck(args.setSkipTests(
|
|
||||||
analysisConfig.asm_style_check == Check.skipTests && !ut));
|
|
||||||
|
|
||||||
if (moduleName.shouldRun!CommaExpressionCheck(analysisConfig))
|
if (moduleName.shouldRun!CommaExpressionCheck(analysisConfig))
|
||||||
checks ~= new CommaExpressionCheck(args.setSkipTests(
|
checks ~= new CommaExpressionCheck(args.setSkipTests(
|
||||||
analysisConfig.comma_expression_check == Check.skipTests && !ut));
|
analysisConfig.comma_expression_check == Check.skipTests && !ut));
|
||||||
|
|
@ -1342,6 +1338,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN
|
||||||
config.useless_assert_check == Check.skipTests && !ut
|
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)
|
foreach (visitor; visitors)
|
||||||
{
|
{
|
||||||
m.accept(visitor);
|
m.accept(visitor);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue