This is an archive of the discontinued LLVM Phabricator instance.

Revert "Implement nullPointerConstant() using a better API."
ClosedPublic

Authored by steveire on Feb 14 2021, 7:37 AM.

Details

Summary

This reverts commit 9148302a (2019-08-22) which broke the pre-existing
unit test for the matcher. Also revert commit 518b2266 (Fix the
nullPointerConstant() test to get bots back to green., 2019-08-22) which
incorrectly changed the test to expect the broken behavior.

Diff Detail

Event Timeline

steveire requested review of this revision.Feb 14 2021, 7:37 AM
steveire created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 14 2021, 7:37 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

I'm a little confused here, Typically if a commit is breaking bots a review doesn't need to be opened to revert it?

njames93 accepted this revision.Feb 20 2021, 9:30 AM
This revision is now accepted and ready to land.Feb 20 2021, 9:30 AM

As discussed elsewhere, the bots are not broken. The changes reverted here were made in 2019.

goncharov added a subscriber: goncharov.EditedFeb 22 2021, 4:46 AM

Hi @steveire! Sorry but I reverted this revert. Could you please give some context of "discussed elsewhere" and "pre-existing unit test for the matcher"? @aaron.ballman for cc

Hi @steveire! Sorry but I reverted this revert. Could you please give some context of "discussed elsewhere" and "pre-existing unit test for the matcher"? @aaron.ballman for cc

@njames93 didn't realize that the offending commits were from 2019, as I wrote in the comment. That's what I told him "elsewhere".

I plan to re-apply this revert. If you have a reason I shouldn't do that, please say what it is.

Hi @steveire! Sorry but I reverted this revert. Could you please give some context of "discussed elsewhere" and "pre-existing unit test for the matcher"? @aaron.ballman for cc

@njames93 didn't realize that the offending commits were from 2019, as I wrote in the comment. That's what I told him "elsewhere".

I plan to re-apply this revert. If you have a reason I shouldn't do that, please say what it is.

@goncharov I see you already re-landed it. In future please open review requests for things like this to avoid confusion.