Opened 21 months ago

Last modified 3 months ago

#280 new defect

Compound Literal Hoisted Out of Function/typeof

Reported by: ajbeach Owned by:
Priority: major Component: cfa-cc
Version: 1.0 Keywords:
Cc:

Description ΒΆ

A simple example of the problem:

forall( T & )
struct proxy {};

forall( T & )
void action( typeof( (proxy(T)){} ) & p ) {
    (void)p;
}
// Becomes (trimmed and cleaned):
proxy(T) _compLit0 = {};
forall(T &)
void action(typeof(_compLit0) &p){
    ((void)p);
}

Because T is a data type and the proxy constructor has no assertions, this actually compiles, it seems adding anything else to it will cause errors. However, it is non-sense and we should not be generating it.

To avoid cases like this, there should not be hoisting out of typeof, sizeof or other non-evaluated contexts. Since they are not evaluated anyways we don't actually need the memory.

If there are other cases where the storage would have to be hoisted out of a function and are evaluated, they may have to forbidden if the constructed type is polymorphic. Or handled specially depending on the case.

Change History (3)

comment:1 Changed 21 months ago by ajbeach

The generated code is invalid (just not in a way that is currently causing any problems) and is (or would be) caught by the new invariant checking code.

I programmed in an exception (in my working code, a merge with it should come soon) so they are not triggered if this happens at a the global level. If this occurs in a nested function, a different pattern is created an a different exception would be needed. Because this happens exactly once across the libraries and tests (array-container/dimexpr-match-cfa), I decided to just remove that case for now.

The fix that allows for the removal of the exception should handle both cases.

comment:2 Changed 3 months ago by ajbeach

I have thought of two possible solutions:

The first is to pass CompoundLiteralExpr through to the resolver without hoisting in non-evaluated contexts (with typeof wrappers inserted, that should always be inside a typeof) and then stop the resolver at those points. In theory, the inside of a typeof is already reduced to a single type, so as long as we get that type correct, it doesn't matter if we fully resolve the expression. A CompoundLiteralExpr's type is explicitly written, so that could be used instead of resolving the expression.

I spent less time on this one, but it did not work. I looked into it less though because A) it occurred to me later and B) it seems more fragile because it makes some important assumptions about non-evaluated contexts and every part of the compiler in that range, from the hoisting pass to the resolver, not conflicting with that.

The second method is to hoist compound literals after the resolver. The non-evaluated contexts should be erased at that point so we could just hoist them as we do now, but it provides more flexibility to do something else. For example, we could check for C-style constructor calls on the compound literal and in those cases, pass them along to C and generate an unmodified compound literal.

I have put work into this solution. It had some interesting problems, but mostly seemed to run into some related resolver issues that may have actually been existing issues that this uncovered, although they could easily have been introduced by other changes as well.

comment:3 Changed 3 months ago by ajbeach

If someone solves this bug, make sure to remove the work around in src/AST/Util.cpp for the checkInvariants pass. There is a lot of code designed to allow exactly this case slip through some checks (including the isInGlobal variable) and all of that could be torn out. It mentions this bug in the comments.

Note: See TracTickets for help on using tickets.