Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Why <metas> attribute in XMIR is optional? #1522

Closed
MikhailLipanin opened this issue Dec 2, 2022 · 14 comments · Fixed by #1783
Closed

Why <metas> attribute in XMIR is optional? #1522

MikhailLipanin opened this issue Dec 2, 2022 · 14 comments · Fixed by #1783
Assignees
@mximp
Labels
bug

Comments

@MikhailLipanin
Copy link
Member

MikhailLipanin commented Dec 2, 2022 •
edited
Loading

I have this program:

[] > main
  QQ.io.stdout > @
    QQ.txt.sprintf "ans is %d"
      13

Since <metas> is optional attribute in XMIR now, and in the program above there is no alias-es and probe-s (and others), there is no <metas/> section in final XMIR.
But work of add-probes.xsl depends on <metas> section and without it, XSL will not work.
So, if I add simple meta --- package, it will work:

+package org.eolang.custom

[] > main
  QQ.io.stdout > @
    QQ.txt.sprintf "ans is %d"
      13

So, we can make <metas> section is mandatory in XMIR-s or rewrite add-probe.xsl and all others XSL-s (if they are exist).
@MikhailLipanin
Copy link
Member Author

@yegor256 WDYT?

@yegor256
Copy link
Member

yegor256 commented Dec 4, 2022

@MikhailLipanin I didn't understand the problem. What doesn't work now?

@MikhailLipanin
Copy link
Member Author

MikhailLipanin commented Dec 4, 2022 •
edited
Loading

When I have this program:

[] > main
  QQ.io.stdout > @
    QQ.txt.sprintf "ans is %d"
      13

I get the following XMIR: (Note that here there is no <metas> section)

<program>
  <listing>
    ...
  </listing>
  <errors>
    <error check="missing-package"
              line=""
              severity="warning"
              sheet="mandatory-package-meta"
              step="0">Missing package</error>
  </errors>
  <sheets>
    ...
  </sheets>
  <objects>
    ...
  </objects>
</program>

But when I have this program (the only difference in +package org.eolang.custom):

+package org.eolang.custom

[] > main
  QQ.io.stdout > @
    QQ.txt.sprintf "ans is %d"
      13

I get the following XMIR:

<program>
  <listing>
    ...
  </listing>
  </errors>
  <sheets>
    ...
  </sheets>
  <metas>
    <meta line="1">
      <head>package</head>
      <tail>org.eolang.custom</tail>
      <part>org.eolang.custom</part>
    </meta>
    <meta line="4">
      <head>probe</head>
      <tail>Q.org.eolang.io.stdout</tail>
      <part>Q.org.eolang.io.stdout</part>
    </meta>
    <meta line="4">
      <head>probe</head>
      <tail>Q.org.eolang.io</tail>
      <part>Q.org.eolang.io</part>
    </meta>
    <meta line="5">
      <head>probe</head>
      <tail>Q.org.eolang.txt.sprintf</tail>
      <part>Q.org.eolang.txt.sprintf</part>
    </meta>
    <meta line="5">
      <head>probe</head>
      <tail>Q.org.eolang.txt</tail>
      <part>Q.org.eolang.txt</part>
    </meta>
    <meta line="5">
      <head>probe</head>
      <tail>Q.org.eolang</tail>
      <part>Q.org.eolang</part>
    </meta>
    <meta line="5">
      <head>probe</head>
      <tail>Q.org</tail>
      <part>Q.org</part>
    </meta>
  </metas>
  <objects>
    ...
  </objects>
</program>

Note, that the difference here is in <metas> section.
The question is: why probes is added only if there is another meta (like +package for example) ?

I think it is because some XSL - templates depends on <metas> section of XMIR (for example if there is <template match="//metas...")

So, we can make <metas> section is mandatory in XMIR-s or rewrite add-probe.xsl and all others XSL-s (if they are exist).

@yegor256 FYI

@yegor256
Copy link
Member

yegor256 commented Dec 4, 2022

@MikhailLipanin why it's a problem? Yes, this is how add-probes.xsl works. Why it's a problem for you?

@MikhailLipanin
Copy link
Member Author

@yegor256 Because we don't correctly add probes for objects if there is no other metas

@MikhailLipanin
Copy link
Member Author

So in first example, I can't say, what is QQ.io.stdout without its probe

@yegor256
Copy link
Member

yegor256 commented Dec 4, 2022

@MikhailLipanin try to always report a bug in this format: 1) I do this, 2) I expect this result, 3) I'm getting this instead.

@MikhailLipanin
Copy link
Member Author

MikhailLipanin commented Dec 4, 2022 •
edited
Loading

Here, I expected in both examples of EO-programs the second XMIR, but get it only in second example

@yegor256 yegor256 added the bug label Dec 4, 2022
@mximp
Copy link
Contributor

mximp commented Dec 5, 2022

@pollyvolk please assist

@mximp mximp removed their assignment Dec 5, 2022
@mximp
Copy link
Contributor

mximp commented Dec 5, 2022

@pollyvolk @MikhailLipanin @yegor256 I believe metas element should be added by default just like it's done for errors (see XeListener in eo-parser).

@yegor256
Copy link
Member

yegor256 commented Dec 5, 2022

@mximp I don't mind, let's make it mandatory

@mximp
Copy link
Contributor

mximp commented Dec 5, 2022

@mximp I don't mind, let's make it mandatory

Just to make it clear: it will not be mandatory from EO language point of view but rather <metas> tag will be added into XMIR by default (even with no metas inside).

@MikhailLipanin
Copy link
Member Author

This one is related to #1323

@yegor256
Copy link
Member

@mximp let's make it mandatory in XMIR (let's update xmir.xsd for that too). In the language syntax it's of course optional

@mximp mximp assigned mximp and unassigned pollyvolk Jan 20, 2023
mximp added a commit to mximp/eo that referenced this issue Feb 10, 2023
@mximp
objectionary#1522 always add meta to XMIR
7ec527d
@mximp mximp mentioned this issue Feb 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mximp mximp

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

#1522 always add metas to XMIR mximp/eo
4 participants
@yegor256 @pollyvolk @mximp @MikhailLipanin