Clean up tab-space soup #15026
Clean up tab-space soup #15026
Conversation
| 5 | assert(x > 0); | ||
| | ^^^^^^^^^^^^^ | ||
| 5 | assert(x > 0); | ||
| | ^^^^^^^^^^^^^ |
There was a problem hiding this comment.
Note that tabs here come from the input file, which is indented with spaces. SMT checker sets the location to the whole line, not just the assert so the snippet includes indentation.
It could be worked around by changing the indentation to use spaces to match our style for Solidity, like I did here, but it's too tedious so I did not do that for other files. We should really fix SMTChecker to match the behavior of normal analysis and not include indents in locations.
There was a problem hiding this comment.
Maybe we should create an issue for that?
There was a problem hiding this comment.
Hmm... ok, maybe we should. I mean, it's a trivial issue and I doubt we'll get to it any time soon. On the other hand, even if it gets closed as stale eventually, maybe it's good to at least have a record that this is happening at all. It's just whitespace so it may not be apparent to people.
There was a problem hiding this comment.
In any case, this should not really block this PR. It was meant to be just a quick cleanup.
Now that I think of it - do you expect it will cause conflicts with your nlohmann JSON PR? If it does, I can help rebasing, as I offered in the description.
There was a problem hiding this comment.
Created an issue here: #15033. Just note that it's very low priority. No particular need to jump at it right now.
| @@ -334,7 +334,7 @@ BOOST_AUTO_TEST_CASE(store_tags_as_unions) | |||
| numSHA3s++; | |||
| }); | |||
| // TEST DISABLED UNTIL 93693404 IS IMPLEMENTED | |||
There was a problem hiding this comment.
Just scrolled through - It's unrelated to this PR, but I'm really wondering now what 93693404 means
There was a problem hiding this comment.
This comment is ancient history (written still in cpp-ethereum and only imported to this repo in #5) so it's probably an ID from whatever issue tracking system the project was using back then. I think it was Pivotal Tracker. There are traces of it in some old issues, e.g. #125 (comment)
We have quite a few sloppy chunks of code that mix spaces and tabs or use tabs for alignment, which often makes the code really annoying to work with. I wanted to clean this up many times but thought it wasn't worth the big diff. But if we're going to clean this up wholesale for
InteractiveTests.hin #15009, we may just as well do it in other places too and be done with it.This could cause some conflicts, but I think it won't be that many since this mess is mostly in places we touch rarely (otherwise it would be gone already). If any PR is significantly I'm willing to even rebase it myself if it means finally getting rid of this.
Note: the PR should be pretty easy to review if you enable the option to ignore whitespace :)