Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang] Remove __is_layout_compatible from revertible type traits list #100572

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

Endilll
Copy link
Contributor

@Endilll Endilll commented Jul 25, 2024

__is_layout_compatible was added in Clang 19 (#81506), and at that time it wasn't entirely clear whether it should be a revertible type trait or not. We decided to follow the example of other type traits. Since then #95969 happened, and now we know that we don't want new revertible type traits.

This patch removes __is_layout_compatible from revertible type traits list, and leaves a comment what revertible type traits are, and that new type traits should not be added there.

The intention is to also cherry-pick this to 19 branch.

@Endilll Endilll added c++ clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 25, 2024
@Endilll Endilll requested review from cor3ntin and AaronBallman July 25, 2024 13:57
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Jul 25, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 25, 2024

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

__is_layout_compatible was added in Clang 19 (#81506), and at that time it wasn't entirely clear whether it should be a revertible type trait or not. We decided to follow the example of other type traits. Since then #95969 happened, and now we know that we don't want new revertible type traits.

This patch removes __is_layout_compatible from revertible type traits list, and leaves a comment what revertible type traits are, and that new type traits should not be added there.

The intention is to also cherry-pick this to 19 branch.


Full diff: https://github.com/llvm/llvm-project/pull/100572.diff

1 Files Affected:

  • (modified) clang/lib/Parse/ParseExpr.cpp (+3-1)
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 0a017ae79de75..e82b565272831 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -763,6 +763,9 @@ class CastExpressionIdValidator final : public CorrectionCandidateCallback {
 bool Parser::isRevertibleTypeTrait(const IdentifierInfo *II,
                                    tok::TokenKind *Kind) {
   if (RevertibleTypeTraits.empty()) {
+// Revertible type trait is a feature for backwards compatibility with older
+// standard libraries that declare their own structs with the same name as
+// the builtins listed below. New builtins should NOT be added to this list.
 #define RTT_JOIN(X, Y) X##Y
 #define REVERTIBLE_TYPE_TRAIT(Name)                                            \
   RevertibleTypeTraits[PP.getIdentifierInfo(#Name)] = RTT_JOIN(tok::kw_, Name)
@@ -790,7 +793,6 @@ bool Parser::isRevertibleTypeTrait(const IdentifierInfo *II,
     REVERTIBLE_TYPE_TRAIT(__is_fundamental);
     REVERTIBLE_TYPE_TRAIT(__is_integral);
     REVERTIBLE_TYPE_TRAIT(__is_interface_class);
-    REVERTIBLE_TYPE_TRAIT(__is_layout_compatible);
     REVERTIBLE_TYPE_TRAIT(__is_literal);
     REVERTIBLE_TYPE_TRAIT(__is_lvalue_expr);
     REVERTIBLE_TYPE_TRAIT(__is_lvalue_reference);

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Endilll Endilll added this to the LLVM 19.X Release milestone Jul 25, 2024
@philnik777
Copy link
Contributor

Should we also rename to __builtin_is_layout_compatible?

@Endilll
Copy link
Contributor Author

Endilll commented Jul 25, 2024

Should we also rename to __builtin_is_layout_compatible?

Yeah, but not in this patch.

@Endilll Endilll merged commit 3295d37 into llvm:main Jul 25, 2024
11 checks passed
@Endilll Endilll deleted the is-layout-compatible-non-revertible branch July 25, 2024 16:15
@Endilll
Copy link
Contributor Author

Endilll commented Jul 25, 2024

/cherry-pick 3295d37

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 25, 2024
…ist (llvm#100572)

`__is_layout_compatible` was added in Clang 19 (llvm#81506), and at that
time it wasn't entirely clear whether it should be a revertible type
trait or not. We decided to follow the example of other type traits.
Since then llvm#95969 happened, and now we know that we don't want new
revertible type traits.

This patch removes `__is_layout_compatible` from revertible type traits
list, and leaves a comment what revertible type traits are, and that new
type traits should not be added there.

The intention is to also cherry-pick this to 19 branch.

(cherry picked from commit 3295d37)
@llvmbot
Copy link
Member

llvmbot commented Jul 25, 2024

/pull-request #100590

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…ist (#100572)

Summary:
`__is_layout_compatible` was added in Clang 19 (#81506), and at that
time it wasn't entirely clear whether it should be a revertible type
trait or not. We decided to follow the example of other type traits.
Since then #95969 happened, and now we know that we don't want new
revertible type traits.

This patch removes `__is_layout_compatible` from revertible type traits
list, and leaves a comment what revertible type traits are, and that new
type traits should not be added there.

The intention is to also cherry-pick this to 19 branch.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250602
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 26, 2024
…ist (llvm#100572)

`__is_layout_compatible` was added in Clang 19 (llvm#81506), and at that
time it wasn't entirely clear whether it should be a revertible type
trait or not. We decided to follow the example of other type traits.
Since then llvm#95969 happened, and now we know that we don't want new
revertible type traits.

This patch removes `__is_layout_compatible` from revertible type traits
list, and leaves a comment what revertible type traits are, and that new
type traits should not be added there.

The intention is to also cherry-pick this to 19 branch.

(cherry picked from commit 3295d37)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

4 participants