Feature checks warn about code wrapped in manual version checks

[nirbheek]

If you have code like:

project('foo', meson_version: '>=0.40')
if meson.version().version_compare('>=0.44')
  disabler()
endif

We will still warn about the use of disabler(). One way to fix this would be to have meson.version() return a special object derived from the string object. When you call version_compare() on this object it will return another special object derived from the bool object. When the if statement gets this object, it would 'push' the current version to 0.44 and 'pop' it on endif.

It will be tricky to implement this, and we probably can't add new API for it or people will have to wrap it in a version check which defeats the purpose of all this.

CCing @MathieuDuponchelle @Salamandar

[nirbheek]

Other cases to take care of:

project('foo', meson_version: '>=0.40')
if meson.version().version_compare('<0.44')
  # target meson_version is >=0.40 <0.44 
else
  # target meson_version is >=0.44
endif

The not operator must also be in the loop:

project('foo', meson_version: '>=0.40')
if not meson.version().version_compare('<0.44')
  # target meson_version is >=0.44
else
  # target meson_version is >=0.40 <0.44 
endif

[Salamandar]

Hmm, that's interesting but will complexify things a bit.

[jon-turney]

One way to fix this would be to have meson.version() return a special object derived from the string object. When you call version_compare() on this object it will return another special object derived from the bool object. When the if statement gets this object, it would 'push' the current version to 0.44 and 'pop' it on endif.

Hmmm...

project('foo', meson_version: '>=0.40')
use_disabler = false
if meson.version().version_compare('>=0.44')
  use_disabler = true
endif

if use_disabler
  disabler()
endif

etc. I don't think this is generally solvable without building a static analyser to determine if a statement is executed or not.

Alternatively, I guess you could add something to project() to declare the highest version from which a feature is conditionally used, which defaults to meson_version (the minimum version required), and use that in the feature checks. (which would give the same result, assuming they are always written correctly)

[nirbheek]

etc. I don't think this is generally solvable without building a static analyser to determine if a statement is executed or not.

I agree, but we can tell people that they should not use new features except inside a conditional (or by saving the result of that check) so I would rewrite that as:

project('foo', meson_version: '>=0.40')
use_disabler = meson.version().version_compare('>=0.44')

if use_disabler
  disabler()
endif

[bruce-richardson]

+1 for this feature. Even the basic version, just working with a simple straight version check (i.e. no saved results, etc.) would be of use.

[nirbheek]

Even the basic version, just working with a simple straight version check (i.e. no saved results, etc.) would be of use.

If you save the result of the version check, it will work everywhere else because the return value of the statement will be saved and re-used, so you don't need to do the check repeatedly at least.

Re: what you said on IRC about unknown kwargs being ignored in previous releases, it's hard for us to know whether that's intentional or not, but in the future this can be alleviated with conditional kwargs: #3819.

Finally, it would be ideal if we add this in a stable release so that people don't have to deal with being spammed by warnings, but it's not a blocker for the stable release.

[nirbheek]

0.47.1 is due soon, punted to 0.47.2

[nirbheek]

Another important reason why we need a way for people to have warning-free build files across releases, is that if we start spamming warnings and deprecations too much with no way to take action about them, people will just start ignoring them. Just like compiler warnings.

[Salamandar]

Just like compiler warnings.

Do YOU ignore compiler warnings ? :o

[Salamandar]

Maybe we should warn only when features are used, not parsed. That way, users won't be spammed by features in blocks like

if false
    disabler()
endif

[nirbheek]

Maybe we should warn only when features are used, not parsed

That would also make sense; how do we make that work with a decorator?

[Salamandar]

That would also make sense; how do we make that work with a decorator?

I actually thought that was what would happen. I'll investigate.

[nirbheek]

@Salamandar were you able to figure this out?

[Salamandar]

I didn't have time to work on it. This month is pretty overbooked on my side unfortunately :p

[nirbheek]

Maybe we should warn only when features are used, not parsed. That way, users won't be spammed by features in blocks like

We only warn when features are used, not when they are parsed. The block to not warn in is:

project('foo', meson_version : '>=0.30')
if ensure_version_is_new_enough
  use_new_feature()
endif

Not sure what I was thinking when I agreed with the original statement. ;)

[Salamandar]

What about this ? (version test not in if statement)

meson_is_recent = meson.version().version_compare('>=0.47.0')

if meson_is_recent
  do_something()
endif

And what about that ? (version test is in 'if…or…' statement)

meson_is_recent = meson.version().version_compare('>=0.47.0')
qt5 = dependency('qt5', required: false)

if meson_is_recent or qt5.found()
  do_something()
endif

[nirbheek]

Both those will work with the mechanism I outlined above. Only @jon-turney's example won't work.

[xclaesse]

Static analyser seems really really hard solution. A trivial-but-ugly solutiuon would be

project('foo', meson_version: '>=0.40')
if meson.version().version_compare('>=0.44')
  begin_ignore_warnings()
  disabler()
  end_ignore_warnings()
endif

or with a codeblock syntax somehow... That would be similar to G_GNUC_BEGIN_IGNORE_DEPRECATIONS and G_GNUC_END_IGNORE_DEPRECATIONS we have in GLib.

It could also be handy with the upcoming --fatal-warnings command line option I'm proposing in PR #4006, to avoid aborting on well-known warnings that devs explicitly want to ignore.

[Salamandar]

On the same path, a new syntax could be:

formesonversion('>=0.44')
  feature()
endformesonversion

@nirbheek Okay, but what about that then ?

mydep = meson.version().version_compare('>=0.44') ? disabler : dependency('', required: false)

[nirbheek]

I am not sure if we should add a way to disable warnings whole-sale, because people will then be surprised when their build files break when we finally do change behaviour.

I mean, the only case where ignoring deprecation warnings makes sense is when it's in an if meson.version conditional, and if we can detect that case and disable warnings, that's a better tool than a hammer.

@nirbheek Okay, but what about that then ?

Sounds like the ternary operator can/should also implement this.

From what I can tell, whether we use this method or not depends on how ugly/invasive the implementation is on the interpreter end. If no one else wants to take a crack at it, I'll see what I can whip up.

[eli-schwartz]

Fixed in #7594