Re: r373159 - For P0784R7: compute whether a variable has constant destruction if it

2020-01-08 Thread Alex L via cfe-commits
Hi Richard,

I posted a patch that partially reverts your commit:
https://reviews.llvm.org/D72411. Unfortunately I can't revert it fully.

Thanks,
Alex

On Mon, 11 Nov 2019 at 15:30, Alex L  wrote:

> Here's a reduced test case.
>
> ```
> // OPTIONS: -x objective-c -std=gnu99 -fobjc-arc -emit-llvm
> @interface Foo
> @end
>
> Foo *foo() {
>   static Foo *f = ((void*)0;
>   return f;
> }
> // CHECK-NOT: cxa_guard
> ```
>
> AFAIK clang should not emit C++ guard variables in non-C++ code. Is that
> correct?
>
> On Mon, 11 Nov 2019 at 14:16, Alex L  wrote:
>
>> Hi Richard,
>>
>> I have a question about this commit. It looks like it started emitting
>> `cxa_guard_acquire/release` calls in Objective-C code, for globals with
>> constant initializers, causing link failures for things that don't link
>> with libc++abi. Was that intentional? I'm still working on a reduced
>> test-case that I can post.
>>
>> Thanks,
>> Alex
>>
>> On Sat, 28 Sep 2019 at 22:06, Richard Smith via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: rsmith
>>> Date: Sat Sep 28 22:08:46 2019
>>> New Revision: 373159
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=373159&view=rev
>>> Log:
>>> For P0784R7: compute whether a variable has constant destruction if it
>>> has a constexpr destructor.
>>>
>>> For constexpr variables, reject if the variable does not have constant
>>> destruction. In all cases, do not emit runtime calls to the destructor
>>> for variables with constant destruction.
>>>
>>> Added:
>>> cfe/trunk/test/CXX/expr/expr.const/p6-2a.cpp
>>> cfe/trunk/test/CodeGenCXX/non-const-init-cxx2a.cpp
>>> Modified:
>>> cfe/trunk/include/clang/AST/Decl.h
>>> cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
>>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> cfe/trunk/lib/AST/ASTContext.cpp
>>> cfe/trunk/lib/AST/Decl.cpp
>>> cfe/trunk/lib/AST/ExprConstant.cpp
>>> cfe/trunk/lib/AST/Interp/Interp.cpp
>>> cfe/trunk/lib/AST/TextNodeDumper.cpp
>>> cfe/trunk/lib/CodeGen/CGCall.cpp
>>> cfe/trunk/lib/CodeGen/CGClass.cpp
>>> cfe/trunk/lib/CodeGen/CGDecl.cpp
>>> cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
>>> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>>> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
>>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>>> cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
>>> cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
>>> cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
>>> cfe/trunk/test/CodeGenCXX/const-init-cxx2a.cpp
>>> cfe/trunk/test/CodeGenCXX/no_destroy.cpp
>>>
>>> Modified: cfe/trunk/include/clang/AST/Decl.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=373159&r1=373158&r2=373159&view=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>>> +++ cfe/trunk/include/clang/AST/Decl.h Sat Sep 28 22:08:46 2019
>>> @@ -808,12 +808,19 @@ struct EvaluatedStmt {
>>>/// valid if CheckedICE is true.
>>>bool IsICE : 1;
>>>
>>> +  /// Whether this variable is known to have constant destruction. That
>>> is,
>>> +  /// whether running the destructor on the initial value is a
>>> side-effect
>>> +  /// (and doesn't inspect any state that might have changed during
>>> program
>>> +  /// execution). This is currently only computed if the destructor is
>>> +  /// non-trivial.
>>> +  bool HasConstantDestruction : 1;
>>> +
>>>Stmt *Value;
>>>APValue Evaluated;
>>>
>>> -  EvaluatedStmt() : WasEvaluated(false), IsEvaluating(false),
>>> CheckedICE(false),
>>> -CheckingICE(false), IsICE(false) {}
>>> -
>>> +  EvaluatedStmt()
>>> +  : WasEvaluated(false), IsEvaluating(false), CheckedICE(false),
>>> +CheckingICE(false), IsICE(false), HasConstantDestruction(false)
>>> {}
>>>  };
>>>
>>>  /// Represents a variable declaration or definition.
>>> @@ -1267,6 +1274,14 @@ public:
>>>/// to untyped APValue if the value could not be evaluated.
>>>APValue *getEvaluatedValue() const;
>>>
>>> +  /// Evaluate the destruction of this variable to determine if it
>>> constitutes
>>> +  /// constant destruction.
>>> +  ///
>>> +  /// \pre isInitICE()
>>> +  /// \return \c true if this variable has constant destruction, \c
>>> false if
>>> +  /// not.
>>> +  bool evaluateDestruction(SmallVectorImpl &Notes)
>>> const;
>>> +
>>>/// Determines whether it is already known whether the
>>>/// initializer is an integral constant expression or not.
>>>bool isInitKnownICE() const;
>>> @@ -1505,9 +1520,14 @@ public:
>>>// has no definition within this source file.
>>>bool isKnownToBeDefined() const;
>>>
>>> -  /// Do we need to emit an exit-time destructor for this variable?
>>> +  /// Is destruction of this variable entirely suppressed? If so, the
>>> variable
>>> +  /// need not have a usable

