Page MenuHomePhabricator

eolian: add support for marking type declarations beta
ClosedPublic

Authored by q66 on Mar 5 2019, 7:28 AM.

Details

Summary

This also simplifies the beta checking API by unifying it under
objects (makes much more sense that way) and reworks the validator
to have betaness support within its context state, allowing checks
to be done easily in any place.

The betaness checks are disabled for types for the time being,
because otherwise there are too many errors (types are assumed
to be stable as they are not tagged beta, but they reference beta
classes all over the place). Set EOLIAN_TYPEDECL_BETA_WARN to 1
in your environment to force enable the checks.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
q66 created this revision.Mar 5 2019, 7:28 AM
q66 requested review of this revision.Mar 5 2019, 7:28 AM
q66 added a comment.Mar 5 2019, 7:29 AM

I know this changes Eolian API so since we're in a freeze it's probably not 100% ideal to merge this, but we need this to be able to stabilize at least something at all.

q66 added a project: Restricted Project.Mar 5 2019, 8:13 AM
bu5hm4n requested changes to this revision.Mar 6 2019, 10:10 AM

The validation code does not work. If i make every type in efl_object.eo. The checks still complain that
eolian: efl_object.eo:420:13: beta class 'Efl.Object' used in stable context.

However, the test code

type @beta atype : int;

class class1 {
  methods {
    amethod {
      return: atype;
    }
  }
}

Does not complain at all. Sooo something is pretty wrong there IMO. I can also not find the place where the struct field types are checked so that the struct is not using beta types if it is not beta.

On the other side, this patches misses the complete generator support, types that i declare @beta are still available without a #define, which is kind of a problem.

This revision now requires changes to proceed.Mar 6 2019, 10:10 AM
q66 added a comment.Mar 7 2019, 5:52 AM

Struct fields are checked implicitly by design just like all other types, it does not need any specific code. When a non-beta function/typedecl/whatever is encountered, the validator switches state into "stable" mode and any dependent types (which includes anything in struct fields) are required to be non-beta. I will investigate the other stuff.

q66 added a comment.Mar 7 2019, 6:05 AM

There is also nothing wrong with the test results of your test, that is fully intended; the class is not in the Efl. namespace, therefore it's not treated as a stable class. Putting it in the Efl. namespace makes it warn normally:

eolian: efl_test.eo:6:15: beta type declaration 'atype' used in stable context
q66 added a comment.Mar 7 2019, 6:11 AM

Also, I can't reproduce your thing with efl_object.eo. As it is right now, efl_object.eo parses fine out of box with validation. Give me a test case please

efl_object.eo.h file which does not generate here: https://phab.enlightenment.org/P272.

Why are only Efl. things validated? The only reason i added this to the class checking was to enable it unconditionally, so everyone sees when he made a mistake, doing this in this check here is useless since the tree is right now in the process of getting rid of legacy and the check is hidden behind a env var anyways ... which means, within a few weeks everyone will have forgotten about it (But thats a different topic)

q66 updated this revision to Diff 20152.Mar 7 2019, 8:10 AM

added generator support, fixed parser bug, rebase

zmike requested changes to this revision.Mar 7 2019, 12:20 PM

Does not work for function types, e.g., function @beta Efl.Dnd.Drag_Icon_Create { errors.

This revision now requires changes to proceed.Mar 7 2019, 12:20 PM
q66 updated this revision to Diff 20172.Mar 7 2019, 12:28 PM

added support for marking function types beta

q66 updated this revision to Diff 20198.Mar 7 2019, 2:20 PM

rebase

q66 updated this revision to Diff 20258.Mar 7 2019, 2:45 PM

can't enable beta guards for types yet, as it triggers too much
brokenness which we can't fix in time

q66 updated this revision to Diff 20283.Mar 8 2019, 4:36 AM

rebase

zmike accepted this revision.Mar 8 2019, 5:14 AM
This revision was not accepted when it landed; it landed in state Needs Review.Mar 8 2019, 5:35 AM
This revision was automatically updated to reflect the committed changes.