[WIP] Warn if using this in constructor

[axic]

Fixes #583.

[chriseth]

I think both the changelog, the error description and the implementation should be changed to:

Warn only if this.f() is used, i.e. both this and this.f are fine. It would also be great to include functions called from the constructor (i.e. record function calls like this.f() while traversing the tree together with the function they are used in and then compile a report in the endVisit function of the contract).

[axic]

Rebased for good measure.

[axic]

@roadriverrail would you be interested in working on this?

[roadriverrail]

@axic Yes, I'll look into this soon

[roadriverrail]

With all my other stuff done or gated only on questions about style, I'm now actively working on this. I'm late to the party, but it seems like we would like to do the following:

1. Identify all function calls made in constructor, and all function calls that those functions call, etc.
2. Determine if any of these functions ever perform a this.foo( ) style call.
3. Report it as a warning.

Yes?

[axic]

@roadriverrail seems correct. The problem here is that while the contract address (this) and function types (this.f) are valid, but the contract at the destination is not yet present, so calling it results in failure. That is what we're warning for.

[roadriverrail]

@axic after talking with @chriseth earlier in the week, I'd effectively be building up a call graph for the contract for the purposes of implementing this analysis fully. We really ought to have a general-purpose call graph and then make the "no this in constructor call tree" part implemented on top of that, rather than building up a call graph for a single purpose. As such, and because there's a lot of enthusiasm for other things I'm working on, @chriseth and I agreed that I'd actually get this working for calls through this directly in the constructor and leave checking functions called by the constructor for later. There are some commits already here, so I'll look into finishing them up and writing tests for them. Perhaps we should break off the rest of the work to a separate issue in the issue tracker?

@chriseth please correct me if I'm wrong about any of that.

[roadriverrail]

Hey...the source branch for this PR is on the ethereum/solidity remote, which I can't push to. I can push it over to my remote, but that will cause a separate PR. Since I'm going to be leaving this work soon to work on other things, maybe I could just get push access to ethereum/solidity for a little while so I can put the clean up and rebase in? There's not much else to do, if we're not going to work on things that require a call graph to do.

[axic]

@roadriverrail if you would like to work on this, feel free to open a new PR and we can close this one.

[roadriverrail]

Yes. I've been meaning to

…

On Wed, Jul 19, 2017, 07:43 Alex Beregszaszi ***@***.***> wrote: @roadriverrail <https://github.com/roadriverrail> if you would like to work on this, feel free to open a new PR and we can close this one. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1646 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAlhYEiBjJ-olyyqhh29HJJMql4uDrlmks5sPevhgaJpZM4L3ltD> .

[roadriverrail]

Obviated by pull request #2605