From 6edc7eb04fad24034d090c1badd846282cbb8921 Mon Sep 17 00:00:00 2001 From: Vladiwostok Date: Sun, 29 Oct 2023 22:59:44 +0200 Subject: [PATCH] Use dmd in AsmStyleCheck --- .gitignore | 3 ++ src/dscanner/analysis/asm_style.d | 51 ++++++++++++++++++++++--------- src/dscanner/analysis/helpers.d | 44 +++++++++++++------------- src/dscanner/analysis/run.d | 12 +++++--- 4 files changed, 67 insertions(+), 43 deletions(-) diff --git a/.gitignore b/.gitignore index 12324736..199e4408 100755 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,9 @@ # Sublime Text 2 *.sublime-workspace +# Idea stuff +/.idea + # Subversion .svn/ diff --git a/src/dscanner/analysis/asm_style.d b/src/dscanner/analysis/asm_style.d index de54c773..97dcb5bc 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/helpers.d b/src/dscanner/analysis/helpers.d index 2c269f3e..e375ca94 100644 --- a/src/dscanner/analysis/helpers.d +++ b/src/dscanner/analysis/helpers.d @@ -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; @@ -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; } @@ -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); } } @@ -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) @@ -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; @@ -160,7 +159,7 @@ void assertAnalyzerWarningsDMD(string code, const StaticAnalysisConfig config, b scope(exit) { assert(exists(deleteme)); - remove(deleteme); + remove(deleteme); } f.write(code); @@ -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(); @@ -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; } @@ -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); } } @@ -249,8 +247,8 @@ 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) @@ -258,4 +256,4 @@ void assertAnalyzerWarningsDMD(string code, const StaticAnalysisConfig config, b immutable string message = "Unexpected warnings:\n" ~ unexpectedWarnings.join("\n"); throw new AssertError(message, file, line); } -} \ No newline at end of file +} diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index a58f7b70..957c3fd3 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);