Skip to content

Continuing work on expressions#2563

Merged
ktf merged 3 commits intoAliceO2Group:devfrom
aalkin:new-expressions
Nov 2, 2019
Merged

Continuing work on expressions#2563
ktf merged 3 commits intoAliceO2Group:devfrom
aalkin:new-expressions

Conversation

@aalkin
Copy link
Copy Markdown
Member

@aalkin aalkin commented Oct 31, 2019

  • Reworked tree-parsing algorithm to use ArrowDatumSpec
  • Added a test of tree-parsing algorithm

@aalkin aalkin requested a review from a team as a code owner October 31, 2019 09:39
@aalkin
Copy link
Copy Markdown
Member Author

aalkin commented Oct 31, 2019

There is QualityControl build log (with error) included in the failing build logs, is it supposed to be there?

@ktf
Copy link
Copy Markdown
Member

ktf commented Oct 31, 2019

No, but it's there everywhere since yesterday. I could not yet understand what changed yet.

@aalkin
Copy link
Copy Markdown
Member Author

aalkin commented Oct 31, 2019

I see that the template instantiation errors in the QC log are definitely connected with Expressions: it picks up the variant from inside ArrowDatumSpec, which is variant<monostate,size_t,variant<int,bool.float,double>,string>, but I can not determine in which context.

@ktf
Copy link
Copy Markdown
Member

ktf commented Oct 31, 2019

The problem itself is due to the fact that the clang used by cling (5.0.2) has a known bug when using GCC headers < 8. The way to fix it is to hide whatever uses variant from cling. Why do you need to expose the ArrowDatum in Expression? Can you simply put an #ifdef __CLING__ around the variant and the header?

* Reworked tree-parsing algorithm to use ArrowDatumSpec
* Added a test of tree-parsing algorithm
@aalkin
Copy link
Copy Markdown
Member Author

aalkin commented Nov 2, 2019

@ktf I moved the parsing helpers into a separate header that is not exposed in include, that should hide variant from cling.

@ktf
Copy link
Copy Markdown
Member

ktf commented Nov 2, 2019

Errors unrelated. This hides variant from ROOT.

@ktf ktf merged commit a59434b into AliceO2Group:dev Nov 2, 2019
@aalkin aalkin deleted the new-expressions branch November 2, 2019 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants