Major rewrite of the rewriter and the static introspection tool

[Volker-Weissmann]

The rewriter and the static introspection tool used to be very broken, now it is less broken.

The most important changes are:

1. We now have class UnknownValue for more explicit handling of situations that are too complex/impossible.
2. If you write

var = 'foo'
name = var
var = 'bar'
executable(name, 'foo.c')

the tool now knows that the name of the executable is foo and not bar. See dataflow_dag and node_to_runtime_value for details on how we do this.

To test my work I wrote a script that:

1. git clone's a couple of big projects using meson (e.g. systemd).
2. Checks if the outputs of meson introspect meson.build --targets and meson introspect build_folder --targets are not contradicting each other.
3. Checks if the output of meson introspect meson.build --targets is the same for two different versions of meson (the one we want to test and a known good version).
4. Iterates over all targets that are found using meson introspect meson.build --targets and checks if

meson rewrite target tgt add rewrite_test_source.c
meson rewrite target tgt add rewrite_test_source.c
meson introspect meson.build --targets
meson rewrite target tgt rm rewrite_test_source.c
meson introspect meson.build --targets

does not crash and produces the expected output.

I think this script is very useful (it found a ton of bugs), but I do not know where to put it, so it currently only exists on my machine. It is too slow (1 hour iirc, haven't measured) to run it in the CI pipeline. In the docs you write

All the software on this list is tested for regressions before release

Is this testing (partially) automated? If so, could we merge my script with it?

@bruchar1 @kcgen Afaik you are one of the few people using the rewriter/static introspection tool in production. Could you test your usecases and voice your opinion?

@eli-schwartz You promised me this in a mail:

I would be willing to review the resulting PRs with the intent of checking that the results "seem to work" and not getting bogged down in nitty-gritty details. 🙂 I agree that it's relatively obscure functionality that doesn't always work and the priority should be, essentially, treating it as brand new code.

[bruchar1]

Thank you for working on this. I will try to have a deeper look and to test it soon.

For the tests, I think the best thing to do is to ensure that each bug you found is covered by a test.

I wonder if this could be splitted into multiple commits, to ease understanding what each part fixes, especially for trivial fixes that are not directly part of the refactor.

[Volker-Weissmann]

Thank you for working on this. I will try to have a deeper look and to test it soon.

For the tests, I think the best thing to do is to ensure that each bug you found is covered by a test.

I should have done that everytime it found a bug. I cannot do it now, since I forgot most of the bugs.

I wonder if this could be splitted into multiple commits, to ease understanding what each part fixes, especially for trivial fixes that are not directly part of the refactor.

I will do that for "trivial fixes that are not directly part of the refactor" , but the rest will still be in one big commit since it is hard/impossible to split.

[Volker-Weissmann]

I splitted the trivial fixes intro seperate commits.

[GranMinigun]

Cloned this branch, ran introspect --all on DOSBox Staging's meson.build (that's what e.g. Qt Creator does), it happily crashed:

Unable to evaluate subdir([<mesonbuild.interpreterbase.baseobjects.UnknownValue object at 0x7f7d6651e490>]) in AstInterpreter --> Skipping
Traceback (most recent call last):
  File "/dev/shm/meson/mesonbuild/mesonmain.py", line 194, in run
    return options.run_func(options)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dev/shm/meson/mesonbuild/mintro.py", line 541, in run
    return print_results(options, results, indent)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dev/shm/meson/mesonbuild/mintro.py", line 501, in print_results
    print(json.dumps(out, indent=indent))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 200, in encode
    chunks = self.iterencode(o, _one_shot=True)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 258, in iterencode
    return _iterencode(o, 0)
           ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 180, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type UnknownValue is not JSON serializable

ERROR: Unhandled python exception

    This is a Meson bug and should be reported!

Python 3.11.5.

[Volker-Weissmann]

Cloned this branch, ran introspect --all on DOSBox Staging's meson.build (that's what e.g. Qt Creator does), it happily crashed:

Confirmed and on my way to fix it.

[Volker-Weissmann]

Cloned this branch, ran introspect --all on DOSBox Staging's meson.build (that's what e.g. Qt Creator does), it happily crashed:

Unable to evaluate subdir([<mesonbuild.interpreterbase.baseobjects.UnknownValue object at 0x7f7d6651e490>]) in AstInterpreter --> Skipping
Traceback (most recent call last):
  File "/dev/shm/meson/mesonbuild/mesonmain.py", line 194, in run
    return options.run_func(options)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dev/shm/meson/mesonbuild/mintro.py", line 541, in run
    return print_results(options, results, indent)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dev/shm/meson/mesonbuild/mintro.py", line 501, in print_results
    print(json.dumps(out, indent=indent))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 200, in encode
    chunks = self.iterencode(o, _one_shot=True)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 258, in iterencode
    return _iterencode(o, 0)
           ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 180, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type UnknownValue is not JSON serializable

