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

[cling] Remove unneed assert() in DynamicLookup.cpp #9245

Merged
merged 1 commit into from
Nov 4, 2021
Merged

[cling] Remove unneed assert() in DynamicLookup.cpp #9245

merged 1 commit into from
Nov 4, 2021

Conversation

jalopezg-git
Copy link
Contributor

@jalopezg-git jalopezg-git commented Nov 4, 2021 •
edited
Loading

The case of ExprAddresses == nullptr seems to be naturally handled in the lines below this assertion, which suggests that it is okay if the asserted predicate does not hold.

Therefore, removing this assertion -as discussed with @vgvassilev in the linked issue- see here.

Changes or fixes:

  • Remove unneded assert(), as discussed above.

Checklist:

  • tested changes locally

This PR fixes #8389.

@jalopezg-git
[cling] Remove unneed assert()
374c30b 
The case of `ExprAddresses == nullptr` seems to be naturally handled in the
lines below. Therefore, removing this assertion -as discussed with vvassilev.

Closes issue root-project#8389.
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@jalopezg-git jalopezg-git self-assigned this Nov 4, 2021
@phsft-bot
Copy link
Collaborator

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-2.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@jalopezg-git
Copy link
Contributor Author

Failing tests:

* [projectroot.tree.dataframe.test.gtest_tree_dataframe_test_dataframe_definepersample](https://lcgapp-services.cern.ch/root-jenkins/job/root-pullrequests-build/129185/testReport/projectroot.tree.dataframe/test/gtest_tree_dataframe_test_dataframe_definepersample/)

* [projectroot.tree.dataframe.test.gtest_tree_dataframe_test_dataframe_samplecallback](https://lcgapp-services.cern.ch/root-jenkins/job/root-pullrequests-build/129185/testReport/projectroot.tree.dataframe/test/gtest_tree_dataframe_test_dataframe_samplecallback/)

These failures are (obviously) unrelated.

vgvassilev
vgvassilev approved these changes Nov 4, 2021
View reviewed changes
Copy link
Member

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jalopezg-git jalopezg-git merged commit 504b9dc into root-project:master Nov 4, 2021
@jalopezg-git jalopezg-git deleted the fix-issue-8389 branch November 4, 2021 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vgvassilev vgvassilev vgvassilev approved these changes

@Axel-Naumann Axel-Naumann Awaiting requested review from Axel-Naumann

@eguiraud eguiraud Awaiting requested review from eguiraud

Assignees

@jalopezg-git jalopezg-git

Labels
affects:master in:Cling
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cling assertion crash
3 participants
@jalopezg-git @phsft-bot @vgvassilev