Make sizeof expression context partially evaluated

[pmatos]

Summary

Ensure sizeof expression context is partially evaluated so that potential typeof operators inside are evaluated if necessary.

Fixes PR31042.

[EricWF]

This isn't correct. For example this change breaks:

struct T { int m; };
int x = sizeof(T::m);

[pmatos]

In D26843#599635, @EricWF wrote:

This isn't correct. For example this change breaks:

struct T { int m; };
int x = sizeof(T::m);

But that is not valid in C afaik and in C++ I get:

error: invalid use of non-static data member 'm'
  int x = sizeof(T::m);
                 ~~~^

Can you post a full reproducible example of what the change breaks?

[EricWF]

But that is not valid in C afaik and in C++ I get:

error: invalid use of non-static data member 'm'
  int x = sizeof(T::m);
                 ~~~^

Can you post a full reproducible example of what the change breaks?

That is a full reproducible example because it's valid C++. Clang currently compiles it.

[pmatos]

In D26843#599673, @EricWF wrote:

But that is not valid in C afaik and in C++ I get:

error: invalid use of non-static data member 'm'
  int x = sizeof(T::m);
                 ~~~^

Can you post a full reproducible example of what the change breaks?

That is a full reproducible example because it's valid C++. Clang currently compiles it.

But what was in the link was:

typedef struct { int m; } T;
int x = sizeof(T);
int main() {}

This is different from your initial snippet and compiles fine with my patch.

[EricWF]

This is different from your initial snippet and compiles fine with my patch.

You're confused. Both examples compile fine w/o your patch and are rejected with it. I just double checked.

[EricWF]

In case your wondering *why* this happens

In D26843#599804, @pmatos wrote:

In D26843#599673, @EricWF wrote:

But that is not valid in C afaik and in C++ I get:

error: invalid use of non-static data member 'm'
  int x = sizeof(T::m);
                 ~~~^

Can you post a full reproducible example of what the change breaks?

That is a full reproducible example because it's valid C++. Clang currently compiles it.

But what was in the link was:

typedef struct { int m; } T;
int x = sizeof(T);
int main() {}

This is different from your initial snippet and compiles fine with my patch.

Sorry your right. That should still say sizeof(T::m). My mistake.

[pmatos]

Apologies if I am being shallow and wasting your time but sizeof(T::m) doesn't compile at the moment with clang trunk. Using the same service you used before.

[pmatos]

OK, with a lot of help from @eli.friedman I have now a fix. Shall I reuse this review by submitting a new diff or open a new one?

[efriedma]

I would say make a new review thread since none of the discussion here is going to be helpful for understanding the new patch... but it really doesn't matter.

[pmatos]

Please refer to the new one: https://reviews.llvm.org/D27800

[pmatos]

Abandoning this revision.

[pmatos]

This is not supposed to be marked as accepted.

lib/Parse/ParseExpr.cpp

return Actions.ActOnSizeofParameterPackExpr(getCurScope(),
                                                 OpTok.getLocation(), 
                                                 *Name, NameLoc,
                                                 RParenLoc);
   }
 
   if (OpTok.isOneOf(tok::kw_alignof, tok::kw__Alignof))
     Diag(OpTok, diag::warn_cxx98_compat_alignof);
 
-  EnterExpressionEvaluationContext Unevaluated(Actions, Sema::Unevaluated,
+  EnterExpressionEvaluationContext Unevaluated(Actions,
+                 OpTok.is(tok::kw_sizeof) ?
+                 Sema::PotentiallyEvaluated :
+                 Sema::Unevaluated,
                                                Sema::ReuseLambdaContextDecl);
 
   bool isCastExpr;
   ParsedType CastTy;
   SourceRange CastRange;
   ExprResult Operand = ParseExprAfterUnaryExprOrTypeTrait(OpTok,
                                                           isCastExpr,
                                                           CastTy,