ERROR: Unhandled python exception

    This is a Meson bug and should be reported!

Python 3.11.5.

Fixed in b98292e7edfd8c0f6f6289b395766b32cbbc8540

[Volker-Weissmann]

Any update?

[bonzini]

Hi! Would you be able to rebase the PR?

[Volker-Weissmann]

Hi! Would you be able to rebase the PR?

I could but only if a maintainer says that it gets merged after rebasing.

[bonzini]

Not a maintainer but I will try to give it a quick review beforehand, at least.

[bonzini]

It looks sane. It's a huge commit, but that can be fixed after conflicts are resolved.

[Volker-Weissmann]

We still need someone with merge permissions.

[bonzini]

Well as long as it has conflicts no one will look at it.

[dcbaker]

I'm willing to defer to @bonzini on the heavy lifting of review. If he's happy and we can resolve the UknownValues thing, I can do the merge.

[Volker-Weissmann]

I'm willing to defer to @bonzini on the heavy lifting of review. If he's happy and we can resolve the UknownValues thing, I can do the merge.

In that case, I will work on the rebasing and the UnknownValues thing in a few weeks.

[Volker-Weissmann]

Well as long as it has conflicts no one will look at it.

It has no conflicts and the pipeline is all green. I hope you'll find the time to do the review.

[bruchar1]

The fourth commit is huge. Would you be able to split it into manageable chunks? It is difficult to me to get the big picture, but some suggestions for the split (now sure of the logical order):

• Introduce UnknownValue
• Add variables handling
• Fix type checking

[julianneswinoga]

