This is an archive of the discontinued LLVM Phabricator instance.

[Sema] Mark GNU compound literal array init as an rvalue.
ClosedPublic

Authored by efriedma on Feb 11 2019, 12:38 PM.

Details

Summary

Basically the same issue as string init, except it didn't really have any visible consequences before I removed the implicit lvalue-to-rvalue conversion from CodeGen.

While I'm here, a couple minor drive-by cleanups: IgnoreParens never returns a ConstantExpr, and there was a potential crash with string init involving a ChooseExpr.

The analyzer test change maybe indicates we could simplify the analyzer code a little with this fix? Apparently a hack was added to support lvalues in initializers in r315750, but I'm not really familiar with the relevant code.

Fixes regression reported in the kernel build at https://bugs.llvm.org/show_bug.cgi?id=40430#c6 .

Diff Detail

Repository
rC Clang

Event Timeline

efriedma created this revision.Feb 11 2019, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2019, 12:38 PM
rsmith accepted this revision.Feb 11 2019, 2:11 PM
rsmith added inline comments.
lib/Sema/SemaInit.cpp
196–197

I guess this is just for UO_Extension. Is it worth asserting that?

This revision is now accepted and ready to land.Feb 11 2019, 2:11 PM
efriedma marked 2 inline comments as done.Feb 11 2019, 2:48 PM
efriedma added inline comments.
lib/Sema/SemaInit.cpp
196–197

I don't think it's likely that anyone would ever trip the assertion, but maybe worthwhile just to document the meaning, sure.

This revision was automatically updated to reflect the committed changes.
efriedma marked an inline comment as done.
NoQ added a subscriber: NoQ.Feb 11 2019, 3:07 PM

The analyzer test change maybe indicates we could simplify the analyzer code a little with this fix? Apparently a hack was added to support lvalues in initializers in r315750, but I'm not really familiar with the relevant code.

Nope, unfortunately, not much can be simplified. This hack is much deeper and covers many more cases than that and was added much earlier :) But thanks for attracting my attention to this!