diff --git a/src/dscanner/analysis/asm_style.d b/src/dscanner/analysis/asm_style.d index de54c773..ab83596d 100644 --- a/src/dscanner/analysis/asm_style.d +++ b/src/dscanner/analysis/asm_style.d @@ -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 @@ -45,7 +66,7 @@ unittest StaticAnalysisConfig sac = disabledConfig(); sac.asm_style_check = Check.enabled; - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ void testAsm() { asm diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index 8adc4b2c..57f5942b 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -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 @@ -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); @@ -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);