Re-add -t analyze

[rlerdorf]

Take this code (note the eval is only to make it a single standalone example, the actual problem also appears on a traditional file-based autoloader):

<?php
$code = <<<EOB
trait my_trait {
  static function test(\$arg) { echo \$arg; }
}
EOB;

function my_autoloader($class) {
    global $code;
    echo "autoloading $class\n";
    eval($code);
}

spl_autoload_register('my_autoloader');

class try_it {
  use my_trait;
}

try_it::test("does it work?\n");

This used to be parsed correctly by hphp -t analyze and also executed correctly directly by hphp:

% hphp --version
HipHop Compiler v2.1.0-dev
Compiler: heads/master-0-gbe9f36d3bcaaedc295b52c0232fcc9c8d7ef8e95
Repo schema: dc35fda17c13c213ce12c174cc87f0e402a65188
% hphp trait.php
autoloading my_trait
does it work?

However, somewhere along the line it broke:

% hphp --version
HipHop Repo Compiler 3.4.0-dev (rel)
Compiler: heads/master-1-gcb9dd4998090362d6c06bffa6a15f7c389e8a8fd
Repo schema: 267ef70d14edf91f1b9f5dfe306eb8b51c41c390
% hphp trait.php

Fatal error: Unknown trait 'my_trait' [analysis] in /home/rlerdorf/trait.php on line 23

[rlerdorf]

@sgolemon I don't understand the label you added there. This code runs fine under hhvm, but it fails hphp static analysis. That doesn't seem like a php5 incompatibility. It seems like an hhvm/hphp incompatibility to me.

[paulbiss]

It looks like this was always silently causing an analysis error that was ignored until @elgenie introduced 11bd030. I believe it was by design as analyze runs WholeProgram analysis that assumes all code is available at compile time.

As a result this won't ever work with evaled code, and will only work with autoloaded code if the compiler can be trained where to look statically for required classes and traits. One way to do this is to use the AutoloadMap HDF setting as I've demonstrated below.

cc @elgenie and @markw65 for more context on this, and possibly other work arounds.

$ cat analyze/bar.php
<?php

function my_autoloader($class) {
    global $code;
    echo "autoloading $class\n";
    require('analyze/foo.php');
}

spl_autoload_register('my_autoloader');

class try_it {
  use my_trait;
}

try_it::test("does it work?\n");

$ cat analyze/foo.php
<?php

trait my_trait {
  static function test($arg) { echo $arg; }
}

$ hphp -vAutoloadMap.class.my_trait=analyze/foo.php analyze/bar.php
autoloading my_trait
does it work?

[rlerdorf]

Ok, I don't really care about it working with eval(). That was just to make the example single-file.
It seems odd that traits and classes are treated so differently here. I don't need to statically map autoloaded classes, but I do for traits?

Also, I think the difference between hphp2 and hphp3 is more than just an ignored analysis error since hphp2 executed the code correctly and hphp3 doesn't.

[fredemmott]

Sorry, I'm not sure what the problem is - could you put up a multi-file example?

This looks equivalent to me: https://gist.github.com/fredemmott/0bd6cc313da613242f10

And works fine in repo authoritative mode: https://gist.github.com/fredemmott/94ee2d62bde7ded1130c

If you could gist a multi-file test and the commands you're using (or a diff adding a failing test to hhvm), that'd be great.

[fredemmott]

Hmm, so, it analyzises and builds the repo fine, but if you run the analysis by itself, it has problems:

[fredemmott@devbig076 ~/fbcode/hphp/test] HHVM_DISABLE_HHBBC2=1 ../../_bin/hphp/hhvm/hhvm --hphp  --config slow/hphp_config.ini --ffile=hphp/test/slow/autoload/trait.inc --ffile=hphp/test/slow/autoload/trait.php  -tanalyze -l0 -k1  "slow/autoload/trait.php"
hphp/compiler/analysis/analysis_result.cpp:93: ~AnalysisResult: assertion `!m_finish' failed.

Is this the mode you're running in? After talking to others, we don't support -t analyze in isolation any more; the backing code was removed, but we haven't yet removed the option. I'll file a separate issue for that.

[rlerdorf]

Yes, that is the case I am talking about. -t analyze in isolation. I am only interested in running the static analysis part. And it seems to still work quite well other than that part, so which backing code was removed? The intent going forward is to not have a way to run a static analysis in isolation?

[jdelong]

Yeah; going forward -t analyze won't be supported anymore. It was built on the old type inference engine from hphpc, which has been slowly bitrotting and wasn't hooked up to much except for -t analyze, so the maintenance burden didn't seem worthwhile (and we've deleted a lot of that code now---apparently I messed up deleting the option though, oops ...). It would probably be reasonable (and not too hard) to build a new one that can catch the same types of errors on top of the code in hphp/hhbbc, but I don't think there's any immediate plan for that (as hh_client covers most of those types of errors and then some).

[fredemmott]

heh, 'analyze' was removed - we assert on any unrecognized -t flag.

[karpa13a]

hh_server --convert ./ ./
sigfaults(

[paulbiss]

@karpa13a can you please open a separate issue for that with more details?

[karpa13a]

@paulbiss not a problem. 1kk lines of wild php code produce lot of issues)
but i unable to find smth like "-t analyze" in current hhvm/hack(
any article or blog post welcome)

[paulbiss]

There really isn't anything equivalent to -t analyze. If it can parse your code hh_client is actually far more powerful, but unfortunately we don't currently have a replacement that works with vanilla php.

[emanuil]

I'd really love to see -t analyze back. It was really useful for plain PHP projects and the only way we use hhvm.

[fredemmott]

When it was included, it was basically free to support as a side effect of how our optimization worked; given changes in how we optimize code, it would now be an expensive project to develop and maintain, instead of being neatly able to re-use existing code. This makes it unlikely - creating a separate static analysis tool could end up being less work.

[karpa13a]

the latest working -t analyze hhvm is 3.3.5 from repo

[paulbiss]

This feature request is not actively being worked on. We would happily consider a pull request implementing it. If you plan to work on this issue let us know in the comments and we will re-open the issue and assign it to you. If you feel strongly that this issue deserves more attention, please comment here with your use-case and we will try to prioritize as appropriate.