Re: r373159 - For P0784R7: compute whether a variable has constant destruction if it

2019-11-11 Thread Alex L via cfe-commits
Here's a reduced test case.

```
// OPTIONS: -x objective-c -std=gnu99 -fobjc-arc -emit-llvm
@interface Foo
@end

Foo *foo() {
  static Foo *f = ((void*)0;
  return f;
}
// CHECK-NOT: cxa_guard
```

AFAIK clang should not emit C++ guard variables in non-C++ code. Is that
correct?

On Mon, 11 Nov 2019 at 14:16, Alex L  wrote:

> Hi Richard,
>
> I have a question about this commit. It looks like it started emitting
> `cxa_guard_acquire/release` calls in Objective-C code, for globals with
> constant initializers, causing link failures for things that don't link
> with libc++abi. Was that intentional? I'm still working on a reduced
> test-case that I can post.
>
> Thanks,
> Alex
>
> On Sat, 28 Sep 2019 at 22:06, Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Sat Sep 28 22:08:46 2019
>> New Revision: 373159
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=373159&view=rev
>> Log:
>> For P0784R7: compute whether a variable has constant destruction if it
>> has a constexpr destructor.
>>
>> For constexpr variables, reject if the variable does not have constant
>> destruction. In all cases, do not emit runtime calls to the destructor
>> for variables with constant destruction.
>>
>> Added:
>> cfe/trunk/test/CXX/expr/expr.const/p6-2a.cpp
>> cfe/trunk/test/CodeGenCXX/non-const-init-cxx2a.cpp
>> Modified:
>> cfe/trunk/include/clang/AST/Decl.h
>> cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/AST/ASTContext.cpp
>> cfe/trunk/lib/AST/Decl.cpp
>> cfe/trunk/lib/AST/ExprConstant.cpp
>> cfe/trunk/lib/AST/Interp/Interp.cpp
>> cfe/trunk/lib/AST/TextNodeDumper.cpp
>> cfe/trunk/lib/CodeGen/CGCall.cpp
>> cfe/trunk/lib/CodeGen/CGClass.cpp
>> cfe/trunk/lib/CodeGen/CGDecl.cpp
>> cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
>> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>> cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
>> cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
>> cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
>> cfe/trunk/test/CodeGenCXX/const-init-cxx2a.cpp
>> cfe/trunk/test/CodeGenCXX/no_destroy.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/Decl.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=373159&r1=373158&r2=373159&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>> +++ cfe/trunk/include/clang/AST/Decl.h Sat Sep 28 22:08:46 2019
>> @@ -808,12 +808,19 @@ struct EvaluatedStmt {
>>/// valid if CheckedICE is true.
>>bool IsICE : 1;
>>
>> +  /// Whether this variable is known to have constant destruction. That
>> is,
>> +  /// whether running the destructor on the initial value is a
>> side-effect
>> +  /// (and doesn't inspect any state that might have changed during
>> program
>> +  /// execution). This is currently only computed if the destructor is
>> +  /// non-trivial.
>> +  bool HasConstantDestruction : 1;
>> +
>>Stmt *Value;
>>APValue Evaluated;
>>
>> -  EvaluatedStmt() : WasEvaluated(false), IsEvaluating(false),
>> CheckedICE(false),
>> -CheckingICE(false), IsICE(false) {}
>> -
>> +  EvaluatedStmt()
>> +  : WasEvaluated(false), IsEvaluating(false), CheckedICE(false),
>> +CheckingICE(false), IsICE(false), HasConstantDestruction(false)
>> {}
>>  };
>>
>>  /// Represents a variable declaration or definition.
>> @@ -1267,6 +1274,14 @@ public:
>>/// to untyped APValue if the value could not be evaluated.
>>APValue *getEvaluatedValue() const;
>>
>> +  /// Evaluate the destruction of this variable to determine if it
>> constitutes
>> +  /// constant destruction.
>> +  ///
>> +  /// \pre isInitICE()
>> +  /// \return \c true if this variable has constant destruction, \c
>> false if
>> +  /// not.
>> +  bool evaluateDestruction(SmallVectorImpl &Notes)
>> const;
>> +
>>/// Determines whether it is already known whether the
>>/// initializer is an integral constant expression or not.
>>bool isInitKnownICE() const;
>> @@ -1505,9 +1520,14 @@ public:
>>// has no definition within this source file.
>>bool isKnownToBeDefined() const;
>>
>> -  /// Do we need to emit an exit-time destructor for this variable?
>> +  /// Is destruction of this variable entirely suppressed? If so, the
>> variable
>> +  /// need not have a usable destructor at all.
>>bool isNoDestroy(const ASTContext &) const;
>>
>> +  /// Do we need to emit an exit-time destructor for this variable, and
>> if so,
>> +  /// what kind?
>> +  QualType::DestructionKind needsDestruction(const ASTContext &Ctx)
>> const;
>> +
>>// Implement isa/cast/dyncast/etc.
>>static bool classof(const Decl *

Re: r373159 - For P0784R7: compute whether a variable has constant destruction if it

2019-11-11 Thread Alex L via cfe-commits
Hi Richard,

I have a question about this commit. It looks like it started emitting
`cxa_guard_acquire/release` calls in Objective-C code, for globals with
constant initializers, causing link failures for things that don't link
with libc++abi. Was that intentional? I'm still working on a reduced
test-case that I can post.

Thanks,
Alex

On Sat, 28 Sep 2019 at 22:06, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Sat Sep 28 22:08:46 2019
> New Revision: 373159
>
> URL: http://llvm.org/viewvc/llvm-project?rev=373159&view=rev
> Log:
> For P0784R7: compute whether a variable has constant destruction if it
> has a constexpr destructor.
>
> For constexpr variables, reject if the variable does not have constant
> destruction. In all cases, do not emit runtime calls to the destructor
> for variables with constant destruction.
>
> Added:
> cfe/trunk/test/CXX/expr/expr.const/p6-2a.cpp
> cfe/trunk/test/CodeGenCXX/non-const-init-cxx2a.cpp
> Modified:
> cfe/trunk/include/clang/AST/Decl.h
> cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/AST/Decl.cpp
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/AST/Interp/Interp.cpp
> cfe/trunk/lib/AST/TextNodeDumper.cpp
> cfe/trunk/lib/CodeGen/CGCall.cpp
> cfe/trunk/lib/CodeGen/CGClass.cpp
> cfe/trunk/lib/CodeGen/CGDecl.cpp
> cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
> cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
> cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
> cfe/trunk/test/CodeGenCXX/const-init-cxx2a.cpp
> cfe/trunk/test/CodeGenCXX/no_destroy.cpp
>
> Modified: cfe/trunk/include/clang/AST/Decl.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=373159&r1=373158&r2=373159&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/Decl.h (original)
> +++ cfe/trunk/include/clang/AST/Decl.h Sat Sep 28 22:08:46 2019
> @@ -808,12 +808,19 @@ struct EvaluatedStmt {
>/// valid if CheckedICE is true.
>bool IsICE : 1;
>
> +  /// Whether this variable is known to have constant destruction. That
> is,
> +  /// whether running the destructor on the initial value is a side-effect
> +  /// (and doesn't inspect any state that might have changed during
> program
> +  /// execution). This is currently only computed if the destructor is
> +  /// non-trivial.
> +  bool HasConstantDestruction : 1;
> +
>Stmt *Value;
>APValue Evaluated;
>
> -  EvaluatedStmt() : WasEvaluated(false), IsEvaluating(false),
> CheckedICE(false),
> -CheckingICE(false), IsICE(false) {}
> -
> +  EvaluatedStmt()
> +  : WasEvaluated(false), IsEvaluating(false), CheckedICE(false),
> +CheckingICE(false), IsICE(false), HasConstantDestruction(false) {}
>  };
>
>  /// Represents a variable declaration or definition.
> @@ -1267,6 +1274,14 @@ public:
>/// to untyped APValue if the value could not be evaluated.
>APValue *getEvaluatedValue() const;
>
> +  /// Evaluate the destruction of this variable to determine if it
> constitutes
> +  /// constant destruction.
> +  ///
> +  /// \pre isInitICE()
> +  /// \return \c true if this variable has constant destruction, \c false
> if
> +  /// not.
> +  bool evaluateDestruction(SmallVectorImpl &Notes)
> const;
> +
>/// Determines whether it is already known whether the
>/// initializer is an integral constant expression or not.
>bool isInitKnownICE() const;
> @@ -1505,9 +1520,14 @@ public:
>// has no definition within this source file.
>bool isKnownToBeDefined() const;
>
> -  /// Do we need to emit an exit-time destructor for this variable?
> +  /// Is destruction of this variable entirely suppressed? If so, the
> variable
> +  /// need not have a usable destructor at all.
>bool isNoDestroy(const ASTContext &) const;
>
> +  /// Do we need to emit an exit-time destructor for this variable, and
> if so,
> +  /// what kind?
> +  QualType::DestructionKind needsDestruction(const ASTContext &Ctx) const;
> +
>// Implement isa/cast/dyncast/etc.
>static bool classof(const Decl *D) { return classofKind(D->getKind()); }
>static bool classofKind(Kind K) { return K >= firstVar && K <= lastVar;
> }
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=373159&r1=373158&r2=373159&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (

r373159 - For P0784R7: compute whether a variable has constant destruction if it

2019-09-28 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Sat Sep 28 22:08:46 2019
New Revision: 373159

URL: http://llvm.org/viewvc/llvm-project?rev=373159&view=rev
Log:
For P0784R7: compute whether a variable has constant destruction if it
has a constexpr destructor.

For constexpr variables, reject if the variable does not have constant
destruction. In all cases, do not emit runtime calls to the destructor
for variables with constant destruction.

Added:
cfe/trunk/test/CXX/expr/expr.const/p6-2a.cpp
cfe/trunk/test/CodeGenCXX/non-const-init-cxx2a.cpp
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/AST/Interp/Interp.cpp
cfe/trunk/lib/AST/TextNodeDumper.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
cfe/trunk/test/CodeGenCXX/const-init-cxx2a.cpp
cfe/trunk/test/CodeGenCXX/no_destroy.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=373159&r1=373158&r2=373159&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Sat Sep 28 22:08:46 2019
@@ -808,12 +808,19 @@ struct EvaluatedStmt {
   /// valid if CheckedICE is true.
   bool IsICE : 1;
 
+  /// Whether this variable is known to have constant destruction. That is,
+  /// whether running the destructor on the initial value is a side-effect
+  /// (and doesn't inspect any state that might have changed during program
+  /// execution). This is currently only computed if the destructor is
+  /// non-trivial.
+  bool HasConstantDestruction : 1;
+
   Stmt *Value;
   APValue Evaluated;
 
-  EvaluatedStmt() : WasEvaluated(false), IsEvaluating(false), 
CheckedICE(false),
-CheckingICE(false), IsICE(false) {}
-
+  EvaluatedStmt()
+  : WasEvaluated(false), IsEvaluating(false), CheckedICE(false),
+CheckingICE(false), IsICE(false), HasConstantDestruction(false) {}
 };
 
 /// Represents a variable declaration or definition.
@@ -1267,6 +1274,14 @@ public:
   /// to untyped APValue if the value could not be evaluated.
   APValue *getEvaluatedValue() const;
 
+  /// Evaluate the destruction of this variable to determine if it constitutes
+  /// constant destruction.
+  ///
+  /// \pre isInitICE()
+  /// \return \c true if this variable has constant destruction, \c false if
+  /// not.
+  bool evaluateDestruction(SmallVectorImpl &Notes) const;
+
   /// Determines whether it is already known whether the
   /// initializer is an integral constant expression or not.
   bool isInitKnownICE() const;
@@ -1505,9 +1520,14 @@ public:
   // has no definition within this source file.
   bool isKnownToBeDefined() const;
 
-  /// Do we need to emit an exit-time destructor for this variable?
+  /// Is destruction of this variable entirely suppressed? If so, the variable
+  /// need not have a usable destructor at all.
   bool isNoDestroy(const ASTContext &) const;
 
+  /// Do we need to emit an exit-time destructor for this variable, and if so,
+  /// what kind?
+  QualType::DestructionKind needsDestruction(const ASTContext &Ctx) const;
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K >= firstVar && K <= lastVar; }

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=373159&r1=373158&r2=373159&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Sat Sep 28 22:08:46 2019
@@ -145,8 +145,10 @@ def note_constexpr_access_volatile_obj :
   "a constant expression">;
 def note_constexpr_volatile_here : Note<
   "volatile %select{temporary created|object declared|member declared}0 here">;
-def note_constexpr_ltor_mutable : Note<
-  "read of mutable member %0 is not allowed in a constant expression">;
+def note_constexpr_access_mutable : Note<
+  "%select{read of|read of|assignment to|increment of|decrement of|"
+  "member call on|dynamic_cast of|typeid applied to|destruction of}0 "
+  "mutable member %1 is not allowed in a con