[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
erichkeane wrote: > I’m curious, because I legitimately don’t know, is it common for CI on > windows to take 9 hours? > > ![image](https://private-user-images.githubusercontent.com/74590115/297492800-203deb5a-b798-4ca2-8a6f-cc45a811d0f6.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MDU1MjA4ODksIm5iZiI6MTcwNTUyMDU4OSwicGF0aCI6Ii83NDU5MDExNS8yOTc0OTI4MDAtMjAzZGViNWEtYjc5OC00Y2EyLThhNmYtY2M0NWE4MTFkMGY2LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDAxMTclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwMTE3VDE5NDMwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTYzNjQxY2Q5YmIxMWYwZTdlNTRiZjJhMjg3ZGQyNGJmZmFkNmQ2MmQ3YzA2ZGI5Mjk3YWE0MTZmODk4YjVmYzMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.My4H1GBinAmCmtTNB9o-x4z5eB1WLlPwdDQf9wOcmQQ) It is unfortunately becoming more and more common. The bot infrastructure is having trouble keeping up these days during the day. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
Sirraide wrote: I’m curious, because I legitimately don’t know, is it common for CI on windows to take 9 hours? ![image](https://github.com/llvm/llvm-project/assets/74590115/203deb5a-b798-4ca2-8a6f-cc45a811d0f6) https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
Sirraide wrote: > I'll commit it when I see the CI done, and can do the release notes conflict. > I wont bother re-running after conflict resolution. Alright, thanks! https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
erichkeane wrote: > > Nope! Once the CI is done, feel free to Squash & Merge. > > I would but I don’t have commit access, so someone else would have to do that > for me. (Also, I do fear that in the time it takes for the CI jobs to > complete, we’ll have another merge conflict w/ the release notes because > they’re updated constantly.) I'll commit it when I see the CI done, and can do the release notes conflict. I wont bother re-running after conflict resolution. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
Sirraide wrote: > Nope! Once the CI is done, feel free to Squash & Merge. I would but I don’t have commit access, so someone else would have to do that for me. (Also, I do fear that in the time it takes for the CI jobs to complete, we’ll have another merge conflict w/ the release notes because they’re updated constantly.) https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
erichkeane wrote: > Just did another rebase to update the release notes. > > Is there anything else that needs to be done for this pr? Nope! Once the CI is done, feel free to Squash & Merge. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
Sirraide wrote: Just did another rebase to update the release notes. Is there anything else that needs to be done for this pr? https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/75883 >From 849b21f202f3c643c12f237920b137c3506996e7 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 19 Dec 2023 02:48:25 +0100 Subject: [PATCH 1/6] [Clang] Support MSPropertyRefExpr as placement arg to new-expression --- clang/include/clang/Sema/Sema.h | 4 ++ clang/lib/Sema/SemaExpr.cpp | 12 ++ clang/lib/Sema/SemaExprCXX.cpp| 3 ++ clang/test/CodeGenCXX/ms-property-new.cpp | 52 +++ clang/test/SemaCXX/ms-property-new.cpp| 44 +++ 5 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 clang/test/CodeGenCXX/ms-property-new.cpp create mode 100644 clang/test/SemaCXX/ms-property-new.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4ef1fe542ea54f0..aeb4075374022fa 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2108,6 +2108,10 @@ class Sema final { bool CheckFunctionReturnType(QualType T, SourceLocation Loc); + /// Check an argument list for placeholders that we won't try to + /// handle later. + bool CheckArgsForPlaceholders(MultiExprArg args); + /// Build a function type. /// /// This routine checks the function type according to C++ rules and diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 049fdae09bb185c..fcc227cf7a74f64 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5058,8 +5058,6 @@ static QualType getDependentArraySubscriptType(Expr *LHS, Expr *RHS, return Result->isDependentType() ? Result : Ctx.DependentTy; } -static bool checkArgsForPlaceholders(Sema , MultiExprArg args); - ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, SourceLocation lbLoc, MultiExprArg ArgExprs, @@ -5163,7 +5161,7 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, return ExprError(); ArgExprs[0] = result.get(); } else { -if (checkArgsForPlaceholders(*this, ArgExprs)) +if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); } @@ -6912,15 +6910,13 @@ static bool isPlaceholderToRemoveAsArg(QualType type) { llvm_unreachable("bad builtin type kind"); } -/// Check an argument list for placeholders that we won't try to -/// handle later. -static bool checkArgsForPlaceholders(Sema , MultiExprArg args) { +bool Sema::CheckArgsForPlaceholders(MultiExprArg args) { // Apply this processing to all the arguments at once instead of // dying at the first failure. bool hasInvalid = false; for (size_t i = 0, e = args.size(); i != e; i++) { if (isPlaceholderToRemoveAsArg(args[i]->getType())) { - ExprResult result = S.CheckPlaceholderExpr(args[i]); + ExprResult result = CheckPlaceholderExpr(args[i]); if (result.isInvalid()) hasInvalid = true; else args[i] = result.get(); } @@ -7194,7 +7190,7 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc, if (Result.isInvalid()) return ExprError(); Fn = Result.get(); - if (checkArgsForPlaceholders(*this, ArgExprs)) + if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); if (getLangOpts().CPlusPlus) { diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 4ae04358d5df7c8..0ca9e5b59e3c329 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) +return ExprError(); + AllocationFunctionScope Scope = UseGlobal ? AFS_Global : AFS_Both; if (!AllocType->isDependentType() && !Expr::hasAnyTypeDependentArguments(PlacementArgs) && diff --git a/clang/test/CodeGenCXX/ms-property-new.cpp b/clang/test/CodeGenCXX/ms-property-new.cpp new file mode 100644 index 000..e21ec3d6702f627 --- /dev/null +++ b/clang/test/CodeGenCXX/ms-property-new.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +struct S { + int GetX() { return 42; } + __declspec(property(get=GetX)) int x; + + int GetY(int i, int j) { return i+j; } + __declspec(property(get=GetY)) int y[]; + + void* operator new(__SIZE_TYPE__, int); +}; + +template +struct TS { + T GetT() { return T(); } + __declspec(property(get=GetT)) T t; + + T GetR(T i, T j) { return i+j; } + __declspec(property(get=GetR)) T
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
Sirraide wrote: Just did a rebase and also updated the release notes to mention these changes https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/75883 >From c46f8efd68bf5c32164a64de36081e65a7171763 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 19 Dec 2023 02:48:25 +0100 Subject: [PATCH 1/6] [Clang] Support MSPropertyRefExpr as placement arg to new-expression --- clang/include/clang/Sema/Sema.h | 4 ++ clang/lib/Sema/SemaExpr.cpp | 12 ++ clang/lib/Sema/SemaExprCXX.cpp| 3 ++ clang/test/CodeGenCXX/ms-property-new.cpp | 52 +++ clang/test/SemaCXX/ms-property-new.cpp| 44 +++ 5 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 clang/test/CodeGenCXX/ms-property-new.cpp create mode 100644 clang/test/SemaCXX/ms-property-new.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 8f44adef38159e..e4a8cef0a91bff 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2108,6 +2108,10 @@ class Sema final { bool CheckFunctionReturnType(QualType T, SourceLocation Loc); + /// Check an argument list for placeholders that we won't try to + /// handle later. + bool CheckArgsForPlaceholders(MultiExprArg args); + /// Build a function type. /// /// This routine checks the function type according to C++ rules and diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 960f513db2..8a70c4166ae8cb 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5056,8 +5056,6 @@ static QualType getDependentArraySubscriptType(Expr *LHS, Expr *RHS, return Result->isDependentType() ? Result : Ctx.DependentTy; } -static bool checkArgsForPlaceholders(Sema , MultiExprArg args); - ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, SourceLocation lbLoc, MultiExprArg ArgExprs, @@ -5161,7 +5159,7 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, return ExprError(); ArgExprs[0] = result.get(); } else { -if (checkArgsForPlaceholders(*this, ArgExprs)) +if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); } @@ -6910,15 +6908,13 @@ static bool isPlaceholderToRemoveAsArg(QualType type) { llvm_unreachable("bad builtin type kind"); } -/// Check an argument list for placeholders that we won't try to -/// handle later. -static bool checkArgsForPlaceholders(Sema , MultiExprArg args) { +bool Sema::CheckArgsForPlaceholders(MultiExprArg args) { // Apply this processing to all the arguments at once instead of // dying at the first failure. bool hasInvalid = false; for (size_t i = 0, e = args.size(); i != e; i++) { if (isPlaceholderToRemoveAsArg(args[i]->getType())) { - ExprResult result = S.CheckPlaceholderExpr(args[i]); + ExprResult result = CheckPlaceholderExpr(args[i]); if (result.isInvalid()) hasInvalid = true; else args[i] = result.get(); } @@ -7192,7 +7188,7 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc, if (Result.isInvalid()) return ExprError(); Fn = Result.get(); - if (checkArgsForPlaceholders(*this, ArgExprs)) + if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); if (getLangOpts().CPlusPlus) { diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 4ae04358d5df7c..0ca9e5b59e3c32 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) +return ExprError(); + AllocationFunctionScope Scope = UseGlobal ? AFS_Global : AFS_Both; if (!AllocType->isDependentType() && !Expr::hasAnyTypeDependentArguments(PlacementArgs) && diff --git a/clang/test/CodeGenCXX/ms-property-new.cpp b/clang/test/CodeGenCXX/ms-property-new.cpp new file mode 100644 index 00..e21ec3d6702f62 --- /dev/null +++ b/clang/test/CodeGenCXX/ms-property-new.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +struct S { + int GetX() { return 42; } + __declspec(property(get=GetX)) int x; + + int GetY(int i, int j) { return i+j; } + __declspec(property(get=GetY)) int y[]; + + void* operator new(__SIZE_TYPE__, int); +}; + +template +struct TS { + T GetT() { return T(); } + __declspec(property(get=GetT)) T t; + + T GetR(T i, T j) { return i+j; } + __declspec(property(get=GetR)) T r[]; +}; +
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/75883 >From c46f8efd68bf5c32164a64de36081e65a7171763 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 19 Dec 2023 02:48:25 +0100 Subject: [PATCH 1/5] [Clang] Support MSPropertyRefExpr as placement arg to new-expression --- clang/include/clang/Sema/Sema.h | 4 ++ clang/lib/Sema/SemaExpr.cpp | 12 ++ clang/lib/Sema/SemaExprCXX.cpp| 3 ++ clang/test/CodeGenCXX/ms-property-new.cpp | 52 +++ clang/test/SemaCXX/ms-property-new.cpp| 44 +++ 5 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 clang/test/CodeGenCXX/ms-property-new.cpp create mode 100644 clang/test/SemaCXX/ms-property-new.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 8f44adef38159e..e4a8cef0a91bff 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2108,6 +2108,10 @@ class Sema final { bool CheckFunctionReturnType(QualType T, SourceLocation Loc); + /// Check an argument list for placeholders that we won't try to + /// handle later. + bool CheckArgsForPlaceholders(MultiExprArg args); + /// Build a function type. /// /// This routine checks the function type according to C++ rules and diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 960f513db2..8a70c4166ae8cb 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5056,8 +5056,6 @@ static QualType getDependentArraySubscriptType(Expr *LHS, Expr *RHS, return Result->isDependentType() ? Result : Ctx.DependentTy; } -static bool checkArgsForPlaceholders(Sema , MultiExprArg args); - ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, SourceLocation lbLoc, MultiExprArg ArgExprs, @@ -5161,7 +5159,7 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, return ExprError(); ArgExprs[0] = result.get(); } else { -if (checkArgsForPlaceholders(*this, ArgExprs)) +if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); } @@ -6910,15 +6908,13 @@ static bool isPlaceholderToRemoveAsArg(QualType type) { llvm_unreachable("bad builtin type kind"); } -/// Check an argument list for placeholders that we won't try to -/// handle later. -static bool checkArgsForPlaceholders(Sema , MultiExprArg args) { +bool Sema::CheckArgsForPlaceholders(MultiExprArg args) { // Apply this processing to all the arguments at once instead of // dying at the first failure. bool hasInvalid = false; for (size_t i = 0, e = args.size(); i != e; i++) { if (isPlaceholderToRemoveAsArg(args[i]->getType())) { - ExprResult result = S.CheckPlaceholderExpr(args[i]); + ExprResult result = CheckPlaceholderExpr(args[i]); if (result.isInvalid()) hasInvalid = true; else args[i] = result.get(); } @@ -7192,7 +7188,7 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc, if (Result.isInvalid()) return ExprError(); Fn = Result.get(); - if (checkArgsForPlaceholders(*this, ArgExprs)) + if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); if (getLangOpts().CPlusPlus) { diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 4ae04358d5df7c..0ca9e5b59e3c32 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) +return ExprError(); + AllocationFunctionScope Scope = UseGlobal ? AFS_Global : AFS_Both; if (!AllocType->isDependentType() && !Expr::hasAnyTypeDependentArguments(PlacementArgs) && diff --git a/clang/test/CodeGenCXX/ms-property-new.cpp b/clang/test/CodeGenCXX/ms-property-new.cpp new file mode 100644 index 00..e21ec3d6702f62 --- /dev/null +++ b/clang/test/CodeGenCXX/ms-property-new.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +struct S { + int GetX() { return 42; } + __declspec(property(get=GetX)) int x; + + int GetY(int i, int j) { return i+j; } + __declspec(property(get=GetY)) int y[]; + + void* operator new(__SIZE_TYPE__, int); +}; + +template +struct TS { + T GetT() { return T(); } + __declspec(property(get=GetT)) T t; + + T GetR(T i, T j) { return i+j; } + __declspec(property(get=GetR)) T r[]; +}; +
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -130,27 +137,24 @@ C &&()(C&) = std::move; // #8 expected-note {{instantiation of}} C &&()(C&) = std::forward; // #9 expected-note {{instantiation of}} int ()(B, B) = std::move; -#if __cplusplus <= 201703L -// expected-warning@#1 {{non-addressable}} -// expected-warning@#2 {{non-addressable}} -// expected-warning@#3 {{non-addressable}} -// expected-warning@#4 {{non-addressable}} -// expected-warning@#5 {{non-addressable}} -// expected-warning@#6 {{non-addressable}} -// expected-warning@#7 {{non-addressable}} -// expected-warning@#8 {{non-addressable}} -// expected-warning@#9 {{non-addressable}} -#else -// expected-error@#1 {{non-addressable}} -// expected-error@#2 {{non-addressable}} -// expected-error@#3 {{non-addressable}} -// expected-error@#4 {{non-addressable}} -// expected-error@#5 {{non-addressable}} -// expected-error@#6 {{non-addressable}} -// expected-error@#7 {{non-addressable}} -// expected-error@#8 {{non-addressable}} -// expected-error@#9 {{non-addressable}} -#endif +// cxx17-warning@#1 {{non-addressable}} Sirraide wrote: I’ve put them on the lines that cause the diagnostics and also aligned them because I found it to be too unreadable otherwise. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/75883 >From f51b382fd33d4cf3f75bdaa172bb8697a6cadc9e Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 19 Dec 2023 02:48:25 +0100 Subject: [PATCH 1/5] [Clang] Support MSPropertyRefExpr as placement arg to new-expression --- clang/include/clang/Sema/Sema.h | 4 ++ clang/lib/Sema/SemaExpr.cpp | 12 ++ clang/lib/Sema/SemaExprCXX.cpp| 3 ++ clang/test/CodeGenCXX/ms-property-new.cpp | 52 +++ clang/test/SemaCXX/ms-property-new.cpp| 44 +++ 5 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 clang/test/CodeGenCXX/ms-property-new.cpp create mode 100644 clang/test/SemaCXX/ms-property-new.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index a4f8fc1845b1ce..90be0a648f0124 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2108,6 +2108,10 @@ class Sema final { bool CheckFunctionReturnType(QualType T, SourceLocation Loc); + /// Check an argument list for placeholders that we won't try to + /// handle later. + bool CheckArgsForPlaceholders(MultiExprArg args); + /// Build a function type. /// /// This routine checks the function type according to C++ rules and diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index c7185d56cc9973..2442c1e104055b 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5058,8 +5058,6 @@ static QualType getDependentArraySubscriptType(Expr *LHS, Expr *RHS, return Result->isDependentType() ? Result : Ctx.DependentTy; } -static bool checkArgsForPlaceholders(Sema , MultiExprArg args); - ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, SourceLocation lbLoc, MultiExprArg ArgExprs, @@ -5163,7 +5161,7 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, return ExprError(); ArgExprs[0] = result.get(); } else { -if (checkArgsForPlaceholders(*this, ArgExprs)) +if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); } @@ -6912,15 +6910,13 @@ static bool isPlaceholderToRemoveAsArg(QualType type) { llvm_unreachable("bad builtin type kind"); } -/// Check an argument list for placeholders that we won't try to -/// handle later. -static bool checkArgsForPlaceholders(Sema , MultiExprArg args) { +bool Sema::CheckArgsForPlaceholders(MultiExprArg args) { // Apply this processing to all the arguments at once instead of // dying at the first failure. bool hasInvalid = false; for (size_t i = 0, e = args.size(); i != e; i++) { if (isPlaceholderToRemoveAsArg(args[i]->getType())) { - ExprResult result = S.CheckPlaceholderExpr(args[i]); + ExprResult result = CheckPlaceholderExpr(args[i]); if (result.isInvalid()) hasInvalid = true; else args[i] = result.get(); } @@ -7194,7 +7190,7 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc, if (Result.isInvalid()) return ExprError(); Fn = Result.get(); - if (checkArgsForPlaceholders(*this, ArgExprs)) + if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); if (getLangOpts().CPlusPlus) { diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 081b568762ae22..49f0921ce324fc 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) +return ExprError(); + AllocationFunctionScope Scope = UseGlobal ? AFS_Global : AFS_Both; if (!AllocType->isDependentType() && !Expr::hasAnyTypeDependentArguments(PlacementArgs) && diff --git a/clang/test/CodeGenCXX/ms-property-new.cpp b/clang/test/CodeGenCXX/ms-property-new.cpp new file mode 100644 index 00..e21ec3d6702f62 --- /dev/null +++ b/clang/test/CodeGenCXX/ms-property-new.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +struct S { + int GetX() { return 42; } + __declspec(property(get=GetX)) int x; + + int GetY(int i, int j) { return i+j; } + __declspec(property(get=GetY)) int y[]; + + void* operator new(__SIZE_TYPE__, int); +}; + +template +struct TS { + T GetT() { return T(); } + __declspec(property(get=GetT)) T t; + + T GetR(T i, T j) { return i+j; } + __declspec(property(get=GetR)) T r[]; +}; +
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -130,27 +137,24 @@ C &&()(C&) = std::move; // #8 expected-note {{instantiation of}} C &&()(C&) = std::forward; // #9 expected-note {{instantiation of}} int ()(B, B) = std::move; -#if __cplusplus <= 201703L -// expected-warning@#1 {{non-addressable}} -// expected-warning@#2 {{non-addressable}} -// expected-warning@#3 {{non-addressable}} -// expected-warning@#4 {{non-addressable}} -// expected-warning@#5 {{non-addressable}} -// expected-warning@#6 {{non-addressable}} -// expected-warning@#7 {{non-addressable}} -// expected-warning@#8 {{non-addressable}} -// expected-warning@#9 {{non-addressable}} -#else -// expected-error@#1 {{non-addressable}} -// expected-error@#2 {{non-addressable}} -// expected-error@#3 {{non-addressable}} -// expected-error@#4 {{non-addressable}} -// expected-error@#5 {{non-addressable}} -// expected-error@#6 {{non-addressable}} -// expected-error@#7 {{non-addressable}} -// expected-error@#8 {{non-addressable}} -// expected-error@#9 {{non-addressable}} -#endif +// cxx17-warning@#1 {{non-addressable}} erichkeane wrote: Even better! https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -130,27 +137,24 @@ C &&()(C&) = std::move; // #8 expected-note {{instantiation of}} C &&()(C&) = std::forward; // #9 expected-note {{instantiation of}} int ()(B, B) = std::move; -#if __cplusplus <= 201703L -// expected-warning@#1 {{non-addressable}} -// expected-warning@#2 {{non-addressable}} -// expected-warning@#3 {{non-addressable}} -// expected-warning@#4 {{non-addressable}} -// expected-warning@#5 {{non-addressable}} -// expected-warning@#6 {{non-addressable}} -// expected-warning@#7 {{non-addressable}} -// expected-warning@#8 {{non-addressable}} -// expected-warning@#9 {{non-addressable}} -#else -// expected-error@#1 {{non-addressable}} -// expected-error@#2 {{non-addressable}} -// expected-error@#3 {{non-addressable}} -// expected-error@#4 {{non-addressable}} -// expected-error@#5 {{non-addressable}} -// expected-error@#6 {{non-addressable}} -// expected-error@#7 {{non-addressable}} -// expected-error@#8 {{non-addressable}} -// expected-error@#9 {{non-addressable}} -#endif +// cxx17-warning@#1 {{non-addressable}} Endilll wrote: I'd put them directly after the lines they are issues for, instead of gathering them in one place and asking readers to follow the markers. Then they will be naturally collated by line instead of language mode. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -130,27 +137,24 @@ C &&()(C&) = std::move; // #8 expected-note {{instantiation of}} C &&()(C&) = std::forward; // #9 expected-note {{instantiation of}} int ()(B, B) = std::move; -#if __cplusplus <= 201703L -// expected-warning@#1 {{non-addressable}} -// expected-warning@#2 {{non-addressable}} -// expected-warning@#3 {{non-addressable}} -// expected-warning@#4 {{non-addressable}} -// expected-warning@#5 {{non-addressable}} -// expected-warning@#6 {{non-addressable}} -// expected-warning@#7 {{non-addressable}} -// expected-warning@#8 {{non-addressable}} -// expected-warning@#9 {{non-addressable}} -#else -// expected-error@#1 {{non-addressable}} -// expected-error@#2 {{non-addressable}} -// expected-error@#3 {{non-addressable}} -// expected-error@#4 {{non-addressable}} -// expected-error@#5 {{non-addressable}} -// expected-error@#6 {{non-addressable}} -// expected-error@#7 {{non-addressable}} -// expected-error@#8 {{non-addressable}} -// expected-error@#9 {{non-addressable}} -#endif +// cxx17-warning@#1 {{non-addressable}} erichkeane wrote: I might suggest 'collating' these a bit, that is, put all of the ones for the same line number 'next' to eachother, rather than by language mode. But I don't feel too strongly. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -163,6 +170,40 @@ void attribute_const() { std::as_const(n); // expected-warning {{ignoring return value}} } +struct D { + void* operator new(__SIZE_TYPE__, D&&(*)(D&)); + void* operator new(__SIZE_TYPE__, D*(*)(D&)); + void* operator new(__SIZE_TYPE__, const D&(*)(D&)); +}; + +#if __cplusplus <= 201703L Sirraide wrote: Alright, done https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/75883 >From f51b382fd33d4cf3f75bdaa172bb8697a6cadc9e Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 19 Dec 2023 02:48:25 +0100 Subject: [PATCH 1/4] [Clang] Support MSPropertyRefExpr as placement arg to new-expression --- clang/include/clang/Sema/Sema.h | 4 ++ clang/lib/Sema/SemaExpr.cpp | 12 ++ clang/lib/Sema/SemaExprCXX.cpp| 3 ++ clang/test/CodeGenCXX/ms-property-new.cpp | 52 +++ clang/test/SemaCXX/ms-property-new.cpp| 44 +++ 5 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 clang/test/CodeGenCXX/ms-property-new.cpp create mode 100644 clang/test/SemaCXX/ms-property-new.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index a4f8fc1845b1ce..90be0a648f0124 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2108,6 +2108,10 @@ class Sema final { bool CheckFunctionReturnType(QualType T, SourceLocation Loc); + /// Check an argument list for placeholders that we won't try to + /// handle later. + bool CheckArgsForPlaceholders(MultiExprArg args); + /// Build a function type. /// /// This routine checks the function type according to C++ rules and diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index c7185d56cc9973..2442c1e104055b 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5058,8 +5058,6 @@ static QualType getDependentArraySubscriptType(Expr *LHS, Expr *RHS, return Result->isDependentType() ? Result : Ctx.DependentTy; } -static bool checkArgsForPlaceholders(Sema , MultiExprArg args); - ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, SourceLocation lbLoc, MultiExprArg ArgExprs, @@ -5163,7 +5161,7 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, return ExprError(); ArgExprs[0] = result.get(); } else { -if (checkArgsForPlaceholders(*this, ArgExprs)) +if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); } @@ -6912,15 +6910,13 @@ static bool isPlaceholderToRemoveAsArg(QualType type) { llvm_unreachable("bad builtin type kind"); } -/// Check an argument list for placeholders that we won't try to -/// handle later. -static bool checkArgsForPlaceholders(Sema , MultiExprArg args) { +bool Sema::CheckArgsForPlaceholders(MultiExprArg args) { // Apply this processing to all the arguments at once instead of // dying at the first failure. bool hasInvalid = false; for (size_t i = 0, e = args.size(); i != e; i++) { if (isPlaceholderToRemoveAsArg(args[i]->getType())) { - ExprResult result = S.CheckPlaceholderExpr(args[i]); + ExprResult result = CheckPlaceholderExpr(args[i]); if (result.isInvalid()) hasInvalid = true; else args[i] = result.get(); } @@ -7194,7 +7190,7 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc, if (Result.isInvalid()) return ExprError(); Fn = Result.get(); - if (checkArgsForPlaceholders(*this, ArgExprs)) + if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); if (getLangOpts().CPlusPlus) { diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 081b568762ae22..49f0921ce324fc 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) +return ExprError(); + AllocationFunctionScope Scope = UseGlobal ? AFS_Global : AFS_Both; if (!AllocType->isDependentType() && !Expr::hasAnyTypeDependentArguments(PlacementArgs) && diff --git a/clang/test/CodeGenCXX/ms-property-new.cpp b/clang/test/CodeGenCXX/ms-property-new.cpp new file mode 100644 index 00..e21ec3d6702f62 --- /dev/null +++ b/clang/test/CodeGenCXX/ms-property-new.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +struct S { + int GetX() { return 42; } + __declspec(property(get=GetX)) int x; + + int GetY(int i, int j) { return i+j; } + __declspec(property(get=GetY)) int y[]; + + void* operator new(__SIZE_TYPE__, int); +}; + +template +struct TS { + T GetT() { return T(); } + __declspec(property(get=GetT)) T t; + + T GetR(T i, T j) { return i+j; } + __declspec(property(get=GetR)) T r[]; +}; +
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -163,6 +170,40 @@ void attribute_const() { std::as_const(n); // expected-warning {{ignoring return value}} } +struct D { + void* operator new(__SIZE_TYPE__, D&&(*)(D&)); + void* operator new(__SIZE_TYPE__, D*(*)(D&)); + void* operator new(__SIZE_TYPE__, const D&(*)(D&)); +}; + +#if __cplusplus <= 201703L Sirraide wrote: > I realize elsewhere does this... but please use `-verify=cpp20,expected` and > `-verify=cpp17,expected` Ah, thanks, I didn’t know about those. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -163,6 +170,40 @@ void attribute_const() { std::as_const(n); // expected-warning {{ignoring return value}} } +struct D { + void* operator new(__SIZE_TYPE__, D&&(*)(D&)); + void* operator new(__SIZE_TYPE__, D*(*)(D&)); + void* operator new(__SIZE_TYPE__, const D&(*)(D&)); +}; + +#if __cplusplus <= 201703L Endilll wrote: Here's a good example of every kind of `verify` prefixes I've been using in C++ DR tests: https://github.com/llvm/llvm-project/blob/569ec185f5dc4a9e4a239948191977ecc2b2b475/clang/test/CXX/drs/dr6xx.cpp#L1-L6 https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -163,6 +170,40 @@ void attribute_const() { std::as_const(n); // expected-warning {{ignoring return value}} } +struct D { + void* operator new(__SIZE_TYPE__, D&&(*)(D&)); + void* operator new(__SIZE_TYPE__, D*(*)(D&)); + void* operator new(__SIZE_TYPE__, const D&(*)(D&)); +}; + +#if __cplusplus <= 201703L erichkeane wrote: I realize elsewhere does this... but please use `-verify=cpp20,expected` and `-verify=cpp17,expected`, then make these `cpp17-warning` and `cpp20-error`. An NFC followup to clean up the other uses here would also be appreciated, but not necessary. @Endilll has been doing similar cleanups elsewhere. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/erichkeane commented: 1 issue with the test, else LGTM. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/Sirraide edited https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) Sirraide wrote: Alright, just got done adding some tests. Just let me know if there are more things that we should consider testing. Also, regarding Objective-C++ properties: I tried following the examples in `clang/test/AST/ast-dump-expr-json.m`, but it didn’t seem to want to treat the subscripts as anything other than pointer arithmetic. It also doesn’t help that I’m not familiar w/ Objective-C/Objective-C++ at all. Could it be that Objective-C subscript properties are disabled in Objective-C++ mode? https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/75883 >From f51b382fd33d4cf3f75bdaa172bb8697a6cadc9e Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 19 Dec 2023 02:48:25 +0100 Subject: [PATCH 1/3] [Clang] Support MSPropertyRefExpr as placement arg to new-expression --- clang/include/clang/Sema/Sema.h | 4 ++ clang/lib/Sema/SemaExpr.cpp | 12 ++ clang/lib/Sema/SemaExprCXX.cpp| 3 ++ clang/test/CodeGenCXX/ms-property-new.cpp | 52 +++ clang/test/SemaCXX/ms-property-new.cpp| 44 +++ 5 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 clang/test/CodeGenCXX/ms-property-new.cpp create mode 100644 clang/test/SemaCXX/ms-property-new.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index a4f8fc1845b1ce..90be0a648f0124 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2108,6 +2108,10 @@ class Sema final { bool CheckFunctionReturnType(QualType T, SourceLocation Loc); + /// Check an argument list for placeholders that we won't try to + /// handle later. + bool CheckArgsForPlaceholders(MultiExprArg args); + /// Build a function type. /// /// This routine checks the function type according to C++ rules and diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index c7185d56cc9973..2442c1e104055b 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5058,8 +5058,6 @@ static QualType getDependentArraySubscriptType(Expr *LHS, Expr *RHS, return Result->isDependentType() ? Result : Ctx.DependentTy; } -static bool checkArgsForPlaceholders(Sema , MultiExprArg args); - ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, SourceLocation lbLoc, MultiExprArg ArgExprs, @@ -5163,7 +5161,7 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, return ExprError(); ArgExprs[0] = result.get(); } else { -if (checkArgsForPlaceholders(*this, ArgExprs)) +if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); } @@ -6912,15 +6910,13 @@ static bool isPlaceholderToRemoveAsArg(QualType type) { llvm_unreachable("bad builtin type kind"); } -/// Check an argument list for placeholders that we won't try to -/// handle later. -static bool checkArgsForPlaceholders(Sema , MultiExprArg args) { +bool Sema::CheckArgsForPlaceholders(MultiExprArg args) { // Apply this processing to all the arguments at once instead of // dying at the first failure. bool hasInvalid = false; for (size_t i = 0, e = args.size(); i != e; i++) { if (isPlaceholderToRemoveAsArg(args[i]->getType())) { - ExprResult result = S.CheckPlaceholderExpr(args[i]); + ExprResult result = CheckPlaceholderExpr(args[i]); if (result.isInvalid()) hasInvalid = true; else args[i] = result.get(); } @@ -7194,7 +7190,7 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc, if (Result.isInvalid()) return ExprError(); Fn = Result.get(); - if (checkArgsForPlaceholders(*this, ArgExprs)) + if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); if (getLangOpts().CPlusPlus) { diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 081b568762ae22..49f0921ce324fc 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) +return ExprError(); + AllocationFunctionScope Scope = UseGlobal ? AFS_Global : AFS_Both; if (!AllocType->isDependentType() && !Expr::hasAnyTypeDependentArguments(PlacementArgs) && diff --git a/clang/test/CodeGenCXX/ms-property-new.cpp b/clang/test/CodeGenCXX/ms-property-new.cpp new file mode 100644 index 00..e21ec3d6702f62 --- /dev/null +++ b/clang/test/CodeGenCXX/ms-property-new.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +struct S { + int GetX() { return 42; } + __declspec(property(get=GetX)) int x; + + int GetY(int i, int j) { return i+j; } + __declspec(property(get=GetY)) int y[]; + + void* operator new(__SIZE_TYPE__, int); +}; + +template +struct TS { + T GetT() { return T(); } + __declspec(property(get=GetT)) T t; + + T GetR(T i, T j) { return i+j; } + __declspec(property(get=GetR)) T r[]; +}; +
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/Sirraide edited https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) Sirraide wrote: Oh, and Objective-C++ properties were suffering from the same problem, so this now also fixes those in new-expressions. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) Sirraide wrote: tl;dr Overall, it seems like this pr also improves some error messages as well as support for MSVC’s `__noop` builtin. I’ll add some tests for the cases above (though again, for some of them, (e.g. the UnknownAny case), I’m not sure whether that’s necessary, so please let me know whether I should add some for those as well). https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) Sirraide wrote: The only expressions that this seems to affect is expressions for which `isPlaceholderToRemoveAsArg` returns `true` by passing them to `CheckPlaceholderExpr`; this only affects a handful of types: - `BuiltinType::PseudoObject`: MSProperty (subscript) properties and ObjC (subscript) properties; the latter are only relevant for Objective-C++ in this case. I’ll add tests for the latter as well (to the best of my abilities as I have never written a line of Objective-C or Objective-C++ before in my life). The comment in `BuiltinTypes.def` also mentions ‘VS.NET's `__property` declarations’, but I couldn’t find anything about `__property` in the rest of the codebase, and I’m candidly not entirely sure what this is in reference to, so I’m only adding tests for Objective-C++ for now. - `BuiltinType::UnknownAny`: For this, `CheckPlaceholderExpr` just emits an error, and I personally don’t see a way in which this would ever be valid as a placement-arg to a new expression, but I could be wrong here. Should we add tests for this case? - `BuiltinType::BoundMember`: `s.foo` and friends, where `foo` is a non-static member function; these are already invalid in placement-args, and `CheckPlaceholderExpr` similarly issues what looks to be the exact same error; we don’t seem to have any tests that check for this error, however, so I’ll add some for this as well. - `BuiltinType::BuiltinFn`: Only `DeclRefExpr`s are affected here as this simply issues an error otherwise (which should be fine because passing a builtin function as a function argument doesn’t make sense anyway); if it is a builtin such as `__builtin_memset`, then this pr also ends up improving the error message from ‘no known conversion from ''’ to ‘builtin functions must be directly called’. If it is a `DeclRefExpr`, there are two cases to consider: the first is the MS `__noop` builtin, which w/o this pr simply errors, but now, it is implicitly converted to a `CallExpr` that returns an `int` (which emits 0 in CG); this matches MSVCs behaviour, but I doubt we have a test for that, so I’ll add one as well. The only other case that is affected here is if we have a standard-namespace builtin (e.g. `std::move`, `std::forward_like`, which before C++20 resulted in a warning, since C++ in an error; this behaviour is unaffected by this change, but I’ll some tests for this as well because from what I can tell, there currently are no tests for this diagnostic at all. - `BuiltinType::IncompleteMatrixIdx`: From what I can tell, this is used for [matrix types](https://clang.llvm.org/docs/MatrixTypes.html), specifically, for the type of an expression such as `a[1]` where `a` is a matrix type because matrix types always require two subscripts; since an expression of this type as a placement-arg is thus always invalid, this should error, but previously, we were emitting a less-than-ideal error (e.g. ‘no known conversion from '' to 'int'’), whereas with this pr, we now get the error ‘single subscript expressions are not allowed for matrix values’. There does not seem to be a test for this that involves placement-new, so I’ll add a few as well. - `BuiltinType::OMPArraySection`, `BuiltinType::OMPArrayShaping`, and `BuiltinType::OMPIterator`: To my understanding, these cannot occur outside of OMP `#pragma`s, so they’re not relevant here. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) Sirraide wrote: Makes sense. It seems like this affects only a small number of builtin types, so in theory, adding tests for every expression that could possibly have those types ought to do (if we don’t already have tests for those); I’m currently collecting a list of everything that could be affected by this. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) erichkeane wrote: A bit of both, this should check the behavior of other expression types. This changes general 'placement new' behaviors, so it looks like it changes existing placement new behaviors that are seemingly untested, so should be showing off THOSE as well. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) Sirraide wrote: I’ll take a look at what other expressions are actually affected by this other than `MSPropertyRefExpr`s. (or do you mean we should just add more placement-new tests in general? Because I would assume we already have a bunch of those, but I haven’t checked to be fair?) https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +struct S { + int GetX() { return 42; } + __declspec(property(get=GetX)) int x; + + int GetY(int i, int j) { return i+j; } + __declspec(property(get=GetY)) int y[]; + + void* operator new(__SIZE_TYPE__, int); +}; + +template +struct TS { + T GetT() { return T(); } + __declspec(property(get=GetT)) T t; + + T GetR(T i, T j) { return i+j; } + __declspec(property(get=GetR)) T r[]; +}; + +// CHECK-LABEL: main +int main(int argc, char **argv) { + S *s; + TS *ts; + + // CHECK: [[X:%.+]] = call noundef i32 @"?GetX@S@@QEAAHXZ"(ptr {{[^,]*}} %{{.+}}) + // CHECK-NEXT: call noundef ptr @"??2S@@SAPEAX_KH@Z"(i64 noundef 1, i32 noundef [[X]]) + new (s->x) S; + + // CHECK: [[Y:%.+]] = call noundef i32 @"?GetY@S@@QEAAHHH@Z"(ptr {{[^,]*}} %{{.+}}, i32 noundef 1, i32 noundef 2) + // CHECK-NEXT: call noundef ptr @"??2S@@SAPEAX_KH@Z"(i64 noundef 1, i32 noundef [[Y]]) + new ((s->y)[1][2]) S; + + // CHECK: [[T:%.+]] = call noundef double @"?GetT@?$TS@N@@QEAANXZ"(ptr {{[^,]*}} %{{.+}}) + // CHECK-NEXT: [[TI:%.+]] = fptosi double [[T]] to i32 + // CHECK-NEXT: call noundef ptr @"??2S@@SAPEAX_KH@Z"(i64 noundef 1, i32 noundef [[TI]]) + new (ts->t) S; + + // CHECK: [[R:%.+]] = call noundef double @"?GetR@?$TS@N@@QEAANNN@Z"(ptr {{[^,]*}} %{{.+}}, double {{[^,]*}}, double {{[^,]*}}) + // CHECK-NEXT: [[RI:%.+]] = fptosi double [[R]] to i32 + // CHECK-NEXT: call noundef ptr @"??2S@@SAPEAX_KH@Z"(i64 noundef 1, i32 noundef [[RI]]) + new ((ts->r)[1][2]) S; +} + +#endif Sirraide wrote: Ah yes, I always forget about that https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +struct S { + int GetX() { return 42; } + __declspec(property(get=GetX)) int x; + + int GetY(int i, int j) { return i+j; } + __declspec(property(get=GetY)) int y[]; + + void* operator new(__SIZE_TYPE__, int); +}; + +template +struct TS { + T GetT() { return T(); } + __declspec(property(get=GetT)) T t; + + T GetR(T i, T j) { return i+j; } + __declspec(property(get=GetR)) T r[]; +}; + +// CHECK-LABEL: main +int main(int argc, char **argv) { + S *s; + TS *ts; + + // CHECK: [[X:%.+]] = call noundef i32 @"?GetX@S@@QEAAHXZ"(ptr {{[^,]*}} %{{.+}}) + // CHECK-NEXT: call noundef ptr @"??2S@@SAPEAX_KH@Z"(i64 noundef 1, i32 noundef [[X]]) + new (s->x) S; + + // CHECK: [[Y:%.+]] = call noundef i32 @"?GetY@S@@QEAAHHH@Z"(ptr {{[^,]*}} %{{.+}}, i32 noundef 1, i32 noundef 2) + // CHECK-NEXT: call noundef ptr @"??2S@@SAPEAX_KH@Z"(i64 noundef 1, i32 noundef [[Y]]) + new ((s->y)[1][2]) S; + + // CHECK: [[T:%.+]] = call noundef double @"?GetT@?$TS@N@@QEAANXZ"(ptr {{[^,]*}} %{{.+}}) + // CHECK-NEXT: [[TI:%.+]] = fptosi double [[T]] to i32 + // CHECK-NEXT: call noundef ptr @"??2S@@SAPEAX_KH@Z"(i64 noundef 1, i32 noundef [[TI]]) + new (ts->t) S; + + // CHECK: [[R:%.+]] = call noundef double @"?GetR@?$TS@N@@QEAANNN@Z"(ptr {{[^,]*}} %{{.+}}, double {{[^,]*}}, double {{[^,]*}}) + // CHECK-NEXT: [[RI:%.+]] = fptosi double [[R]] to i32 + // CHECK-NEXT: call noundef ptr @"??2S@@SAPEAX_KH@Z"(i64 noundef 1, i32 noundef [[RI]]) + new ((ts->r)[1][2]) S; +} + +#endif erichkeane wrote: need newlines at the end of the tests https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
@@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) erichkeane wrote: This likely needs a good collection of tests to show what it changes for NON-property related 'new' operators. https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
llvmbot wrote: @llvm/pr-subscribers-clang Author: None (Sirraide) Changes It seems we were forgetting to call `checkArgsForPlaceholders` on the placement arguments of new-expressions in Sema. I don't think that was intended—at least doing so doesn't seem to break anything—so this pr adds that. This also fixes #65053 --- Full diff: https://github.com/llvm/llvm-project/pull/75883.diff 5 Files Affected: - (modified) clang/include/clang/Sema/Sema.h (+4) - (modified) clang/lib/Sema/SemaExpr.cpp (+4-8) - (modified) clang/lib/Sema/SemaExprCXX.cpp (+3) - (added) clang/test/CodeGenCXX/ms-property-new.cpp (+52) - (added) clang/test/SemaCXX/ms-property-new.cpp (+44) ``diff diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index a4f8fc1845b1ce..90be0a648f0124 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2108,6 +2108,10 @@ class Sema final { bool CheckFunctionReturnType(QualType T, SourceLocation Loc); + /// Check an argument list for placeholders that we won't try to + /// handle later. + bool CheckArgsForPlaceholders(MultiExprArg args); + /// Build a function type. /// /// This routine checks the function type according to C++ rules and diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index c7185d56cc9973..2442c1e104055b 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5058,8 +5058,6 @@ static QualType getDependentArraySubscriptType(Expr *LHS, Expr *RHS, return Result->isDependentType() ? Result : Ctx.DependentTy; } -static bool checkArgsForPlaceholders(Sema , MultiExprArg args); - ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, SourceLocation lbLoc, MultiExprArg ArgExprs, @@ -5163,7 +5161,7 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, return ExprError(); ArgExprs[0] = result.get(); } else { -if (checkArgsForPlaceholders(*this, ArgExprs)) +if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); } @@ -6912,15 +6910,13 @@ static bool isPlaceholderToRemoveAsArg(QualType type) { llvm_unreachable("bad builtin type kind"); } -/// Check an argument list for placeholders that we won't try to -/// handle later. -static bool checkArgsForPlaceholders(Sema , MultiExprArg args) { +bool Sema::CheckArgsForPlaceholders(MultiExprArg args) { // Apply this processing to all the arguments at once instead of // dying at the first failure. bool hasInvalid = false; for (size_t i = 0, e = args.size(); i != e; i++) { if (isPlaceholderToRemoveAsArg(args[i]->getType())) { - ExprResult result = S.CheckPlaceholderExpr(args[i]); + ExprResult result = CheckPlaceholderExpr(args[i]); if (result.isInvalid()) hasInvalid = true; else args[i] = result.get(); } @@ -7194,7 +7190,7 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc, if (Result.isInvalid()) return ExprError(); Fn = Result.get(); - if (checkArgsForPlaceholders(*this, ArgExprs)) + if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); if (getLangOpts().CPlusPlus) { diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 081b568762ae22..49f0921ce324fc 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) +return ExprError(); + AllocationFunctionScope Scope = UseGlobal ? AFS_Global : AFS_Both; if (!AllocType->isDependentType() && !Expr::hasAnyTypeDependentArguments(PlacementArgs) && diff --git a/clang/test/CodeGenCXX/ms-property-new.cpp b/clang/test/CodeGenCXX/ms-property-new.cpp new file mode 100644 index 00..e21ec3d6702f62 --- /dev/null +++ b/clang/test/CodeGenCXX/ms-property-new.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +struct S { + int GetX() { return 42; } + __declspec(property(get=GetX)) int x; + + int GetY(int i, int j) { return i+j; } + __declspec(property(get=GetY)) int y[]; + + void* operator new(__SIZE_TYPE__, int); +}; + +template +struct TS { + T GetT() { return T(); } + __declspec(property(get=GetT)) T t; + + T GetR(T i, T j) { return i+j; } + __declspec(property(get=GetR)) T r[]; +}; + +// CHECK-LABEL: main +int main(int argc, char **argv) { + S *s; + TS *ts; +
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
Sirraide wrote: CC @AaronBallman https://github.com/llvm/llvm-project/pull/75883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)
https://github.com/Sirraide created https://github.com/llvm/llvm-project/pull/75883 It seems we were forgetting to call `checkArgsForPlaceholders` on the placement arguments of new-expressions in Sema. I don't think that was intended—at least doing so doesn't seem to break anything—so this pr adds that. This also fixes #65053 >From f51b382fd33d4cf3f75bdaa172bb8697a6cadc9e Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 19 Dec 2023 02:48:25 +0100 Subject: [PATCH] [Clang] Support MSPropertyRefExpr as placement arg to new-expression --- clang/include/clang/Sema/Sema.h | 4 ++ clang/lib/Sema/SemaExpr.cpp | 12 ++ clang/lib/Sema/SemaExprCXX.cpp| 3 ++ clang/test/CodeGenCXX/ms-property-new.cpp | 52 +++ clang/test/SemaCXX/ms-property-new.cpp| 44 +++ 5 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 clang/test/CodeGenCXX/ms-property-new.cpp create mode 100644 clang/test/SemaCXX/ms-property-new.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index a4f8fc1845b1ce..90be0a648f0124 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2108,6 +2108,10 @@ class Sema final { bool CheckFunctionReturnType(QualType T, SourceLocation Loc); + /// Check an argument list for placeholders that we won't try to + /// handle later. + bool CheckArgsForPlaceholders(MultiExprArg args); + /// Build a function type. /// /// This routine checks the function type according to C++ rules and diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index c7185d56cc9973..2442c1e104055b 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5058,8 +5058,6 @@ static QualType getDependentArraySubscriptType(Expr *LHS, Expr *RHS, return Result->isDependentType() ? Result : Ctx.DependentTy; } -static bool checkArgsForPlaceholders(Sema , MultiExprArg args); - ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, SourceLocation lbLoc, MultiExprArg ArgExprs, @@ -5163,7 +5161,7 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, return ExprError(); ArgExprs[0] = result.get(); } else { -if (checkArgsForPlaceholders(*this, ArgExprs)) +if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); } @@ -6912,15 +6910,13 @@ static bool isPlaceholderToRemoveAsArg(QualType type) { llvm_unreachable("bad builtin type kind"); } -/// Check an argument list for placeholders that we won't try to -/// handle later. -static bool checkArgsForPlaceholders(Sema , MultiExprArg args) { +bool Sema::CheckArgsForPlaceholders(MultiExprArg args) { // Apply this processing to all the arguments at once instead of // dying at the first failure. bool hasInvalid = false; for (size_t i = 0, e = args.size(); i != e; i++) { if (isPlaceholderToRemoveAsArg(args[i]->getType())) { - ExprResult result = S.CheckPlaceholderExpr(args[i]); + ExprResult result = CheckPlaceholderExpr(args[i]); if (result.isInvalid()) hasInvalid = true; else args[i] = result.get(); } @@ -7194,7 +7190,7 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc, if (Result.isInvalid()) return ExprError(); Fn = Result.get(); - if (checkArgsForPlaceholders(*this, ArgExprs)) + if (CheckArgsForPlaceholders(ArgExprs)) return ExprError(); if (getLangOpts().CPlusPlus) { diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 081b568762ae22..49f0921ce324fc 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, bool PassAlignment = getLangOpts().AlignedAllocation && Alignment > NewAlignment; + if (CheckArgsForPlaceholders(PlacementArgs)) +return ExprError(); + AllocationFunctionScope Scope = UseGlobal ? AFS_Global : AFS_Both; if (!AllocType->isDependentType() && !Expr::hasAnyTypeDependentArguments(PlacementArgs) && diff --git a/clang/test/CodeGenCXX/ms-property-new.cpp b/clang/test/CodeGenCXX/ms-property-new.cpp new file mode 100644 index 00..e21ec3d6702f62 --- /dev/null +++ b/clang/test/CodeGenCXX/ms-property-new.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +struct S { + int GetX() { return 42; } + __declspec(property(get=GetX)) int x; + + int GetY(int i, int j) { return i+j; } +