Page MenuHomePhabricator

Fix assertion failure with constant lvalues.
ClosedPublic

Authored by johngarvin on May 15 2015, 5:22 PM.

Details

Reviewers
rsmith
Summary

(Resubmitted with Phabricator for easier reviewing.)

This fixes an assertion failure in assignment if an lvalue is constant but its
type is not const-qualified.

An expression may be MLV_ConstQualified either because the type is actually
“const” or because it’s in the OpenCL constant address space (see
ExprClassification.cpp:608). Unfortunately, isReferenceToConstCapture asserts
that the type must actually be “const”, which is not necessarily true. This
patch changes the assertion to what I think is intended.

Diff Detail

Event Timeline

johngarvin updated this revision to Diff 25908.May 15 2015, 5:22 PM
johngarvin retitled this revision from to Fix assertion failure with constant lvalues..
johngarvin updated this object.
johngarvin edited the test plan for this revision. (Show Details)
johngarvin added a subscriber: Unknown Object (MLST).
rsmith requested changes to this revision.May 15 2015, 6:09 PM
rsmith edited edge metadata.

It seems to me that the bug is using MLV_ConstQualified to represent a situation other than a const qualifier. A better fix would be to add a separate enumeration value for this case.

This revision now requires changes to proceed.May 15 2015, 6:09 PM
johngarvin updated this revision to Diff 26112.May 19 2015, 6:05 PM
johngarvin edited edge metadata.

Add new enumeration value for variables in constant address space.

rsmith accepted this revision.May 19 2015, 6:08 PM
rsmith edited edge metadata.

LGTM

It'd be nice to give a more specific diagnostic for this change, but let's get the assert fix in for now. Do you have commit access?

This revision is now accepted and ready to land.May 19 2015, 6:08 PM

No, no commit access.

John

Ping. Is this ready to commit?

John

rsmith closed this revision.May 21 2015, 6:18 PM

Committed as r237983.