[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363488: [clang] perform semantic checking in constant 
context (authored by Tyker, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62009?vs=200662&id=204906#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/lib/Sema/Sema.cpp
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/SemaCXX/attr-nonnull.cpp
  cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp

Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -599,7 +599,8 @@
   /// which we can fold and convert to a boolean condition using
   /// any crazy technique that we want to, even if the expression has
   /// side-effects.
-  bool EvaluateAsBooleanCondition(bool &Result, const ASTContext &Ctx) const;
+  bool EvaluateAsBooleanCondition(bool &Result, const ASTContext &Ctx,
+  bool InConstantContext = false) const;
 
   enum SideEffectsKind {
 SE_NoSideEffects,  ///< Strictly evaluate the expression.
@@ -611,20 +612,21 @@
   /// EvaluateAsInt - Return true if this is a constant which we can fold and
   /// convert to an integer, using any crazy technique that we want to.
   bool EvaluateAsInt(EvalResult &Result, const ASTContext &Ctx,
- SideEffectsKind AllowSideEffects = SE_NoSideEffects) const;
+ SideEffectsKind AllowSideEffects = SE_NoSideEffects,
+ bool InConstantContext = false) const;
 
   /// EvaluateAsFloat - Return true if this is a constant which we can fold and
   /// convert to a floating point value, using any crazy technique that we
   /// want to.
-  bool
-  EvaluateAsFloat(llvm::APFloat &Result, const ASTContext &Ctx,
-  SideEffectsKind AllowSideEffects = SE_NoSideEffects) const;
+  bool EvaluateAsFloat(llvm::APFloat &Result, const ASTContext &Ctx,
+   SideEffectsKind AllowSideEffects = SE_NoSideEffects,
+   bool InConstantContext = false) const;
 
   /// EvaluateAsFloat - Return true if this is a constant which we can fold and
   /// convert to a fixed point value.
-  bool EvaluateAsFixedPoint(
-  EvalResult &Result, const ASTContext &Ctx,
-  SideEffectsKind AllowSideEffects = SE_NoSideEffects) const;
+  bool EvaluateAsFixedPoint(EvalResult &Result, const ASTContext &Ctx,
+SideEffectsKind AllowSideEffects = SE_NoSideEffects,
+bool InConstantContext = false) const;
 
   /// isEvaluatable - Call EvaluateAsRValue to see if this expression can be
   /// constant folded without side-effects, but discard the result.
@@ -660,7 +662,8 @@
 
   /// EvaluateAsLValue - Evaluate an expression to see if we can fold it to an
   /// lvalue with link time known address, with no side-effects.
-  bool EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx) const;
+  bool EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx,
+bool InConstantContext = false) const;
 
   /// EvaluateAsInitializer - Evaluate an expression as if it were the
   /// initializer of the given declaration. Returns true if the initializer
Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -797,6 +797,15 @@
 }
   };
 
+  /// Used to change context to isConstantEvaluated without pushing a heavy
+  /// ExpressionEvaluationContextRecord object.
+  bool isConstantEvaluatedOverride;
+
+  bool isConstantEvaluated() {
+return ExprEvalContexts.back().isConstantEvaluated() ||
+   isConstantEvaluatedOverride;
+  }
+
   /// RAII object to handle the state changes required to synthesize
   /// a function body.
   class SynthesizedFunctionScope {
Index: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
@@ -85,6 +85,8 @@
   "access array element of|ERROR|"
   "access real component of|access imaginary component of}0 "
   "pointer past the end of object">;
+def note_non_null_attribute_failed : Note<
+  "null passed to a callee that requires a non-null argument">;
 def note_constexpr_null_subobject : Note<
   "cannot %select{access base class of|access derived class of|access field of|"
   "access array element of|perform pointer arith

[PATCH] D62009: [clang] perform semantic checking in constant context

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks, looks great.




Comment at: clang/lib/Sema/SemaChecking.cpp:13941
 ///
+/// \param isConstantEvaluated wether the evalaution should be permormed in
+/// constant context.

permormed -> performed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

@rsmith ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

@rsmith friendly ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 200662.
Tyker added a comment.

most comments were fixed. but there is a few point on which the direction isn't 
clear.

> I think all of the new warnings in the test cases here are undesirable (they 
> duplicate errors produced by the constant evaluator)

in the last revision `warn_impcast_integer_precision_constant` was always 
diagnosed in constant expression and diagnosed if reachable otherwise. this 
warning was responsible of all new warnings but isn't diagnosed by the constant 
evaluator. which is correct because AFAIK integral casts have implementation 
defined behavior and not undefined behavior.

> For code patterns that are suspicious but defined (eg, we think they might do 
> something other than what the programmer intended), we should typically 
> diagnose using Diag, regardless of whether they appear in a constant context.

this is not currently done even in non constant context. for example 
`warn_impcast_integer_precision_constant` is diagnosed via 
`DiagRuntimeBehavior`.

in this revision

  constexpr int i0 = (long long)__builtin_is_constant_evaluated() * (1ll << 
33); //#1

is diagnosed as it should by the reason it is diagnosed isn't correct. it is 
diagnosed only because `CheckCompletedExpr` doesn't push an 
`ExpressionEvaluationContextRecord` but just set isConstantEvaluatedOverride. 
so `DiagRuntimeBehavior` doesn't see the context as constant context. This 
means that other context that are correctly in constant context like:

  case (long long)__builtin_is_constant_evaluated() * (1ll << 33):; //#1

will not be diagnosed even thought they probably should.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticIDs.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/attr-nonnull.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp

Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -438,8 +438,8 @@
 
 constexpr char c0 = "nought index"[0];
 constexpr char c1 = "nice index"[10];
-constexpr char c2 = "nasty index"[12]; // expected-error {{must be initialized by a constant expression}} expected-warning {{is past the end}} expected-note {{read of dereferenced one-past-the-end pointer}}
-constexpr char c3 = "negative index"[-1]; // expected-error {{must be initialized by a constant expression}} expected-warning {{is before the beginning}} expected-note {{cannot refer to element -1 of array of 15 elements}}
+constexpr char c2 = "nasty index"[12]; // expected-error {{must be initialized by a constant expression}} expected-note {{read of dereferenced one-past-the-end pointer}}
+constexpr char c3 = "negative index"[-1]; // expected-error {{must be initialized by a constant expression}} expected-note {{cannot refer to element -1 of array of 15 elements}}
 constexpr char c4 = ((char*)(int*)"no reinterpret_casts allowed")[14]; // expected-error {{must be initialized by a constant expression}} expected-note {{cast that performs the conversions of a reinterpret_cast}}
 
 constexpr const char *p = "test" + 2;
@@ -547,7 +547,7 @@
 constexpr int xs0 = p[-3]; // ok
 constexpr int xs_1 = p[-4]; // expected-error {{constant expression}} expected-note {{cannot refer to element -1}}
 
-constexpr int zs[2][2][2][2] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }; // expected-note {{array 'zs' declared here}}
+constexpr int zs[2][2][2][2] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
 static_assert(zs[0][0][0][0] == 1, "");
 static_assert(zs[1][1][1][1] == 16, "");
 static_assert(zs[0][0][0][2] == 3, ""); // expected-error {{constant expression}} expected-note {{read of dereferenced one-past-the-end pointer}}
@@ -557,8 +557,7 @@
 static_assert(*(&(&(*(*&(&zs[2] - 1)[0] + 2 - 2))[2])[-1][2] - 2) == 11, "");
 constexpr int err_zs_1_2_0_0 = zs[1][2][0][0]; // \
 expected-error {{constant expression}} \
-expected-note {{cannot access array element of pointer past the end}} \
-expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
+expected-note {{cannot access array element of pointer past the end}}
 
 constexpr int fail(const int &p) {
   return (&p)[64]; // expected-note {{cannot refer to element 64 of array of 2 elements}}
Index: clang/test/SemaCXX/attr-nonnull.cpp
===
--- clang/test/SemaCXX/attr-nonnull.cpp
+++ clang/test/SemaCXX/attr-nonnull.cpp
@@ -52,3 +52,35 @@
   (void)(x != 0);  // expected-warning{{null passed}}
 }
 }
+
+namespace test5 {
+
+constexpr int c = 0;
+
+__attribute__((nonnull))
+c

[PATCH] D62009: [clang] perform semantic checking in constant context

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Interesting. I think all of the new warnings in the test cases here are 
undesirable (they duplicate errors produced by the constant evaluator), but the 
removed warnings all look like improvements.

Generally, I think our goals should be:

For code patterns that might lead to undefined behavior when executed with 
certain values:

- If the code appears in a constant context (`isConstantEvaluated()` or a 
subexpression of a `ConstantExpr`), we should diagnose the problem from the 
constant evaluator if it actually happens and otherwise not diagnose it.
- Otherwise, we should warn on the problem if the code is reachable and 
otherwise not diagnose it (that is, use `DiagRuntimeBehavior`).

For code patterns that are suspicious but defined (eg, we think they might do 
something other than what the programmer intended), we should typically 
diagnose using `Diag`, regardless of whether they appear in a constant context.

`DiagRuntimeBehavior` already deals with much of this for us: if called in a 
`ConstantEvaluated` context, it suppresses diagnostics.

One way we could proceed would be to extend what you did for 
`CheckCompletedExpr` to the cases where `SemaChecking.cpp` (and friends) 
recurse into a `ConstantExpr`: temporarily create a `ConstantEvaluated` 
expression evaluation context for the purpose of the checks, so that we can 
track that we're within such a construct. But an 
`ExpressionEvaluationContextRecord` is not a trivial thing to create and pass 
around, and carries additional semantic implications on top of the context 
kind, so perhaps just adding a flag to it to indicate that we're performing 
semantic analysis inside a `ConstantExpr` within the context would be better.




Comment at: clang/lib/Sema/SemaChecking.cpp:4066-4073
 static void CheckNonNullArgument(Sema &S,
  const Expr *ArgExpr,
  SourceLocation CallSiteLoc) {
   if (CheckNonNullExpr(S, ArgExpr))
-S.DiagRuntimeBehavior(CallSiteLoc, ArgExpr,
-   S.PDiag(diag::warn_null_arg) << ArgExpr->getSourceRange());
+DiagRuntimeOrConstant(S, CallSiteLoc, ArgExpr,
+  S.PDiag(diag::warn_null_arg)
+  << ArgExpr->getSourceRange());

For `__attribute__((nonnull))`, I think the right thing to do is to change the 
constant expression evaluator to consider passing a null pointer argument for a 
nonnull-attributed parameter to be non-constant. (It's undefined behavior, so 
we should not permit it in constant evaluations.) For `_Nonnull`, I think the 
situation is less clear -- that case is explicitly not undefined behavior, so 
it's not clear that treating it as non-constant is the right choice. But as 
with the other cases here, `isConstantEvaluated()` does not imply the code is 
necessarily reachable, so bypassing `DiagRuntimeBehavior`'s check for 
reachability doesn't seem like the appropriate behavior.



Comment at: clang/lib/Sema/SemaChecking.cpp:6089
   if (Result.getSExtValue() < Low || Result.getSExtValue() > High) {
-if (RangeIsError)
+if (RangeIsError || isConstantEvaluated())
   return Diag(TheCall->getBeginLoc(), diag::err_argument_invalid_range)

`isConstantEvaluated()` doesn't imply that the expression is necessarily 
reachable, so always giving an error here doesn't seem right. Instead, we 
should (already) have the relevant range checks in the constant evaluator for 
such builtins that are usable in constant expressions, and should never need to 
check here (if the code is reached during a constant evaluation, then the 
evaluation will be non-constant and hence ill-formed).

I think we should just return early from this function if 
`isConstantEvaluated()` like you do in `checkFortifiedBuiltinMemoryFunction`.



Comment at: clang/lib/Sema/SemaChecking.cpp:6612-6613
 bool Cond;
-if (C->getCond()->EvaluateAsBooleanCondition(Cond, S.getASTContext())) {
+if (C->getCond()->EvaluateAsBooleanCondition(Cond, S.getASTContext(),
+ S.isConstantEvaluated())) {
   if (Cond)

If we're in a constant-evaluated context, I think we can and should simply skip 
all format string checking. There are three cases here:

1) The call with the format attribute is unreached: no diagnostic should be 
issued.
2) The call with the format attribute is reached but not constant-evaluatable: 
an error will be produced that we couldn't constant-evaluate.
3) The call with the format attribute is reached and is constant-evaluatable: 
the constant evaluator needs to check that the format string matches the 
arguments anyway (because in general we permit non-constant format strings), so 
it will need to properly handle this case.

Case 3 is currently theoretical (we don't constant-evaluate any format-string 
functions). But in any case, I think the con

[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

I changed the approach in the last revision. the information is now propagated 
using the expression evaluation context and then via booleans. I should have 
added a comment to make it clear.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62009: [clang] perform semantic checking in constant context

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D62009#1506415 , @Tyker wrote:

> there are issues with using ConstantExpr to mark expressions as constant for 
> semantic checking:
>
> - #1 multpile Expr::Ignore* operation remove ConstantExpr from the expression.


`Ignore*` is generally used when walking the syntactic form of an expression, 
so this is usually appropriate.

> - #2 Semantic checking Traverses the AST so all methods that only mark the 
> top-level Node will not completely work.

Semantic checks that care about constant evaluation should not walk over a 
`ConstantExpr` node. Generally, we want rather different checking inside a 
`ConstantExpr` node than outside (for example, we shouldn't be performing any 
checks for potential overflow or narrowing or conversions, and instead should 
be finding out what *actually* happens by evaluating the expression and 
diagnosing problems in it). So this seems fine too, though there may still be 
places that need updates to properly handle `ConstantExpr`.

> - #3 at short term: adding ConstantExpr modifies the AST structure, so adding 
> it everywhere it is needed for semantic checking will require changing code 
> that depend closely on the AST structure.

Yes. But that's going to be the case for any approach we take here.

> - push a ExpressionEvaluationContext::ConstantEvaluated so Sema will 
> propagate it and propagate from Sema to the expression evaluator via boolean 
> values.
> - put all semantic checking function's in a SemaCheckContext class and 
> propagate via this class to the expression evaluator. this class will be 
> usable to propagate Sema and probably other informations.
> - keep the bit in ExprBitfield but make all nodes created in 
> ExpressionEvaluationContext::ConstantEvaluated marked for constant evaluation 
> to fixes limitation #2.

We don't always know whether an expression is constant evaluated until after 
we've parsed it (eg, for a call to a function that might be `consteval` we need 
to perform overload resolution before we find out). And this would require 
changing all the (many) places that create `Expr` nodes to pass in new 
information. That sounds unpleasant and expensive, and adds a permanent cost to 
all future AST changes.

So far we don't appear to need any per-`Expr` indicator of whether that 
expression is transitively within a `ConstantExpr`, so let's not add that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 200231.
Tyker edited the summary of this revision.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CXX/expr/expr.const/p3-0x.cpp
  clang/test/Sema/integer-overflow.c
  clang/test/Sema/switch.c
  clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/integer-overflow.cpp

Index: clang/test/SemaCXX/integer-overflow.cpp
===
--- clang/test/SemaCXX/integer-overflow.cpp
+++ clang/test/SemaCXX/integer-overflow.cpp
@@ -92,6 +92,7 @@
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
   case ((1ULL * ((4608) * ((1024) * (1024))) + 2ULL)):
 return 9;
+// expected-warning@+3 {{implicit conversion}}
 // expected-warning@+2 2{{overflow in expression; result is 536870912 with type 'int'}}
 // expected-warning@+1 {{overflow converting case value to switch condition type (288230376151711744 to 0)}}
   case ((uint64_t)((uint64_t)(4608 * 1024 * 1024) * (uint64_t)(4608 * 1024 * 1024))):
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -438,8 +438,8 @@
 
 constexpr char c0 = "nought index"[0];
 constexpr char c1 = "nice index"[10];
-constexpr char c2 = "nasty index"[12]; // expected-error {{must be initialized by a constant expression}} expected-warning {{is past the end}} expected-note {{read of dereferenced one-past-the-end pointer}}
-constexpr char c3 = "negative index"[-1]; // expected-error {{must be initialized by a constant expression}} expected-warning {{is before the beginning}} expected-note {{cannot refer to element -1 of array of 15 elements}}
+constexpr char c2 = "nasty index"[12]; // expected-error {{must be initialized by a constant expression}} expected-note {{read of dereferenced one-past-the-end pointer}}
+constexpr char c3 = "negative index"[-1]; // expected-error {{must be initialized by a constant expression}} expected-note {{cannot refer to element -1 of array of 15 elements}}
 constexpr char c4 = ((char*)(int*)"no reinterpret_casts allowed")[14]; // expected-error {{must be initialized by a constant expression}} expected-note {{cast that performs the conversions of a reinterpret_cast}}
 
 constexpr const char *p = "test" + 2;
@@ -547,7 +547,7 @@
 constexpr int xs0 = p[-3]; // ok
 constexpr int xs_1 = p[-4]; // expected-error {{constant expression}} expected-note {{cannot refer to element -1}}
 
-constexpr int zs[2][2][2][2] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }; // expected-note {{array 'zs' declared here}}
+constexpr int zs[2][2][2][2] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
 static_assert(zs[0][0][0][0] == 1, "");
 static_assert(zs[1][1][1][1] == 16, "");
 static_assert(zs[0][0][0][2] == 3, ""); // expected-error {{constant expression}} expected-note {{read of dereferenced one-past-the-end pointer}}
@@ -557,8 +557,7 @@
 static_assert(*(&(&(*(*&(&zs[2] - 1)[0] + 2 - 2))[2])[-1][2] - 2) == 11, "");
 constexpr int err_zs_1_2_0_0 = zs[1][2][0][0]; // \
 expected-error {{constant expression}} \
-expected-note {{cannot access array element of pointer past the end}} \
-expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
+expected-note {{cannot access array element of pointer past the end}}
 
 constexpr int fail(const int &p) {
   return (&p)[64]; // expected-note {{cannot refer to element 64 of array of 2 elements}}
Index: clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
===
--- clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
+++ clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
@@ -119,3 +119,15 @@
 };
 TestConditionalExplicit e = 42;
 #endif
+
+constexpr int i1 = (long long)__builtin_is_constant_evaluated() * (1ll << 33);
+// expected-warning@-1 {{implicit conversion}}
+
+constexpr int i2 = (long long)!__builtin_is_constant_evaluated() * (1ll << 33);
+
+ void f(int i) {
+   switch (i) {
+   case (long long)__builtin_is_constant_evaluated() * (1ll << 33):;
+// expected-warning@-1 {{implicit conversion}}
+   }
+ }
Index: clang/test/Sema/switch.c
===
--- clang/test/Sema/switch.c
+++ clang/test/Sema/switch.c
@@ -8,7 +8,7 @@
 void foo(int X) {
   switch (X) {
   case 42: ; // expected-note {{previous case}}
-  case 50LL: // expected-warning {{overflow}}
+  case 50LL: // expected-warning {{overflow}} expected-warning {{implicit conversion}} 
   case 42:

[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

I checked how we can propagate the information about constant context through 
semantic checking.
there are issues with using ConstantExpr to mark expressions as constant for 
semantic checking:

- #1 multpile Expr::Ignore* operation remove ConstantExpr from the expression.
- #2 Semantic checking Traverses the AST so all methods that only mark the 
top-level Node not will fail.
- #3 at short term: adding ConstantExpr modifies the AST structure, so adding 
it everywhere it is needed for semantic checking will require changing a lot of 
code that depend closely on the AST structure.

Note: the limitation #2 also applies to the bit in ExprBitfield solution in its 
current form.

propagating constant context to the expression evaluator via a boolean value 
will be a lot of boilerplate and in my opinion this should be propagated more 
"automatically".
 so I think the best solutions are:

- push a ExpressionEvaluationContext::ConstantEvaluated so Sema will propagate 
it and propagate from Sema to the expression evaluator via boolean values.
- put all semantic checking function's in a SemaCheckContext class and 
propagate via this class to the expression evaluator. this class will be usable 
to propagate Sema and probably other informations.
- keep the bit in ExprBitfield but make all nodes created in 
ExpressionEvaluationContext::ConstantEvaluated marked for constant evaluation 
to fixes limitation #2.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62009: [clang] perform semantic checking in constant context

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D62009#1505263 , @Tyker wrote:

> i added this bit because when we eventually store the result of evaluation in 
> ConstantExpr we will probably want to trail-allocate the APValue's possible 
> representations separately to limit memory consumption. but if we do this the 
> ConstantExpr node will need to be created after evaluation of the value to 
> allocate the right space and we will need an other way to mark that the 
> expression should be evaluated in constant context.


Even if we want to tail-allocate (which does seem like a nice idea in the long 
term), we don't need to mark the expression for that: we can pass a flag into 
the constant evaluator to indicate that we're evaluating an expression with the 
intent of forming a `ConstantExpr`. (We already do that for `EvaluateAsRValue`; 
we'll need to do the same for other entry points that could be evaluating at 
the top level in a constant context.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

i added this bit because when we eventually store the result of evaluation in 
ConstantExpr we will probably want to trail-allocate the APValue's possible 
representations separately to limit memory consumption. but if we do this the 
ConstantExpr node will need to be created after evaluation of the value, so we 
cannot use it to mark that the expression should be evaluated in constant 
context.
if we won't store APValue's possible representations separately in 
trail-allocate space, wrapping the expression in a ConstantExpr is the right 
solution.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62009: [clang] perform semantic checking in constant context

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

The approach we've been taking for this up until now is to use a `ConstantExpr` 
node to mark the entry point of a constant context; it looks like that would 
continue to work here, but we're missing those nodes in some cases? (This is 
preferable to using a flag because it also -- eventually -- gives us a place to 
store the evaluated value, which we're going to need for various upcoming 
features, particularly `consteval`.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 199830.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/builtin-is-constant-evaluated.cpp

Index: clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
===
--- clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
+++ clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
@@ -119,3 +119,11 @@
 };
 TestConditionalExplicit e = 42;
 #endif
+
+constexpr int i = (long long)__builtin_is_constant_evaluated() * (1ll << 33);
+// expected-warning@-1 {{implicit conversion}}
+
+void f() {
+  if constexpr ((long long)__builtin_is_constant_evaluated() * (1ll << 33))
+ ;// expected-error@-1 {{cannot be narrowed to type 'bool'}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5408,6 +5408,7 @@
   if (checkPlaceholderForOverload(S, From))
 return ExprError();
 
+  From->setIsConstantContext();
   // C++1z [expr.const]p3:
   //  A converted constant expression of type T is an expression,
   //  implicitly converted to type T, where the converted
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -11100,6 +11100,10 @@
   return;
   }
 
+  Init->setIsConstantContext(
+  VDecl->isConstexpr() ||
+  (VDecl->getType().isConstQualified() && !Init->isValueDependent()));
+
   // dllimport cannot be used on variable definitions.
   if (VDecl->hasAttr() && !VDecl->isStaticDataMember()) {
 Diag(VDecl->getLocation(), diag::err_attribute_dllimport_data_definition);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11439,7 +11439,7 @@
 bool Expr::EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx,
 bool InConstantContext) const {
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
-  Info.InConstantContext = InConstantContext;
+  Info.InConstantContext = InConstantContext || IsConstantContext();
   return ::EvaluateAsRValue(this, Result, Ctx, Info);
 }
 
@@ -11453,12 +11453,14 @@
 bool Expr::EvaluateAsInt(EvalResult &Result, const ASTContext &Ctx,
  SideEffectsKind AllowSideEffects) const {
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
+  Info.InConstantContext = IsConstantContext();
   return ::EvaluateAsInt(this, Result, Ctx, AllowSideEffects, Info);
 }
 
 bool Expr::EvaluateAsFixedPoint(EvalResult &Result, const ASTContext &Ctx,
 SideEffectsKind AllowSideEffects) const {
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
+  Info.InConstantContext = IsConstantContext();
   return ::EvaluateAsFixedPoint(this, Result, Ctx, AllowSideEffects, Info);
 }
 
@@ -11478,7 +11480,7 @@
 
 bool Expr::EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx) const {
   EvalInfo Info(Ctx, Result, EvalInfo::EM_ConstantFold);
-
+  Info.InConstantContext = IsConstantContext();
   LValue LV;
   if (!EvaluateLValue(this, LV, Info) || Result.HasSideEffects ||
   !CheckLValueConstantExpression(Info, getExprLoc(),
@@ -11588,6 +11590,7 @@
   EvalResult EVResult;
   if (!FastEvaluateAsRValue(this, EVResult, Ctx, IsConst)) {
 EvalInfo Info(Ctx, EVResult, EvalInfo::EM_EvaluateForOverflow);
+Info.InConstantContext = IsConstantContext();
 (void)::EvaluateAsRValue(Info, this, EVResult.Val);
   }
 }
@@ -12115,6 +12118,7 @@
   SmallVector Diags;
   Status.Diag = &Diags;
   EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantExpression);
+  Info.InConstantContext = IsConstantContext();
 
   APValue Scratch;
   bool IsConstExpr = ::EvaluateAsRValue(Info, this, Result ? *Result : Scratch);
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -318,8 +318,9 @@
 unsigned ValueDependent : 1;
 unsigned InstantiationDependent : 1;
 unsigned ContainsUnexpandedParameterPack : 1;
+unsigned IsConstantContext : 1;
   };
-  enum { NumExprBits = NumStmtBits + 9 };
+  enum { NumExprBits = NumStmtBits + 10 };
 
   class PredefinedExprBitfields {
 friend class ASTStmtReader;
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -223,6 +223,18 @@
 ExprBits.ContainsUnexpandedParameterPack = PP;
   }
 
+  /// Wether this expressi