ocamlfind icon indicating copy to clipboard operation
ocamlfind copied to clipboard

Some problems with warning/error in findlib

Open chetmurthy opened this issue 3 years ago • 4 comments

Gerd, I'm finding some problems with findlib's warning/error. Here's an example:

ocamlfind ocamlc -verbose -package camlp5.pa_o,camlp5.pr_o -syntax camlp5r -c foo.ml  "
Effective set of preprocessor predicates: preprocessor,syntax,camlp5r
Effective set of compiler predicates: pkg_camlp5.pa_o,pkg_camlp5.pr_o,syntax,autolink,byte

In the META file for camlp5, I have:

package "pa_o" (
  error(camlp5r) = "camlp5.pa_o cannot be used with syntax camlp5r"
...
}

But this is useless, because error/warning are evaluated against compiler predicates, and "camlp5o" is a syntax predicate. I think that, in addition to adding "syntax" to syntax_preds and predicates, it would be useful to add syntax_<syntax> (where <syntax> is the specified syntax, e.g. syntax_camlp5o).

What do you think?

It's a small change, and I can send you a PR, but wanted to get your thoughts first.

P.S. Obviously I can make this change in chetmurthy/not-ocamlfind and it will suffice for many of my needs, but I figured, this is a useful consistency-check for findlib itself. It will make error/warning variables much more useful.

chetmurthy avatar Mar 16 '21 20:03 chetmurthy

I think this is a nice idea, and I would like to merge such a PR.

There is the question whether (1) to keep the two predicate namespaces (where one space is mapped to the other), or (2) to merge the namespaces completely. In (1) you cannot set -predicate syntax_foo but only -syntax foo, and syntax_foo is allowed as selector only. In (2) -predicate syntax_foo would be seen as equivalent to -syntax foo.

I'm not sure which one is more consistent in the end. but I think (2) looks more promising.

gerdstolpmann avatar Mar 16 '21 21:03 gerdstolpmann

Gerd,

Sure, why not merge them? Um, may I make an attempt at a PR which does these two things? That is, my original proposal, plus your #2 ?

ETA: The only problem I see, is that all packages that use syntax predicates will need to be updated. But this is a finite set, and I would expect that we can find them and update them. Maybe one could do it as follows (a staged approach):

(1) make my suggested change, but DO NOT remove syntax predicates. Instead, everywhere that syntax predicates is used, concat syntax_preds and predicates

This will allow all current packages to continue working, but allow to rewrite packages to the new way of doing things.

Release this.

(2) then we can go thru and fix every package that uses preprocessors

(3) and finally a new release that removes syntax_preds.

Thoughts?

chetmurthy avatar Mar 16 '21 21:03 chetmurthy

Let's try (1) and gather experience.

gerdstolpmann avatar Mar 16 '21 21:03 gerdstolpmann

Gotit. So I'll just add more predicates to "predicates" and -not- merge the two sets in further calculations.

chetmurthy avatar Mar 16 '21 22:03 chetmurthy