Created attachment 114523 [details] [review]
Here is a patch for the problem that tries to do minimal changes to the interface

I've made Report a class, renamed error and warning to err and warn respectively, added a report (construct) property in CodeContext, and added two static methods to Report error and warning that can be used like the old error and warning and call the appropriate instance's err and warn.

The instance is obtained from the context referenced by the SourceReference passed to error or warning.

I've also modified parsers, valac, vapigen, and ccodegenerator to use the new interface for get_errors (since it cannot be made static).

Created attachment 114720 [details] [review]
Updated patch : Report.{error,warning} accept only non null source reference

I've also modified all "Report.error (null, ..." to use context.report.err, there remains only three instances that cannot use it because they are in static methods, any ideas about it ?

vala/valasemanticanalyzer.vala:1914
gobject/valaccodecompiler.vala:40
gobject/valaccodegenerator.vala:3976

At least it applies on compiles.

Thanks for the patch. We can probably change them to instance methods.

I'm not sure this is the right way to do it, maybe internal errors (I think this is the only use of Report.error (null, ...)) should use the default instance.

Any news on this?

I just thought about an alternative solution. What about keeping a context stack around and always report errors and warnings to the current (i.e. topmost) context?

class CodeContext {
    ...
    Report report;
    static List<CodeContext> stack;

    static CodeContext get () {
        return stack.get (stack.size - 1);
    }

    static void push (CodeContext context) {
        stack.add (context);
    }

    static void pop () {
        stack.remove_at (stack.size - 1);
    }
}

class Report {
    ...
    static void error (...) {
        CodeContext.get ().report.err (...);
    }
}

Whenever you want to switch context you do:

CodeContext.push (other_context);
// call other methods
CodeContext.pop ();

This should be flexible and still easy to use. Would this be ok for your usecase?

Seems ok to me, All I need is getting error reports for the code context I'm interested in to report them in the GUI.

I'm highly interested in seeing Vala support in Anjuta, and I'd love to try out Abderrahim's language support plugin described at http://abderrahim.arablug.org/blog/ .  Unfortunately that plugin depends on the patches attached to this bug, and those patches were made last July, so I can't easily apply them to a current version of Vala.

So Vala support in Anjuta is effectively blocked on this bug.  Abderrahim, are you planning to create an updated version of the patches based on Jurg's suggestion above?  If you have no time for this, please let us know and perhaps someone else can help out.

Other possible solutions:

- We could keep a stack of Report objects around and simply always report to the topmost Report.

- We could simply have a global Report object which can be replaced if the caller wants custom error handling.

None of these solutions (including the CodeContext stack) are thread-safe; they will all fail if more than one thread is trying to invoke the Vala compiler at the same time.  Then again, the compiler evidently wasn't intended to be thread-safe anyway since the code uses statics in many places.

Created attachment 127179 [details] [review]
fix

Here's a fix that implements the simplest of my suggestions above: it allows the caller to provide a global Report object for custom error handling.  This is a very small change, and should be adequate for the Anjuta plugin's needs.  I personally see no reason to implement a stack of Report or CodeContext objects; that still wouldn't be thread-safe, and I'm not aware that any libvala caller requires such a capability.

Note that even with this patch, the Anjuta plugin still won't build; it's fallen out of date with the Vala codebase for other reasons unrelated to Report objects (e.g. it expects a method Vala.Scope.get_symbol_table(), which does not presently exist).  I hope we can bring the plugin back to life soon.

Hi Adam,

I've already fixed the API change, I'll try to push it tomorrow.
About get_symbol_table, I thought I've already sent a patch for it. seems I didn't.
I'll try to post everything needed to have something working tomorrow.

The solution I suggest is something like g_log where one would add a callback to handle the error report.

Created attachment 127384 [details] [review]
latest diff

Here is the diff of what I'm using now, you should be able to compile and run the plugin with this and the latest revision from hg

Created attachment 128162 [details] [review]
g_log-like solution

Here is a patch trying to solve the issue in another way, it doesn't make
Vala.Report a type, instead one just registers a callback.

2009-02-20  Jürg Billeter  <j@bitron.ch>

	* vala/valacodecontext.vala:
	* vala/valareport.vala:

	Support context-specific error reporting and add a context stack,
	based on patch by Abderrahim Kitouni, fixes bug 542920

	* vala/valagenieparser.vala:
	* vala/valaparser.vala:
	* gobject/valaccodebasemodule.vala:
	* compiler/valacompiler.vala:
	* vapigen/valavapigen.vala:

	Adapt to interface changes

Fixed in r2461.

I wish I've seen this bug before, since I'm a multithread user of libvala and this change has a severe impact on the completion engine of Vala Toys for gEdit.

Can we rethink all of this along the lines of comment #14 ?

I used libvala for some time now with two thread and at least the Parser seem thread safe. For Vtg it's pretty useful to use one thread to parse the sources that change frequently during the editing (e.g. the opened ones) and another for all the other project sources and referenced vapis.