Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions source/rock/frontend/BuildParams.ooc
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,9 @@ BuildParams: class {
// More debug messages
veryVerbose := false

// Give a warning when `this` is added implicitly
explicitThis := false

// Debugging purposes
debugLoop := false
debugLibcache := false
Expand Down
4 changes: 4 additions & 0 deletions source/rock/frontend/CommandLine.ooc
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,10 @@ CommandLine: class {
params verboser = true
params veryVerbose = true

} else if (option == "explicitThis") {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write --explicitThis to turn on the warnings.


params explicitThis = true

} else if (option == "stats") {

if(!longOption) warnUseLong("stats")
Expand Down
14 changes: 11 additions & 3 deletions source/rock/middle/Addon.ooc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import structs/[ArrayList, HashMap, MultiMap]
import ../frontend/BuildParams
import Node, Type, TypeDecl, FunctionDecl, FunctionCall, Visitor, VariableAccess, PropertyDecl, ClassDecl, CoverDecl
import tinker/[Trail, Resolver, Response, Errors]

Expand Down Expand Up @@ -155,9 +156,16 @@ Addon: class extends Node {
}

if (call suggest(fDecl, res, trail)) {
if (fDecl hasThis() && !call getExpr()) {
// add `this` if needed.
call setExpr(VariableAccess new("this", call token))
if (!call getExpr()) {
if (fDecl hasThis()) {
// add `this` if needed.
call setExpr(VariableAccess new("this", call token))
}
// Not yet tested
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just have to assume that it does not give false detection until someone uses addons while not using implicit this.

if (res params explicitThis) {
name := fDecl hasThis() ? "this" : "This"
call token printMessage("Implicit " + name + " detected for call to " + fDecl name + "!")
}
}
}
)
Expand Down
36 changes: 34 additions & 2 deletions source/rock/middle/TypeDecl.ooc
Original file line number Diff line number Diff line change
Expand Up @@ -1253,6 +1253,29 @@ TypeDecl: abstract class extends Declaration {
varAcc := VariableAccess new("this", access token)
varAcc reverseExpr = access
access expr = varAcc
if (res params explicitThis) {
mustBeExplicit := true

// Generic variables do not require explicit this
if (vDecl getType()) {
if (vDecl getType() toString() == "Class") {
mustBeExplicit = false
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filter out variables like T from generic classes since the variable is also a type used to declare other variables.

}
}

// Access to a variable from within its own declaration does not have to be explicit
depth := trail getSize() - 1
while (depth >= 0) {
if (trail get(depth) == vDecl) {
mustBeExplicit = false
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filter out access from inside the variable's own declaration since most of them are generated getters and setters.

}
depth -= 1
}

if (mustBeExplicit) {
access token printMessage("Implicit this detected for variable access of " + vDecl name + "!")
}
}
}
return 0
}
Expand Down Expand Up @@ -1360,8 +1383,14 @@ TypeDecl: abstract class extends Declaration {
if (fDecl) {
if (call debugCondition()) " \\o/ Found fDecl for %s, it's %s" format(call name, fDecl toString()) println()
if (call suggest(fDecl, res, trail)) {
if (fDecl hasThis() && !call getExpr()) {
call setExpr(VariableAccess new("this", call token))
if (!call getExpr()) {
if (fDecl hasThis()) {
call setExpr(VariableAccess new("this", call token))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the this argument is not done for static functions but by splitting the condition into missing this (!call getExpr()) and expect this (fDecl hasThis()), we can give warnings for static functions too.

}
if (res params explicitThis && fDecl name != "init" && fDecl name != ClassDecl DEFAULTS_FUNC_NAME) {
name := fDecl hasThis() ? "this" : "This"
call token printMessage("Implicit " + name + " detected for call to " + fDecl name + "!")
}
}
return 0
}
Expand Down Expand Up @@ -1391,6 +1420,9 @@ TypeDecl: abstract class extends Declaration {
// if the variable is static, use class scope not instance
name := vDecl isStatic() ? "This" : "this"
call setExpr(VariableAccess new(name, call token))
if (res params explicitThis) {
call token printMessage("Implicit " + name + " detected for call to " + vDecl name + "!")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that this is function calls by pointer from the table since variables are used for access.

}
}
}
}
Expand Down