Re: [PATCH v3 1/1] PR11096: Add support for the "module" argument to @var

[Josh Stone <jistone at redhat dot com>]

I have just two small comments, which you can probably just fix without
further review, then I think this is ready to push!

>  void
> +dwarf_var_expanding_visitor::visit_atvar_op (atvar_op *e)
> +{
> +  if (e->module.empty() && e->cu_name.empty())
> +    {
> +      // Fill in the module name so that even if there is no match among
> +      // local variables, dwarf_atvar_expanding_visitor can still scan
> +      // all the CUs in the current module for a global variable match.
> +      e->module = q.dw.module_name;

Add a similar e->module update in sdt_uprobe_var_expanding_visitor.
That's superfluous when an sdt module does have debuginfo, since it will
end up in this dwarf visitor eventually.  But SDT can technically get
resolved without debuginfo, and we don't want any @var from this context
to default to "kernel" in that case.  (It will still fail without
debuginfo, but it should fail with a correct message.)

> +      /* Unable to find the variable in the current module, so we chain
> +       * an error in atvar_op */
> +      semantic_error  er(_F("unable to find global '%s' in %s, %s %s",
> +                            e->sym_name().c_str(), module.c_str(),
> +                            e->cu_name.empty() ? "" : _("in"),
> +                            e->cu_name.c_str()));

This message looks awkward for the empty-cu case, e.g.

[...]unable to find global 'foo' in kernel,  : operator '@var'[...]

I suggest changing "%s, %s %s" to "%s%s%s", and "in" to ", in ".


Thanks,
Josh