MaskRay updated this revision to Diff 486641.
MaskRay edited the summary of this revision.
MaskRay added a comment.
add release note (this makes it slightly difficult to backport)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140868/new/
https://reviews.llvm.org/D140868
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/CodeGen/attr-noreturn.c
Index: clang/test/CodeGen/attr-noreturn.c
===================================================================
--- clang/test/CodeGen/attr-noreturn.c
+++ clang/test/CodeGen/attr-noreturn.c
@@ -13,6 +13,7 @@
// CHECK-LABEL: @test_conditional_gnu(
// CHECK: %cond = select i1 %tobool, ptr @t1, ptr @t2
// CHECK: call void %cond(
+// CHECK: call void %cond2(
// CHECK-NEXT: unreachable
// CHECK-CXX-LABEL: @_Z20test_conditional_gnui(
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -8262,7 +8262,9 @@
lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
- QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee);
+ QualType CompositeTy = S.Context.mergeTypes(
+ lhptee, rhptee, /*OfBlockPointer=*/false, /*Unqualified=*/false,
+ /*BlockReturnType=*/false, /*IsConditionalOperator=*/true);
if (CompositeTy.isNull()) {
// In this situation, we assume void* type. No especially good
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10191,7 +10191,8 @@
QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
bool OfBlockPointer, bool Unqualified,
- bool AllowCXX) {
+ bool AllowCXX,
+ bool IsConditionalOperator) {
const auto *lbase = lhs->castAs<FunctionType>();
const auto *rbase = rhs->castAs<FunctionType>();
const auto *lproto = dyn_cast<FunctionProtoType>(lbase);
@@ -10254,9 +10255,27 @@
if (lbaseInfo.getNoCfCheck() != rbaseInfo.getNoCfCheck())
return {};
- // FIXME: some uses, e.g. conditional exprs, really want this to be 'both'.
- bool NoReturn = lbaseInfo.getNoReturn() || rbaseInfo.getNoReturn();
-
+ // When merging declarations, it's common for supplemental information like
+ // attributes to only be present in one of the declarations, and we generally
+ // want type merging to preserve the union of information. So a merged
+ // function type should be noreturn if it was noreturn in *either* operand
+ // type.
+ //
+ // But for the conditional operator, this is backwards. The result of the
+ // operator could be either operand, and its type should conservatively
+ // reflect that. So a function type in a composite type is noreturn only
+ // if it's noreturn in *both* operand types.
+ //
+ // Arguably, noreturn is a kind of subtype, and the conditional operator
+ // ought to produce the most specific common supertype of its operand types.
+ // That would differ from this rule in contravariant positions. However,
+ // neither C nor C++ generally uses this kind of subtype reasoning. Also,
+ // as a practical matter, it would only affect C code that does abstraction of
+ // higher-order functions (taking noreturn callbacks!), which is uncommon to
+ // say the least. So we use the simpler rule.
+ bool NoReturn = IsConditionalOperator
+ ? lbaseInfo.getNoReturn() && rbaseInfo.getNoReturn()
+ : lbaseInfo.getNoReturn() || rbaseInfo.getNoReturn();
if (lbaseInfo.getNoReturn() != NoReturn)
allLTypes = false;
if (rbaseInfo.getNoReturn() != NoReturn)
@@ -10389,9 +10408,9 @@
return {};
}
-QualType ASTContext::mergeTypes(QualType LHS, QualType RHS,
- bool OfBlockPointer,
- bool Unqualified, bool BlockReturnType) {
+QualType ASTContext::mergeTypes(QualType LHS, QualType RHS, bool OfBlockPointer,
+ bool Unqualified, bool BlockReturnType,
+ bool IsConditionalOperator) {
// For C++ we will not reach this code with reference types (see below),
// for OpenMP variant call overloading we might.
//
@@ -10684,7 +10703,8 @@
ArrayType::ArraySizeModifier(), 0);
}
case Type::FunctionNoProto:
- return mergeFunctionTypes(LHS, RHS, OfBlockPointer, Unqualified);
+ return mergeFunctionTypes(LHS, RHS, OfBlockPointer, Unqualified,
+ /*AllowCXX=*/false, IsConditionalOperator);
case Type::Record:
case Type::Enum:
return {};
Index: clang/include/clang/AST/ASTContext.h
===================================================================
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -2865,10 +2865,12 @@
bool canBindObjCObjectType(QualType To, QualType From);
// Functions for calculating composite types
- QualType mergeTypes(QualType, QualType, bool OfBlockPointer=false,
- bool Unqualified = false, bool BlockReturnType = false);
- QualType mergeFunctionTypes(QualType, QualType, bool OfBlockPointer=false,
- bool Unqualified = false, bool AllowCXX = false);
+ QualType mergeTypes(QualType, QualType, bool OfBlockPointer = false,
+ bool Unqualified = false, bool BlockReturnType = false,
+ bool IsConditionalOperator = false);
+ QualType mergeFunctionTypes(QualType, QualType, bool OfBlockPointer = false,
+ bool Unqualified = false, bool AllowCXX = false,
+ bool IsConditionalOperator = false);
QualType mergeFunctionParameterTypes(QualType, QualType,
bool OfBlockPointer = false,
bool Unqualified = false);
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -335,6 +335,9 @@
incorrect line numbers in some cases when a header file used an end of line
character sequence that differed from the primary source file.
`Issue 59736 <https://github.com/llvm/llvm-project/issues/59736>`_
+- In C mode, when ``e1`` has ``__attribute__((noreturn))`` but ``e2`` doesn't,
+ ``(c ? e1 : e2)`` is no longer considered noreturn.
+ `Issue 59792 <https://github.com/llvm/llvm-project/issues/59792>`_
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
