Sirraide wrote:
Hmm, I’m not sure I’m doing this properly; I’ll try and see if I can open a pr
manually.
https://github.com/llvm/llvm-project/pull/83103
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/lis
llvmbot wrote:
Failed to cherry-pick:
https://github.com/Sirraide/llvm-project/commit/74fa05dead4d52eef3c33406d05dd1bbaf10d546
https://github.com/llvm/llvm-project/actions/runs/8147057328
Please manually backport the fix and push it to your github fork. Once this is
done, please create a [p
Sirraide wrote:
/cherry-pick
https://github.com/Sirraide/llvm-project/commit/74fa05dead4d52eef3c33406d05dd1bbaf10d546
https://github.com/llvm/llvm-project/pull/83103
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bi
llvmbot wrote:
Failed to cherry-pick: d23ef9ef3685eb42ebf719bc28cfe2e4651932fc
https://github.com/llvm/llvm-project/actions/runs/8146857249
Please manually backport the fix and push it to your github fork. Once this is
done, please create a [pull
request](https://github.com/llvm/llvm-projec
Sirraide wrote:
/cherry-pick
https://github.com/llvm/llvm-project/commit/d23ef9ef3685eb42ebf719bc28cfe2e4651932fc
https://github.com/llvm/llvm-project/pull/83103
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/ma
https://github.com/Sirraide milestoned
https://github.com/llvm/llvm-project/pull/83103
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
AaronBallman wrote:
> @AaronBallman @Sirraide would this patch be reasonable to backport to the
> 18.x release branch? It fixes a problem with our application on the
> FreeBSD-CURRENT branch.
> [SerenityOS/serenity#23365](https://github.com/SerenityOS/serenity/issues/23365)
Yeah, I think the
ADKaster wrote:
@AaronBallman @Sirraide would this patch be reasonable to backport to the 18.x
release branch? It fixes a problem with our application on the FreeBSD-CURRENT
branch. https://github.com/SerenityOS/serenity/issues/23365
https://github.com/llvm/llvm-project/pull/83103
https://github.com/AaronBallman closed
https://github.com/llvm/llvm-project/pull/83103
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/AaronBallman updated
https://github.com/llvm/llvm-project/pull/83103
>From 071b6287e89e4601d9e441978f0b4bd53e757c26 Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Tue, 27 Feb 2024 06:19:05 +0100
Subject: [PATCH 1/3] [Clang] [Sema] Handle placeholders in '.*' expressions
---
AaronBallman wrote:
> > LGTM!
>
> Thanks. Just so you know, I still don’t have commit access, so you’d have to
> merge it for me—I should really look into obtaining commit access at this
> point perhaps
Thank you for mentioning that! It turns out that GitHub is confusing:
.
Ah yes, I keep forgetting about that
https://github.com/llvm/llvm-project/pull/83103
___
@@ -14474,6 +14474,23 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation
OpLoc,
CurFPFeatureOverrides());
}
+ // If this is the .* operator, which is not overloadable, just
+ // create a built-in binary operator.
+ if (Opc ==
@@ -14474,6 +14474,23 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation
OpLoc,
CurFPFeatureOverrides());
}
+ // If this is the .* operator, which is not overloadable, just
+ // create a built-in binary operator.
+ if (Opc ==
https://github.com/AaronBallman commented:
Thank you for the fix! Please be sure to add a release note to
`clang/docs/ReleaseNotes.rst` so users know about the fix (and be sure to
mention the issue # in the release note).
https://github.com/llvm/llvm-project/pull/83103
https://github.com/AaronBallman edited
https://github.com/llvm/llvm-project/pull/83103
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Sirraide wrote:
Lastly, it also seems weird to me that a function called
`CreateOverloadedBinOp()` is called to handle `.*`—an operator that can’t be
overloaded—but seeing as this function has apparently been handling this case
for over a decade now, I’m probably not going to question this any
Sirraide wrote:
In my opinion, we ought to get `.*` ought of the way early—as I’m currently
doing in this pr—as it makes little sense to do overloading-specific
placeholder handling on an operator that isn’t even overloadable—we should
instead just handle all placeholders immediately.
https:/
Sirraide wrote:
Yeah, it seems the change that ultimately caused this to break was made in
2011, which moved the handling of placeholders for this code path up into
`SemaOverload.cpp`, and from what I can tell, the case of either operand of
`.*` potentially being an overload set when `.*` is n
Sirraide wrote:
It seems like the assertion has been in Clang since 2011, and back then, we
*were* checking for placeholders in `CreateBuiltinBinOp()`, so at that point it
made sense, but this check seems to have been removed since then.
https://github.com/llvm/llvm-project/pull/83103
___
Sirraide wrote:
One more thing: this code seems to not crash and issue a diagnostic just fine
if we simply remove the assertion, so that would also be an option, but I
didn’t simply want to remove an assertion without fully knowing why it’s there,
so I’ve gone with this as an alternative for n
Sirraide wrote:
CC @AaronBallman, @cor3ntin
https://github.com/llvm/llvm-project/pull/83103
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (Sirraide)
Changes
When analysing whether we should handle a binary expression as an overloaded
operator call or a builtin operator, we were calling
`checkPlaceholderForOverload()`, which takes care of any placeholders that are
not
https://github.com/Sirraide created
https://github.com/llvm/llvm-project/pull/83103
When analysing whether we should handle a binary expression as an overloaded
operator call or a builtin operator, we were calling
`checkPlaceholderForOverload()`, which takes care of any placeholders that are
27 matches
Mail list logo
