diff --git a/compiler/src/build.d b/compiler/src/build.d index 2114282bdad9..96f598984fa4 100755 --- a/compiler/src/build.d +++ b/compiler/src/build.d @@ -1592,6 +1592,7 @@ auto sourceFiles() visitor/strict.d visitor/transitive.d cparse.d dfa/entry.d dfa/utils.d dfa/fast/structure.d dfa/fast/analysis.d dfa/fast/report.d dfa/fast/expression.d dfa/fast/statement.d + lint/engine.d "), backendHeaders: fileArray(env["C"], " cc.d cdef.d cgcv.d code.d dt.d el.d global.d diff --git a/compiler/src/dmd/frontend.h b/compiler/src/dmd/frontend.h index a5b710cfd503..ebb46e8c6ea5 100644 --- a/compiler/src/dmd/frontend.h +++ b/compiler/src/dmd/frontend.h @@ -3772,6 +3772,7 @@ class FuncDeclaration : public Declaration AttributeViolation* pureViolation; AttributeViolation* nothrowViolation; ParametersDFAInfo* parametersDFAInfo; + void* lintInfo; bool purityInprocess() const; bool purityInprocess(bool v); bool safetyInprocess() const; @@ -5942,6 +5943,29 @@ enum class CPU : uint8_t native = 12u, }; +enum class LintFlags : uint32_t +{ + none = 0u, + constSpecial = 1u, + unusedParams = 2u, + all = 4294967295u, +}; + +struct TrackedParam final +{ + VarDeclaration* decl; + bool used; + TrackedParam() : + decl(), + used() + { + } + TrackedParam(VarDeclaration* decl, bool used = false) : + decl(decl), + used(used) + {} +}; + enum class ErrorPrintMode : uint8_t { simpleError = 0u, @@ -7871,6 +7895,40 @@ class StatementWalker : public SemanticTimeTransitiveVisitor void visit(Statement* st) final override; }; +class LintVisitor final : public Visitor +{ +public: + using Visitor::visit; + _d_dynamicArray< LintFlags > flagsStack; + _d_dynamicArray< TrackedParam > activeParams; + LintVisitor(); + LintFlags currentFlags(); + void visit(Dsymbol* s) override; + void visit(Statement* s) override; + void visit(Expression* e) override; + void visit(Initializer* i) override; + void visit(DeclarationExp* de) override; + void visit(Module* m) override; + void visit(AttribDeclaration* ad) override; + void visit(PragmaDeclaration* pd) override; + void visit(PragmaStatement* ps) override; + void visit(AggregateDeclaration* ad) override; + void visit(TemplateInstance* ti) override; + void visit(FuncDeclaration* fd) override; + void visit(VarExp* ve) override; + void visit(CompoundStatement* s) override; + void visit(ExpStatement* s) override; + void visit(IfStatement* s) override; + void visit(ReturnStatement* s) override; + void visit(ForStatement* s) override; + void visit(BinExp* e) override; + void visit(UnaExp* e) override; + void visit(CallExp* e) override; + void visit(VarDeclaration* vd) override; + void visit(ExpInitializer* ei) override; + void visit(FuncExp* fe) override; +}; + extern _d_real creall(complex_t x); extern _d_real cimagl(complex_t x); @@ -8871,6 +8929,13 @@ struct Id final static Identifier* ident; static Identifier* packed; static Identifier* op; + static Identifier* lint; + static Identifier* constSpecial; + static Identifier* unusedParams; + static Identifier* LintParams; + static Identifier* enabled; + static Identifier* none; + static Identifier* all; static void initialize(); Id() { diff --git a/compiler/src/dmd/func.d b/compiler/src/dmd/func.d index a2ccc4c2256f..cef28894cded 100644 --- a/compiler/src/dmd/func.d +++ b/compiler/src/dmd/func.d @@ -302,6 +302,8 @@ extern (C++) class FuncDeclaration : Declaration ParametersDFAInfo* parametersDFAInfo; + void* lintInfo; + /// See the `FUNCFLAG` struct import dmd.common.bitfields; mixin(generateBitFields!(FUNCFLAG, uint)); diff --git a/compiler/src/dmd/id.d b/compiler/src/dmd/id.d index 49f47e4a9a0a..94187595f2fe 100644 --- a/compiler/src/dmd/id.d +++ b/compiler/src/dmd/id.d @@ -539,6 +539,15 @@ immutable Msgtable[] msgtable = // for inline assembler { "op" }, + // lint + { "lint" }, + { "constSpecial" }, + { "unusedParams" }, + { "LintParams" }, + { "enabled" }, + { "none" }, + { "all" }, + ]; diff --git a/compiler/src/dmd/lint/engine.d b/compiler/src/dmd/lint/engine.d new file mode 100644 index 000000000000..c3249d2f8184 --- /dev/null +++ b/compiler/src/dmd/lint/engine.d @@ -0,0 +1,398 @@ +module dmd.lint.engine; + +import dmd.aggregate; +import dmd.arraytypes; +import dmd.astenums; +import dmd.attrib; +import dmd.declaration; +import dmd.dmodule; +import dmd.dsymbol; +import dmd.dtemplate; +import dmd.expression; +import dmd.func; +import dmd.id; +import dmd.statement; +import dmd.visitor; +import dmd.init; + +import dmd.errors : warning; + +extern (D) enum LintFlags : uint +{ + none = 0, + constSpecial = 1 << 0, + unusedParams = 1 << 1, + all = ~0 +} + +private struct TrackedParam +{ + VarDeclaration decl; + bool used; +} + +extern(C++) final class LintVisitor : Visitor +{ + alias visit = Visitor.visit; + + LintFlags[] flagsStack; + TrackedParam[] activeParams; + + this() + { + flagsStack ~= LintFlags.none; + } + + LintFlags currentFlags() + { + return flagsStack.length > 0 ? flagsStack[$ - 1] : LintFlags.none; + } + + override void visit(Dsymbol s) { } + override void visit(Statement s) { } + override void visit(Expression e) { } + override void visit(Initializer i) { } + + override void visit(DeclarationExp de) + { + if (de && de.declaration) + de.declaration.accept(this); + } + + //override void visit(AliasDeclaration ad) + //{ + // if (ad && ad.aliassym) + // ad.aliassym.accept(this); + //} + + override void visit(Module m) + { + if (!m || !m.members) return; + foreach (s; *m.members) + if (s) s.accept(this); + } + + override void visit(AttribDeclaration ad) + { + if (ad && ad.decl) + { + foreach (s; *ad.decl) + if (s) s.accept(this); + } + } + + override void visit(PragmaDeclaration pd) + { + if (!pd) return; + bool pushed = pushLintFlags(pd); + + if (pd.decl) + { + foreach (s; *pd.decl) + if (s) s.accept(this); + } + + if (pushed) + flagsStack.length--; + } + + override void visit(PragmaStatement ps) + { + if (!ps) return; + bool pushed = false; + if (ps.ident == Id.lint) + { + pushed = true; + flagsStack ~= parsePragmaArgs(ps.args); + } + + if (ps._body) + ps._body.accept(this); + + if (pushed) + flagsStack.length--; + } + + override void visit(AggregateDeclaration ad) + { + if (!ad || !ad.members) return; + foreach (s; *ad.members) + if (s) s.accept(this); + } + + override void visit(TemplateInstance ti) + { + if (!ti || !ti.members) return; + foreach (s; *ti.members) + if (s) s.accept(this); + } + + override void visit(FuncDeclaration fd) + { + if (!fd) return; + + const flags = currentFlags(); + + if (flags & LintFlags.constSpecial) + checkConstSpecial(fd); + + bool checkUnused = (flags & LintFlags.unusedParams) != 0; + + if (checkUnused) + { + import dmd.astenums : LINK; + if (!fd.fbody || + (fd.vtblIndex != -1 && !fd.isFinalFunc()) || + (fd.foverrides.length > 0) || + (fd._linkage == LINK.c || fd._linkage == LINK.cpp || fd._linkage == LINK.windows)) + { + checkUnused = false; + } + } + + size_t paramStart = activeParams.length; + + if (checkUnused && fd.parameters) + { + for (size_t i = 0; i < fd.parameters.length; i++) + { + VarDeclaration v = (*fd.parameters)[i]; + + if (!v || !v.ident) continue; + + bool isIgnoredName = v.ident.toChars()[0] == '_'; + + if (!(v.storage_class & STC.temp) && !isIgnoredName) + { + activeParams ~= TrackedParam(v, false); + } + } + } + + if (fd.fbody) + fd.fbody.accept(this); + + if (checkUnused) + { + for (size_t i = paramStart; i < activeParams.length; i++) + { + if (!activeParams[i].used) + { + warning(activeParams[i].decl.loc, "[unusedParams] function parameter `%s` is never used", activeParams[i].decl.ident.toChars()); + } + } + } + + activeParams.length = paramStart; + } + + private void checkConstSpecial(FuncDeclaration fd) + { + if (fd.isGenerated || (fd.storage_class & STC.const_) || (fd.type && fd.type.isConst())) + return; + + if (fd.ident != Id.opEquals && fd.ident != Id.opCmp && + fd.ident != Id.tohash && fd.ident != Id.tostring) + return; + + if (!fd.toParent2() || !fd.toParent2().isStructDeclaration()) + return; + + warning(fd.loc, "[constSpecial] special method `%s` should be marked as `const`", fd.ident ? fd.ident.toChars() : fd.toChars()); + } + + private bool pushLintFlags(PragmaDeclaration pd) + { + if (pd && pd.ident == Id.lint) + { + flagsStack ~= parsePragmaArgs(pd.args); + return true; + } + return false; + } + + private LintFlags parsePragmaArgs(Expressions* args) + { + import core.stdc.string : strcmp; + + LintFlags newFlags = currentFlags(); + if (!args || args.length == 0) + { + newFlags |= LintFlags.all; + return newFlags; + } + + foreach (arg; *args) + { + if (!arg) continue; + + if (auto ve = arg.isVarExp()) + { + if (ve.var && ve.var.isVarDeclaration() && ve.var.isVarDeclaration()._init) + { + if (auto ei = ve.var.isVarDeclaration()._init.isExpInitializer()) + arg = ei.exp; + } + } + + if (auto id = arg.isIdentifierExp()) + { + if (id.ident == Id.constSpecial) newFlags |= LintFlags.constSpecial; + else if (id.ident == Id.unusedParams) newFlags |= LintFlags.unusedParams; + else if (id.ident == Id.none) newFlags = LintFlags.none; + else if (id.ident == Id.all) newFlags |= LintFlags.all; + } + + else if (auto sle = arg.isStructLiteralExp()) + { + if (sle.sd && sle.sd.ident && strcmp(sle.sd.ident.toChars(), "LintParams") == 0) + { + bool masterEnabled = true; + LintFlags tempFlags = newFlags; + + for (size_t i = 0; i < sle.elements.length; i++) + { + Expression e = (*sle.elements)[i]; + if (!e) continue; + + VarDeclaration field = sle.sd.fields[i]; + if (!field || !field.ident) continue; + + bool isFieldEnabled = false; + + if (auto ie = e.isIntegerExp()) + { + isFieldEnabled = ie.getInteger() != 0; + } + else if (auto ce = e.isCastExp()) + { + if (auto ce_ie = ce.e1.isIntegerExp()) + isFieldEnabled = ce_ie.getInteger() != 0; + } + + const(char)* fieldName = field.ident.toChars(); + + if (strcmp(fieldName, "enabled") == 0) + { + masterEnabled = isFieldEnabled; + } + else if (strcmp(fieldName, "constSpecial") == 0) + { + if (isFieldEnabled) tempFlags |= LintFlags.constSpecial; + else tempFlags &= ~LintFlags.constSpecial; + } + else if (strcmp(fieldName, "unusedParams") == 0) + { + if (isFieldEnabled) tempFlags |= LintFlags.unusedParams; + else tempFlags &= ~LintFlags.unusedParams; + } + } + + if (!masterEnabled) + newFlags = LintFlags.none; + else + newFlags = tempFlags; + } + } + } + return newFlags; + } + + override void visit(VarExp ve) + { + if (!ve || !ve.var) return; + if (auto vd = ve.var.isVarDeclaration()) + { + for (size_t i = activeParams.length; i-- > 0; ) + { + if (activeParams[i].decl == vd) + { + activeParams[i].used = true; + break; + } + } + } + } + + override void visit(CompoundStatement s) + { + if (s && s.statements) + foreach (stmt; *s.statements) + if (stmt) stmt.accept(this); + } + + override void visit(ExpStatement s) + { + if (s && s.exp) s.exp.accept(this); + } + + override void visit(IfStatement s) + { + if (!s) return; + if (s.condition) s.condition.accept(this); + if (s.ifbody) s.ifbody.accept(this); + if (s.elsebody) s.elsebody.accept(this); + } + + override void visit(ReturnStatement s) + { + if (s && s.exp) s.exp.accept(this); + } + + override void visit(ForStatement s) + { + if (!s) return; + if (s._init) s._init.accept(this); + if (s.condition) s.condition.accept(this); + if (s.increment) s.increment.accept(this); + if (s._body) s._body.accept(this); + } + + override void visit(BinExp e) + { + if (!e) return; + if (e.e1) e.e1.accept(this); + if (e.e2) e.e2.accept(this); + } + + override void visit(UnaExp e) + { + if (e && e.e1) e.e1.accept(this); + } + + override void visit(CallExp e) + { + if (!e) return; + if (e.e1) e.e1.accept(this); + if (e.arguments) + foreach (arg; *e.arguments) + if (arg) arg.accept(this); + } + + override void visit(VarDeclaration vd) + { + if (vd && vd._init) + vd._init.accept(this); + } + + override void visit(ExpInitializer ei) + { + if (ei && ei.exp) + ei.exp.accept(this); + } + + override void visit(FuncExp fe) + { + if (fe && fe.fd) + fe.fd.accept(this); + } +} + +extern(D) void runLinter(Module[] modules) +{ + scope visitor = new LintVisitor(); + foreach (m; modules) + { + if (m) m.accept(visitor); + } +} diff --git a/compiler/src/dmd/main.d b/compiler/src/dmd/main.d index 5b37bd3cd68b..d0e2d40b9134 100644 --- a/compiler/src/dmd/main.d +++ b/compiler/src/dmd/main.d @@ -53,6 +53,7 @@ import dmd.id; import dmd.identifier; import dmd.inline; import dmd.link; +import dmd.lint.engine : runLinter; import dmd.location; import dmd.mars; import dmd.mtype; @@ -665,6 +666,8 @@ private int tryMain(const(char)[][] argv, out Param params) if (global.errors) removeHdrFilesAndFail(params, modules); + runLinter(modules[]); + { timeTraceBeginEvent(TimeTraceEventType.inlineGeneral); scope (exit) timeTraceEndEvent(TimeTraceEventType.inlineGeneral); diff --git a/compiler/src/dmd/pragmasem.d b/compiler/src/dmd/pragmasem.d index 83408a1bb338..097ea320fa32 100644 --- a/compiler/src/dmd/pragmasem.d +++ b/compiler/src/dmd/pragmasem.d @@ -206,6 +206,29 @@ void pragmaDeclSemantic(PragmaDeclaration pd, Scope* sc) .error(pd.loc, "%s `%s` takes no argument", pd.kind, pd.toPrettyChars); return declarations(); } + else if (pd.ident == Id.lint) + { + if (pd.args) + { + for (size_t i = 0; i < pd.args.length; i++) + { + Expression e = (*pd.args)[i]; + if (auto id = e.isIdentifierExp()) + { + if (id.ident == Id.constSpecial || id.ident == Id.unusedParams || + id.ident == Id.none || id.ident == Id.all) + { + continue; + } + } + + e = e.expressionSemantic(sc); + e = e.ctfeInterpret(); + (*pd.args)[i] = e; + } + } + return declarations(); + } else if (!global.params.ignoreUnsupportedPragmas) { error(pd.loc, "unrecognized `pragma(%s)`", pd.ident.toErrMsg()); @@ -329,6 +352,28 @@ bool pragmaStmtSemantic(PragmaStatement ps, Scope* sc) if (!pragmaMangleSemantic(ps.loc, sc, ps.args, decls.length ? &decls : null)) return false; } + else if (ps.ident == Id.lint) + { + if (ps.args) + { + for (size_t i = 0; i < ps.args.length; i++) + { + Expression e = (*ps.args)[i]; + if (auto id = e.isIdentifierExp()) + { + if (id.ident == Id.constSpecial || id.ident == Id.unusedParams || + id.ident == Id.none || id.ident == Id.all) + { + continue; + } + } + + e = e.expressionSemantic(sc); + e = e.ctfeInterpret(); + (*ps.args)[i] = e; + } + } + } else if (!global.params.ignoreUnsupportedPragmas) { error(ps.loc, "unrecognized `pragma(%s)`", ps.ident.toErrMsg()); diff --git a/compiler/test/compilable/lint_struct_params.d b/compiler/test/compilable/lint_struct_params.d new file mode 100644 index 000000000000..d7096c5e2054 --- /dev/null +++ b/compiler/test/compilable/lint_struct_params.d @@ -0,0 +1,32 @@ +/* +REQUIRED_ARGS: -w +TEST_OUTPUT: +---- +---- +*/ + +struct LintParams { + bool enabled = true; + bool constSpecial = true; + bool unusedParams = true; +} + +enum MyLintParams = LintParams(true, false, false); + +pragma(lint, MyLintParams); + +struct TestStruct +{ + bool opEquals(ref const TestStruct other) { return true; } +} + +class A +{ + void foo(int x) + { + } +} + +void main() +{ +} diff --git a/compiler/test/compilable/lint_unused_params.d b/compiler/test/compilable/lint_unused_params.d new file mode 100644 index 000000000000..730e390e4617 --- /dev/null +++ b/compiler/test/compilable/lint_unused_params.d @@ -0,0 +1,15 @@ +/* +REQUIRED_ARGS: -w +TEST_OUTPUT: +---- +---- +*/ + +pragma(lint, unusedParams): + +class A +{ + void foo(int x) + { + } +} diff --git a/compiler/test/compilable/lint_unused_params2.d b/compiler/test/compilable/lint_unused_params2.d new file mode 100644 index 000000000000..5d582852b4c2 --- /dev/null +++ b/compiler/test/compilable/lint_unused_params2.d @@ -0,0 +1,19 @@ +/* +REQUIRED_ARGS: -w +TEST_OUTPUT: +---- +---- +*/ + +pragma(lint, unusedParams): + +class A +{ + void foo(int x) {} +} + +void main() +{ + auto a = new A(); + a.foo(1); +} diff --git a/compiler/test/fail_compilation/lint_const_special.d b/compiler/test/fail_compilation/lint_const_special.d new file mode 100644 index 000000000000..27760feae6e1 --- /dev/null +++ b/compiler/test/fail_compilation/lint_const_special.d @@ -0,0 +1,76 @@ +/* +REQUIRED_ARGS: -w +TEST_OUTPUT: +--- +fail_compilation/lint_const_special.d(20): Warning: [constSpecial] special method `opEquals` should be marked as `const` +fail_compilation/lint_const_special.d(25): Warning: [constSpecial] special method `toHash` should be marked as `const` +fail_compilation/lint_const_special.d(30): Warning: [constSpecial] special method `opCmp` should be marked as `const` +fail_compilation/lint_const_special.d(35): Warning: [constSpecial] special method `toString` should be marked as `const` +Error: warnings are treated as errors + Use -wi if you wish to treat warnings only as informational. +--- +*/ + +// Enable our new lint rule for the file +pragma(lint, constSpecial): + +struct BadStruct +{ + // LINT: special method `opEquals` should be marked as `const` + bool opEquals(ref const BadStruct rhs) { + return true; + } + + // LINT: special method `toHash` should be marked as `const` + hash_t toHash() { + return 0; + } + + // LINT: special method `opCmp` should be marked as `const` + int opCmp(ref const BadStruct rhs) { + return 0; + } + + // LINT: special method `toString` should be marked as `const` + string toString() { + return "BadStruct"; + } +} + +struct GoodStruct +{ + // OK: method is marked as const + bool opEquals(ref const GoodStruct rhs) const { + return true; + } + + // OK: method is marked as const + hash_t toHash() const { + return 1; + } + + // OK: method is marked as const + int opCmp(ref const GoodStruct rhs) const { + return 0; + } + + // OK: method is marked as const + string toString() const { + return "GoodStruct"; + } +} + +// Disable the linter for the following code +pragma(lint, none): + +struct IgnoredStruct +{ + // No lint messages here, as the linter is disabled! + bool opEquals(ref const IgnoredStruct rhs) { + return true; + } +} + +void main() +{ +} diff --git a/compiler/test/fail_compilation/lint_struct_params_fail.d b/compiler/test/fail_compilation/lint_struct_params_fail.d new file mode 100644 index 000000000000..ca86a34e0b86 --- /dev/null +++ b/compiler/test/fail_compilation/lint_struct_params_fail.d @@ -0,0 +1,30 @@ +/* +REQUIRED_ARGS: -w +TEST_OUTPUT: +--- +fail_compilation/lint_struct_params_fail.d(24): Warning: [constSpecial] special method `opEquals` should be marked as `const` +fail_compilation/lint_struct_params_fail.d(29): Warning: [unusedParams] function parameter `x` is never used +Error: warnings are treated as errors + Use -wi if you wish to treat warnings only as informational. +--- +*/ + +struct LintParams { + bool enabled = true; + bool constSpecial = true; + bool unusedParams = true; +} + +enum StrictLint = LintParams(true, true, true); +pragma(lint, StrictLint): + +struct BadStruct +{ + // LINT: constSpecial + bool opEquals(ref const BadStruct _rhs) { return true; } +} + +class A +{ + final void foo(int x) {} +} diff --git a/compiler/test/fail_compilation/lint_unused_params.d b/compiler/test/fail_compilation/lint_unused_params.d new file mode 100644 index 000000000000..583fd6dd3152 --- /dev/null +++ b/compiler/test/fail_compilation/lint_unused_params.d @@ -0,0 +1,72 @@ +/* +REQUIRED_ARGS: -w +TEST_OUTPUT: +---- +fail_compilation/lint_unused_params.d(17): Warning: [unusedParams] function parameter `y` is never used +fail_compilation/lint_unused_params.d(36): Warning: [unusedParams] function parameter `b` is never used +fail_compilation/lint_unused_params.d(52): Warning: [unusedParams] function parameter `b` is never used +fail_compilation/lint_unused_params.d(64): Warning: [unusedParams] function parameter `z` is never used +Error: warnings are treated as errors + Use -wi if you wish to treat warnings only as informational. +---- +*/ + +pragma(lint, unusedParams): + +// 1. Regular functions +void testBasic(int x, int y) +{ + cast(void)x; +} + +// 2. Interfaces and abstract methods (no body - ignored) +interface I { void ifaceMethod(int a); } +abstract class AbstractBase { abstract void absMethod(int a); } + +// 3. Virtual and overridden methods (ignored) +class Base { void foo(int a) {} } +class Derived : Base +{ + override void foo(int a) {} +} + +// 4. Final methods (cannot be overridden, so parameters must be used) +class Normal +{ + final void bar(int a, int b) + { + cast(void)a; + } +} + +// 5. Template functions (checked upon instantiation) +void tplFunc(T)(T x, T y) +{ + cast(void)x; +} +alias instantiateTpl = tplFunc!int; + +// 6. Delegates and lambdas +void testDelegate() +{ + auto dg = (int a, int b) { + cast(void)a; + }; +} + +// 7. Unnamed parameters (ignored by the compiler as they have STC.temp) +void unnamedParam(int, int x) +{ + cast(void)x; +} + +// 8. Default parameters +void defaultArg(int x = 5, int z = 10) +{ + cast(void)x; +} + +// 9. Disable linter for the rest of the file +pragma(lint, none): + +void completelyIgnored(int a) {}