Came across this PR (I'm trying to do some more advanced AST parsing), FYI it's crashing for me:

$ ~/tools/meson/meson.py introspect --ast meson.build
Traceback (most recent call last):
  File "/home/jswinoga/tools/meson/mesonbuild/mesonmain.py", line 193, in run
    return options.run_func(options)
  File "/home/jswinoga/tools/meson/mesonbuild/mintro.py", line 540, in run
    intr.analyze()
  File "/home/jswinoga/tools/meson/mesonbuild/ast/introspection.py", line 355, in analyze
    self.parse_project()
  File "/home/jswinoga/tools/meson/mesonbuild/interpreterbase/interpreterbase.py", line 129, in parse_project
    self.evaluate_codeblock(self.ast, end=1)
  File "/home/jswinoga/tools/meson/mesonbuild/interpreterbase/interpreterbase.py", line 194, in evaluate_codeblock
    raise e
  File "/home/jswinoga/tools/meson/mesonbuild/interpreterbase/interpreterbase.py", line 186, in evaluate_codeblock
    self.evaluate_statement(cur)
  File "/home/jswinoga/tools/meson/mesonbuild/ast/interpreter.py", line 820, in evaluate_statement
    super().evaluate_statement(cur)
  File "/home/jswinoga/tools/meson/mesonbuild/interpreterbase/interpreterbase.py", line 200, in evaluate_statement
    return self.function_call(cur)
  File "/home/jswinoga/tools/meson/mesonbuild/ast/interpreter.py", line 823, in function_call
    ret = super().function_call(node)
  File "/home/jswinoga/tools/meson/mesonbuild/interpreterbase/interpreterbase.py", line 530, in function_call
    res = func(node, func_args, kwargs)
  File "/home/jswinoga/tools/meson/mesonbuild/ast/introspection.py", line 127, in func_project
    def_opts = self.flatten_args(kwargs.get('default_options', []))
  File "/home/jswinoga/tools/meson/mesonbuild/ast/interpreter.py", line 790, in flatten_args
    raise NotImplementedError
NotImplementedError

meson.build:1:0: ERROR: Unhandled python exception

    This is a Meson bug and should be reported!

---

Side note - in stable meson there doesn't seem to be a way to get the AST of subdir() calls? Asking --ast to parse the top-level doesn't recurse into subdir() calls, and asking --ast to parse individual meson.build files complain about a missing project() call...

[Volker-Weissmann]

Came across this PR (I'm trying to do some more advanced AST parsing), FYI it's crashing for me:

$ ~/tools/meson/meson.py introspect --ast meson.build
Traceback (most recent call last):
  File "/home/jswinoga/tools/meson/mesonbuild/mesonmain.py", line 193, in run
    return options.run_func(options)
  File "/home/jswinoga/tools/meson/mesonbuild/mintro.py", line 540, in run
    intr.analyze()
  File "/home/jswinoga/tools/meson/mesonbuild/ast/introspection.py", line 355, in analyze
    self.parse_project()
  File "/home/jswinoga/tools/meson/mesonbuild/interpreterbase/interpreterbase.py", line 129, in parse_project
    self.evaluate_codeblock(self.ast, end=1)
  File "/home/jswinoga/tools/meson/mesonbuild/interpreterbase/interpreterbase.py", line 194, in evaluate_codeblock
    raise e
  File "/home/jswinoga/tools/meson/mesonbuild/interpreterbase/interpreterbase.py", line 186, in evaluate_codeblock
    self.evaluate_statement(cur)
  File "/home/jswinoga/tools/meson/mesonbuild/ast/interpreter.py", line 820, in evaluate_statement
    super().evaluate_statement(cur)
  File "/home/jswinoga/tools/meson/mesonbuild/interpreterbase/interpreterbase.py", line 200, in evaluate_statement
    return self.function_call(cur)
  File "/home/jswinoga/tools/meson/mesonbuild/ast/interpreter.py", line 823, in function_call
    ret = super().function_call(node)
  File "/home/jswinoga/tools/meson/mesonbuild/interpreterbase/interpreterbase.py", line 530, in function_call
    res = func(node, func_args, kwargs)
  File "/home/jswinoga/tools/meson/mesonbuild/ast/introspection.py", line 127, in func_project
    def_opts = self.flatten_args(kwargs.get('default_options', []))
  File "/home/jswinoga/tools/meson/mesonbuild/ast/interpreter.py", line 790, in flatten_args
    raise NotImplementedError
NotImplementedError

meson.build:1:0: ERROR: Unhandled python exception

    This is a Meson bug and should be reported!

Can you share your meson.build files so that I can reproduce the bug?

Side note - in stable meson there doesn't seem to be a way to get the AST of subdir() calls? Asking --ast to parse the top-level doesn't recurse into subdir() calls, and asking --ast to parse individual meson.build files complain about a missing project() call...

I think it's best if you open an issue (and @mention me) after this PR is merged.

[julianneswinoga]

Sorry, unfortunately I can't share the build files

[Volker-Weissmann]

Sorry, unfortunately I can't share the build files

You can try to fix the bug and make a PR

[Volker-Weissmann]

The fourth commit is huge. Would you be able to split it into manageable chunks? It is difficult to me to get the big picture, but some suggestions for the split (now sure of the logical order):

* Introduce UnknownValue

* Add variables handling

* Fix type checking

I finally managed to split the 4 commits into 23 commits. Hope you'll find the time to review them.

[Volker-Weissmann]

And if you just find the time to review the first n commits, We might consider merging just the first n commits, because otherwise the constant rebasing will be quite some work.

[Volker-Weissmann]

@bruchar1 Any update?

[bonzini]

Ok with 1.8 settled it's time to look at this one. Please rebase, hopefully for the last time.

[Volker-Weissmann]

Ok with 1.8 settled it's time to look at this one. Please rebase, hopefully for the last time.

Just did that. The two test failures are note related. Good luck reviewing.

@bonzini

[Volker-Weissmann]

The last commit is odd, since you are fixing a bug you introduced in a previous commit... You should probably squash it with the commit that introduced UnknownValue.

I deliberately put this commit after 9461cbc4 to have a test that succeeds after this commit, but fails before this commit. I think having that is valuable if one has doubts whether this commit is useful/necessary at all.

(If I put the last commit before 9461cbc4 , then running the rewriter in systemd fails for different reason. )

[dcbaker]

I'm fine with a failing test in between, I haven't looked at the implementation, but it would be great if we can structure things so that the test suite succeeds across the series, ie, using unttests.expectedFailure or putting thins in test cases/failures/ and then moving it when we expect it to pass.

[Volker-Weissmann]

I'm fine with a failing test in between, I haven't looked at the implementation, but it would be great if we can structure things so that the test suite succeeds across the series, ie, using unttests.expectedFailure or putting thins in test cases/failures/ and then moving it when we expect it to pass.

Afaik, I did it the way you described. The "bug that is introduced and then fixed in the last commit" does not break any test case in the meson repository, it breaks running the introspection tool in the systemd repository, i.e. a manual test.

[dcbaker]

@Volker-Weissmann then I'm perfectly happy. I personally like introducing tests with an expected fail status and then fixing them (see the Fortran dependency scanner work), I think there is value in seeing "Look, this doesn't work" and then "See, I fixed it"