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][AST] Don't use canonical type when checking dependence in Type::isOverloadable #98563

Merged
merged 4 commits into from
Jul 24, 2024

Conversation

sdkrystian
Copy link
Member

Fixes #97646.

Still need to add tests.

@sdkrystian sdkrystian requested a review from shafik July 11, 2024 22:42
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 11, 2024

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

Fixes #97646.

Still need to add tests.


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

1 Files Affected:

  • (modified) clang/include/clang/AST/Type.h (+1-1)
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index a2194361abd53..8b236af78ea22 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -8438,7 +8438,7 @@ inline bool Type::isUndeducedType() const {
 /// Determines whether this is a type for which one can define
 /// an overloaded operator.
 inline bool Type::isOverloadableType() const {
-  if (!CanonicalType->isDependentType())
+  if (!isDependentType())
     return isRecordType() || isEnumeralType();
   return !isArrayType() && !isFunctionType() && !isAnyPointerType() &&
          !isMemberPointerType();

@sdkrystian sdkrystian requested a review from mizvekov July 22, 2024 15:08
@sdkrystian sdkrystian force-pushed the fix-is-overloadable branch 2 times, most recently from e4ff4eb to d60492d Compare July 23, 2024 09:31
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.

LGTM, modulo

  • missing release note
  • can you move the test in a GH97646 namespace and put it in decltype.cpp,
    or rename typeof_unqual.cpp to typeof.cpp and put it there?

Maybe, while you are at it, add a test for __typeof_unqual

Thanks!

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.

LGTM

@sdkrystian sdkrystian merged commit 073c199 into llvm:main Jul 24, 2024
6 of 7 checks passed
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…e::isOverloadable (#98563)

Summary: Fixes #97646.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250645
ahatanaka pushed a commit to swiftlang/llvm-project that referenced this pull request Sep 17, 2024
…e::isOverloadable (llvm#98563)

Fixes llvm#97646.
Conflicts:
	clang/docs/ReleaseNotes.rst
	clang/test/SemaCXX/decltype.cpp
ahatanaka pushed a commit to swiftlang/llvm-project that referenced this pull request Sep 17, 2024
…e::isOverloadable (llvm#98563)

Fixes llvm#97646.
Conflicts:
	clang/docs/ReleaseNotes.rst
	clang/test/SemaCXX/decltype.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang crashes in Sema::ImpCastExprToType
3 participants