[clang] [Driver] Restrict Ofast deprecation help message to Clang (PR #101682)
https://github.com/AaronBallman approved this pull request. Thank you! Changes LGTM; please give @MaskRay a day before landing though, in case he's got concerns. Once this lands, we should cherry-pick to 19.x https://github.com/llvm/llvm-project/pull/101682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Restrict Ofast deprecation help message to Clang (PR #101682)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/101682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Restrict Ofast deprecation help message to Clang (PR #101682)
@@ -932,8 +932,9 @@ def O_flag : Flag<["-"], "O">, Visibility<[ClangOption, CC1Option, FC1Option]>, Alias, AliasArgs<["1"]>; def Ofast : Joined<["-"], "Ofast">, Group, Visibility<[ClangOption, CC1Option, FlangOption]>, - HelpText<"Deprecated; use '-O3 -ffast-math' for the same behavior," - " or '-O3' to enable only conforming optimizations">; + HelpTextForVariants<[ClangOption,CC1Option], AaronBallman wrote: ```suggestion HelpTextForVariants<[ClangOption, CC1Option], ``` https://github.com/llvm/llvm-project/pull/101682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C11] Claim conformance to WG14 N1396 (PR #101214)
AaronBallman wrote: 07aaab8345f1728815575639821ce581590d8479 has the changes for updating the status for overall Annex F conformance https://github.com/llvm/llvm-project/pull/101214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 07aaab8 - Update C status page rationale for Annex F conformance
Author: Aaron Ballman Date: 2024-08-02T10:03:30-04:00 New Revision: 07aaab8345f1728815575639821ce581590d8479 URL: https://github.com/llvm/llvm-project/commit/07aaab8345f1728815575639821ce581590d8479 DIFF: https://github.com/llvm/llvm-project/commit/07aaab8345f1728815575639821ce581590d8479.diff LOG: Update C status page rationale for Annex F conformance This is setting expectations that Clang will likely never conform to Annex F on 32-bit x86 with SSE2 disabled because of the way x87 works. Added: Modified: clang/www/c_status.html Removed: diff --git a/clang/www/c_status.html b/clang/www/c_status.html index 0a80039a10578..a5d04506b642b 100644 --- a/clang/www/c_status.html +++ b/clang/www/c_status.html @@ -306,7 +306,10 @@ C99 implementation status glibc, https://git.musl-libc.org/cgit/musl/tree/include/stdc-predef.h;> musl will define the macro regardless of compiler support unless the compiler defines __GCC_IEC_559, which Clang does not - currently define. + currently define. + Additionally, Clang intentionally will not conform to Annex F on + 32-bit x86 without SSE2 due to the behavior of floating-point + operations in x87. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] prevent assertion failure by avoiding required literal type checking in C context (PR #101426)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/101426 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] prevent assertion failure by avoiding required literal type checking in C context (PR #101426)
https://github.com/AaronBallman approved this pull request. LGTM! This seems like a pretty reasonable candidate to backport to 19.x as well: https://www.llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches https://github.com/llvm/llvm-project/pull/101426 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Ofast deprecation clarifications (PR #101005)
AaronBallman wrote: > We could add similar clarifications to the release notes, which were > initially included in an earlier version of this patch. So same question how > we would need to approach that. In terms of updating the release notes, once the cherry-pick happens, there's an automated bot message asking if you want to add a release note for the changes. I think the best way forward would be to follow those instructions and ask the release managers to help apply the changes. https://github.com/llvm/llvm-project/pull/101005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Ofast deprecation clarifications (PR #101005)
https://github.com/AaronBallman milestoned https://github.com/llvm/llvm-project/pull/101005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Ofast deprecation clarifications (PR #101005)
AaronBallman wrote: We have instructions at https://www.llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches but the basic gist of it is to add a special comment and add the PR to the correct milestone. As in: /cherry-pick 48d4d4b641702bf6db03a1bac73b7e13dea28349 https://github.com/llvm/llvm-project/pull/101005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C11] Claim conformance to WG14 N1396 (PR #101214)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/101214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Ofast deprecation clarifications (PR #101005)
https://github.com/AaronBallman approved this pull request. LGTM, let's go ahead and merge this and get it backported to 19.x, thank you! https://github.com/llvm/llvm-project/pull/101005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [BPF] introduce `__attribute__((bpf_fastcall))` (PR #101228)
AaronBallman wrote: > Is there some reason this is an attribute, and not a calling convention, at > the IR level? And why would it not be named `__attribute__((fastcall))` when targeting BPF? (e.g., do we need a new calling convention at all?) https://github.com/llvm/llvm-project/pull/101228 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C11] Claim conformance to WG14 N1396 (PR #101214)
AaronBallman wrote: > I mentioned this offline, but worth repeating here: I feel _something_ needs > to be marked as partial/no for the x87 case. Aaron suggested Annex F support > be that value rather than this paper, which I guess satisfies that. Agreed; the way I'd like to proceed is to land this (once folks are happy with it) and set the status here to conforming, then update the entry for "IEC 60559 support" (under C99, which is already marked as `Partial`) to mention 32-bit x86 as another reason why we only partially conform. Does that sound reasonable? https://github.com/llvm/llvm-project/pull/101214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C] Disable use of NRVO (PR #101038)
AaronBallman wrote: > Given that, I think that #100868 and #100902 should probably be handled > independently. I've reopened https://github.com/llvm/llvm-project/issues/100868 and removed the `duplicate` label for it. https://github.com/llvm/llvm-project/pull/101038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C] Disable use of NRVO (PR #101038)
AaronBallman wrote: > > Should we try to convince at least the GCC folks that they're getting this > > wrong too? > > It does feel like a wider conversation is necessary, yeah. The fact that > everybody is doing this even on x86_64 where it's pretty incontrovertibly > forbidden seems significant. CC @pinskia for awareness https://github.com/llvm/llvm-project/pull/101038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AIX] Turn on `#pragma mc_func` check by default (PR #101336)
https://github.com/AaronBallman approved this pull request. LGTM! Please make sure 1) this gets backported to the 19.x branch (https://www.llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches), 2) the 19.x release branch gets a release note for the pragma and command line options. https://github.com/llvm/llvm-project/pull/101336 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Remove __is_nullptr (PR #99038)
@@ -447,9 +447,9 @@ Decl *Parser::ParseLinkage(ParsingDeclSpec ,
DeclaratorContext Context) {
///
/// HLSL: Parse export function declaration.
///
-/// export-function-declaration:
+/// export-function-declaration:
/// 'export' function-declaration
-///
+///
AaronBallman wrote:
Spurious whitespace changes
https://github.com/llvm/llvm-project/pull/99038
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Remove __is_nullptr (PR #99038)
@@ -117,7 +121,7 @@ Improvements to Clang's diagnostics - Some template related diagnostics have been improved. .. code-block:: c++ - + AaronBallman wrote: Spurious whitespace change. https://github.com/llvm/llvm-project/pull/99038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Remove __is_nullptr (PR #99038)
https://github.com/AaronBallman approved this pull request. LGTM (modulo whitespace nits), but please make sure the 19.x branch gets a release note officially deprecating the trait and saying it will be removed in Clang 20. https://github.com/llvm/llvm-project/pull/99038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Remove __is_nullptr (PR #99038)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/99038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C11] Claim conformance to WG14 N1396 (PR #101214)
@@ -501,7 +501,13 @@ C11 implementation status Wide function returns (alternate proposal) https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1396.htm;>N1396 - Unknown + +Yes* +Clang conforms to this paper on all targets except 32-bit x86 without +SSE2. However, Clang does not claim conformance or intend to conform to +Annex F on that target, so we vacuously conform. AaronBallman wrote: Good call on reformulation as `not X and not Y`, I'll take care of that. I'll also try to reword the "vacuously conform" to be more clear. The intent is: because the changes in the paper only apply to Annex F, and Annex F support is optional, and we don't claim to conform to Annex F on any target (and we will never claim to conform to Annex F for that particular target), we conform to the paper by doing nothing. https://github.com/llvm/llvm-project/pull/101214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Improve diagnostics with incompatible VLA types (PR #101261)
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void func(int n) {
+int grp[n][n];
+int (*ptr)[n];
+
+for (int i = 0; i < n; i++)
+ptr = [i]; // expected-error {{incompatible assignment of pointers
of variable-length array type 'int (*)[n]'; consider using a typedef to use the
same variable-length array type for both operands}}
+}
AaronBallman wrote:
You should add a newline to the end of the test file. We're missing some
additional test coverage, but we can handle that once we know more about what
direction we want to take the fix.
https://github.com/llvm/llvm-project/pull/101261
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Improve diagnostics with incompatible VLA types (PR #101261)
@@ -16644,7 +16644,32 @@ bool Sema::DiagnoseAssignmentResult(AssignConvertType
ConvTy,
if (Action == AA_Passing_CFAudited) {
DiagKind = diag::err_arc_typecheck_convert_incompatible_pointer;
} else if (getLangOpts().CPlusPlus) {
- DiagKind = diag::err_typecheck_convert_incompatible_pointer;
+ DiagKind = [this, , ] {
+const VariableArrayType *SrcTypeVLA = nullptr, *DstTypeVLA = nullptr;
+if (const PointerType *P = SrcType->getAs())
+ SrcTypeVLA = Context.getAsVariableArrayType(P->getPointeeType());
+if (const PointerType *P = DstType->getAs())
+ DstTypeVLA = Context.getAsVariableArrayType(P->getPointeeType());
+
+if (SrcTypeVLA == nullptr || DstTypeVLA == nullptr)
+ return diag::err_typecheck_convert_incompatible_pointer;
+
+DeclRefExpr *SrcSizeExpr = nullptr, *DstSizeExpr = nullptr;
+if (ImplicitCastExpr *I =
+dyn_cast(SrcTypeVLA->getSizeExpr()))
+ SrcSizeExpr = dyn_cast(I->getSubExpr());
+if (ImplicitCastExpr *I =
+dyn_cast(DstTypeVLA->getSizeExpr()))
+ DstSizeExpr = dyn_cast(I->getSubExpr());
AaronBallman wrote:
For this, I would do:
```
auto GetSizeDeclRef = [](const VariableArrayType *VAT) -> const DeclRefExpr * {
if (const Expr *Size = VAT->getSizeExpr())
return dyn_cast(Size->IgnoreParenImpCasts())
return nullptr;
};
DeclRefExpr *SrcSizeExpr = GetSizeDeclRef(SrcTypeVLA),
*DestSizeExpr = GetSizeDeclRef(DestTypeVLA);
```
This will ignore more than just implicit cast expressions, but other types of
implicit AST nodes (things like parentheses expressions, etc), so it will catch
more situations.
https://github.com/llvm/llvm-project/pull/101261
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Improve diagnostics with incompatible VLA types (PR #101261)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/101261 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Improve diagnostics with incompatible VLA types (PR #101261)
https://github.com/AaronBallman commented:
Thank you for working on this!
I think there may be a way we can implement this by updating the handling of
`%diff` in `FormatDiagnostic()` in `Diagnostic.cpp`. That syntax only accepts
two `QualType` objects and is used when we're going to report a diagnostic
about a type mismatch between the two. So any such diagnostic could plausibly
be handed two VLA types and run into the same sort of confusing output for the
user.
I can think of two ways to do this (other ways may exist).
1) We could add new syntax for `%diff` to let the diagnostic author pick a
custom string when the two types would print identical output. e.g.,
`%diff{assigning $ to a variable of type $|assigning with different
types|assigning with different VLA types $}0,1`
2) We could be clever and generate text when formatting the diagnostic. e.g.,
notice that the output of the types would be identical and append `; VLA types
differ despite using the same array size expression` or something along those
lines.
I worry that #2 will wind up with awkward diagnostic wording that seems magical
to diagnostic authors because it won't be immediately clear where the wording
comes from. I worry that #1 doesn't scale well and forces everyone to think
about VLAs when it may not be clear to them how to work such a diagnostic. So
my idea may be a terrible one, and if so, feel free to tell me. ;-)
https://github.com/llvm/llvm-project/pull/101261
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Improve diagnostics with incompatible VLA types (PR #101261)
@@ -16644,7 +16644,32 @@ bool Sema::DiagnoseAssignmentResult(AssignConvertType
ConvTy,
if (Action == AA_Passing_CFAudited) {
DiagKind = diag::err_arc_typecheck_convert_incompatible_pointer;
} else if (getLangOpts().CPlusPlus) {
- DiagKind = diag::err_typecheck_convert_incompatible_pointer;
+ DiagKind = [this, , ] {
+const VariableArrayType *SrcTypeVLA = nullptr, *DstTypeVLA = nullptr;
+if (const PointerType *P = SrcType->getAs())
+ SrcTypeVLA = Context.getAsVariableArrayType(P->getPointeeType());
+if (const PointerType *P = DstType->getAs())
+ DstTypeVLA = Context.getAsVariableArrayType(P->getPointeeType());
+
+if (SrcTypeVLA == nullptr || DstTypeVLA == nullptr)
+ return diag::err_typecheck_convert_incompatible_pointer;
+
+DeclRefExpr *SrcSizeExpr = nullptr, *DstSizeExpr = nullptr;
+if (ImplicitCastExpr *I =
+dyn_cast(SrcTypeVLA->getSizeExpr()))
+ SrcSizeExpr = dyn_cast(I->getSubExpr());
+if (ImplicitCastExpr *I =
+dyn_cast(DstTypeVLA->getSizeExpr()))
+ DstSizeExpr = dyn_cast(I->getSubExpr());
+
+if (SrcSizeExpr == nullptr || DstSizeExpr == nullptr)
+ return diag::err_typecheck_convert_incompatible_pointer;
+
+return SrcSizeExpr->getDecl()->getName() ==
+ DstSizeExpr->getDecl()->getName()
AaronBallman wrote:
I think it would be good to have a comment here explaining that comparing
declaration *names* is intentional because you can have code with the same size
expression names but differing declarations. e.g.,
https://godbolt.org/z/sTqoKrs1j
But, the downside to this is that it's only worried about `DeclRefExpr`s, but
there can still be other situations like `MemberExpr`
(https://godbolt.org/z/ET8fMG6aT) or `CallExpr`
(https://godbolt.org/z/hxc8G1494), etc.
That's why I'm hoping we can find *some* way to handle this from the
diagnostics engine. If the two types print to the same string, it would be nice
to find some way to alert the user "these types are spelled the same way but
are potentially unique types".
https://github.com/llvm/llvm-project/pull/101261
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Update argument checking tablegen code to use a 'full' name (PR #99993)
AaronBallman wrote: > This causes significant compile-time regressions, especially for unoptimized > builds: > https://llvm-compile-time-tracker.com/compare.php?from=39b6900852e7a1187bd742ba5c1387ca1be58e2c=2acf77f987331c05520c5bfd849326909ffce983=instructions:u > > You probably need to separately match the syntax and the name without > constructing a temporary `std::string`. Thank you for identifying this, @nikic! > That should elimate the need for any new std::strings. @AaronBallman does > this sound reasonable? That seems like a good approach to me https://github.com/llvm/llvm-project/pull/3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement CWG2627 Bit-fields and narrowing conversions (PR #78112)
https://github.com/AaronBallman approved this pull request. Generally LGTM, just a few minor things. Thank you for this! https://github.com/llvm/llvm-project/pull/78112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement CWG2627 Bit-fields and narrowing conversions (PR #78112)
@@ -0,0 +1,44 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s +// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -std=c++11 %s +// RUN: %clang_cc1 -triple x86_64-windows-pc -fsyntax-only -verify -std=c++11 %s +// RUN: %clang_cc1 -triple i386-linux-pc -fsyntax-only -verify -std=c++11 %s +// RUN: %clang_cc1 -triple i386-windows-pc -fsyntax-only -verify -std=c++11 %s + +#if __BITINT_MAXWIDTH__ >= 35 AaronBallman wrote: No need for this predicate, it's required to be at least the same width as `long long`. https://github.com/llvm/llvm-project/pull/78112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement CWG2627 Bit-fields and narrowing conversions (PR #78112)
@@ -0,0 +1,44 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s +// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -std=c++11 %s +// RUN: %clang_cc1 -triple x86_64-windows-pc -fsyntax-only -verify -std=c++11 %s +// RUN: %clang_cc1 -triple i386-linux-pc -fsyntax-only -verify -std=c++11 %s +// RUN: %clang_cc1 -triple i386-windows-pc -fsyntax-only -verify -std=c++11 %s AaronBallman wrote: Could we get away with two RUN lines instead of four by doing `-triple x86_64` and `-triple i386` and leaving the rest of the triple to the test runner? https://github.com/llvm/llvm-project/pull/78112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement CWG2627 Bit-fields and narrowing conversions (PR #78112)
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-windows-pc -fsyntax-only -verify -std=c++11
%s
+// RUN: %clang_cc1 -triple i386-linux-pc -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple i386-windows-pc -fsyntax-only -verify -std=c++11 %s
+
+#if __BITINT_MAXWIDTH__ >= 35
+struct {
+ _BitInt(35) i : 33;
+} x;
+struct {
+ _BitInt(35) i : 34;
+} y;
+_BitInt(33) xx{ x.i };
+_BitInt(33) yy{ y.i };
+// expected-error@-1 {{non-constant-expression cannot be narrowed from type
'_BitInt(35)' to '_BitInt(33)' in initializer list}}
+// FIXME-expected-note@-2 {{insert an explicit cast to silence this issue}}
+#endif
+
+#if __BITINT_MAXWIDTH__ >= 3
AaronBallman wrote:
Same here
https://github.com/llvm/llvm-project/pull/78112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement CWG2627 Bit-fields and narrowing conversions (PR #78112)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/78112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C11] Claim conformance to WG14 N1396 (PR #101214)
AaronBallman wrote: Thank you! I updated the test and status information; let me know if you'd like to see further adjustments. https://github.com/llvm/llvm-project/pull/101214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Introduce ffp-model=aggressive (PR #100453)
@@ -102,6 +102,16 @@ Deprecated Compiler Flags Modified Compiler Flags --- +- The ``-ffp-model`` option has been updated to enable a more limited set of + optimizations when the ``fast`` argument is used and to accept a new argument, + ``aggressive``. The behavior of ``-ffp-model=aggressive`` is equivalent + to the previous behavior of ``-ffp-model=fast``. The updated + ``-ffp-model=fast`` behavior no longer assumes finite math only, uses + the ``promoted`` algorithm for complex division when possible rather than the + less basic (limited range) algorithm, and sets + ``-ffp-contract=fast-honor-pragmas`` rather than ``-ffp-contract=fast``. AaronBallman wrote: FWIW, @andykaylor and I spoke about my concerns offline and I'm less concerned now. The silent behavior change is actually protecting the user from more aggressive optimizations they may not have intended to apply to their code anyway, so while it is a silent behavioral change, it's likely to be the better behavior for the most users, even if it's worse for a few users. https://github.com/llvm/llvm-project/pull/100453 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] thread-safety: Support the new capability-based names for all related attributes. (PR #99919)
AaronBallman wrote: > > Thank you for these changes! Can you also add a release note to > > `clang/docs/ReleaseNotes.rst` so users know about the new attribute > > spellings? > > They are already sort of documented as such – but will update the > ReleaseNotes.rst. > > Does anyone know why the new names were documented but not actually > implemented? I think this was my oversight -- I can't find any discussion where we intentionally made a decision to do this. CC @delesley who also worked on this at the same time. https://github.com/llvm/llvm-project/pull/99919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C] Disable use of NRVO (PR #101038)
AaronBallman wrote: I'm a bit less clear on what to do now. It sounds like nobody is opposed to a language dialect flag (default off) so users can retain the existing behavior. But it also sounds like perhaps the correct approach to this is to modify codegen and not the NRVO markings when building the AST. (Then again, wouldn't we want to drop that bit from the AST in C anyway because we wouldn't want AST consumers to presume that NRVO is valid, such as one-off AST matchers for use in clang-tidy, etc?) https://github.com/llvm/llvm-project/pull/101038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C] Disable use of NRVO (PR #101038)
AaronBallman wrote: > We shouldn't merge this PR unless we need an immediate fix, which seems > unlikely given how longstanding this behavior is. Agreed, it's definitely not in shape for committing. As for needing an immediate fix, what surprised me was that two different people ran into this same issue within a few days of each other (https://github.com/llvm/llvm-project/issues/100902 and https://github.com/llvm/llvm-project/issues/100868) https://github.com/llvm/llvm-project/pull/101038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C] Disable use of NRVO (PR #101038)
AaronBallman wrote: > Instead of forcing this off for C compilations, how would you feel about > instead changing the default for -felide-constructors to be off for C, so > people can still re-enable the optimization to avoid performance / stack > usage regressions? (Weird flag name for C, I know; maybe we should also think > about adding an alias such as -felide-return-copies.) I think that's a reasonable language dialect for users, and agree that the name should change. Thanks for the suggestion! https://github.com/llvm/llvm-project/pull/101038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C] Disable use of NRVO (PR #101038)
AaronBallman wrote: I'll get on top of fixing the other failing test cases, but I'd appreciate confirmation that we agree with the direction this patch is heading first. https://github.com/llvm/llvm-project/pull/101038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C] Disable use of NRVO (PR #101038)
AaronBallman wrote: One question I had is: this is an ABI breaking change for us, isn't it? Do we need/want ABI tags in that case? Should I call it out as a potentially breaking change? https://github.com/llvm/llvm-project/pull/101038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Don't invalidate the super region when a std object ctor runs (PR #100405)
@@ -923,12 +923,31 @@ SVal AnyCXXConstructorCall::getCXXThisVal() const {
return UnknownVal();
}
+static bool isWithinStdNamespace(const Decl *D) {
AaronBallman wrote:
Huh, that behavior would be surprising to me because nested subclasses are not
in the `std` namespace. (Not saying it's wrong for the static analyzer, but I
would not expect that behavior when doing code reviews, for example.)
https://github.com/llvm/llvm-project/pull/100405
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix hasName matcher assertion with inline namespaces (PR #100975)
https://github.com/AaronBallman approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/100975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Don't invalidate the super region when a std object ctor runs (PR #100405)
@@ -923,12 +923,31 @@ SVal AnyCXXConstructorCall::getCXXThisVal() const {
return UnknownVal();
}
+static bool isWithinStdNamespace(const Decl *D) {
AaronBallman wrote:
We've already got one of these at home:
https://github.com/llvm/llvm-project/blob/ee57ce57d8094026e2795182758bc57027a72293/clang/include/clang/AST/DeclBase.h#L488
https://github.com/llvm/llvm-project/pull/100405
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement CWG2627 Bit-fields and narrowing conversions (PR #78112)
https://github.com/AaronBallman commented: I'm pretty sure the C implementation of `constexpr` leans on this code to determine if the value is exactly representable without a change in value. But it looks like Clang currently doesn't get this quite right: https://godbolt.org/z/W3a8cPjnM Can you add some tests for C as well? (We can use the tests to document existing behavior, but there may be bugs here to be addressed. CC @Fznamznon) https://github.com/llvm/llvm-project/pull/78112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Ofast deprecation clarifications (PR #101005)
@@ -429,8 +429,12 @@ Code Generation Options :option:`-Ofast` Enables all the optimizations from :option:`-O3` along with other aggressive optimizations that may violate strict compliance with -language standards. This is deprecated in favor of :option:`-O3` -in combination with :option:`-ffast-math`. +language standards. This is deprecated in Clang-19 and a warning is emitted AaronBallman wrote: ```suggestion language standards. This is deprecated in Clang 19 and a warning is emitted ``` https://github.com/llvm/llvm-project/pull/101005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Ofast deprecation clarifications (PR #101005)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/101005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Ofast deprecation clarifications (PR #101005)
https://github.com/AaronBallman commented: FWIW, I'm happy enough with the proposed wording (I want to give others a chance to weigh in before approving though). https://github.com/llvm/llvm-project/pull/101005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Introduce ffp-model=aggressive (PR #100453)
@@ -102,6 +102,16 @@ Deprecated Compiler Flags Modified Compiler Flags --- +- The ``-ffp-model`` option has been updated to enable a more limited set of + optimizations when the ``fast`` argument is used and to accept a new argument, + ``aggressive``. The behavior of ``-ffp-model=aggressive`` is equivalent + to the previous behavior of ``-ffp-model=fast``. The updated + ``-ffp-model=fast`` behavior no longer assumes finite math only, uses + the ``promoted`` algorithm for complex division when possible rather than the + less basic (limited range) algorithm, and sets + ``-ffp-contract=fast-honor-pragmas`` rather than ``-ffp-contract=fast``. AaronBallman wrote: I'm not super comfortable with this approach because it's a silent behavior change between compiler releases; would it be reasonable to deprecate "fast" entirely (leaving its behavior alone) and introduce two new options: `aggressive` and `user-hostile` (or something perhaps less loaded, lol)? https://github.com/llvm/llvm-project/pull/100453 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-cl] [Sema] Support MSVC non-const lvalue to user-defined temporary reference (PR #99833)
AaronBallman wrote: > > This should be controllable via -fms-extensions/-fno-ms-extensions; having > > its own dialect flag is a bit novel but I'm not strongly opposed. CC > > @MaskRay @jansvoboda11 for driver/options opinions > > For this feedback I intentionally didn't do this because this ms extension > isn't one intended to be always enabled. -fms-extensions enables extensions > that are always valid to use with msvc such as the `__try` and `__finally` > keywords for SEH. That's kind of the crux of this -- `-fms-extensions` enables Microsoft extensions, and this is a Microsoft extension. Our documentation is pretty quiet about what it means to "enable Microsoft extensions": https://clang.llvm.org/docs/UsersManual.html#microsoft-extensions > MSVC allows controlling this option independently here, > [/Zc:ReferenceBinding](https://learn.microsoft.com/en-us/cpp/build/reference/zc-referencebinding-enforce-reference-binding-rules?view=msvc-170). > MSVC `/permissive-` disables this reference binding extension, however > `__try` and friends are still available as they are vital for operating > within a Win32 environment. Newer C++ modes like C++20 also implicitly > disable this reference binding extension in MSVC as they move towards proper > C++ conformance by default. > > This is a one off extension similar to -fms-volatile or -fms-define-stdc. I > would like to keep it a separate option that can be individually controlled > instead of grouping it with -fms-extensions. Oh, I think it should still be individually controlled via its own flag. The scenario I am thinking about is when the user passed `-fms-extensions` and no other individual flags. I think that mode should enable all of the Microsoft extensions. Then users can opt out of whatever extensions they don't want to enable. But I think it's confusing if `-fms-extensions` still requires the user to explicitly opt in to other Microsoft-specific extensions. CC @zmodem @rnk for additional opinions. https://github.com/llvm/llvm-project/pull/99833 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AIX] Detect `#pragma mc_func` (PR #99888)
AaronBallman wrote: Oh, this is even weirder than I realized -- that example gives a declaration for `add_logical()` (line 6), so that's why there's no "unknown function" diagnostic from the compiler, just a link error. That's a plausible reason for this to be handled in a special way, but now I'm wondering why this option isn't on by default for AIX. I don't think anyone is going to know to turn this option on, whereas if it's on by default, users have to make a decision as to how to proceed, which seems like the better default. https://github.com/llvm/llvm-project/pull/99888 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix confusing diagnositcs related to explicit this parameters (PR #100351)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/100351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] [codegen] Reduce the state in TBAA. NFC for static compilation. (PR #98138)
AaronBallman wrote: > Hi @AaronBallman, could we move that forward? I've added the Clang codegen code owners, as they really should be involved in this sort of change. https://github.com/llvm/llvm-project/pull/98138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ASTMatchers] Extend hasName matcher when matching templates (PR #100349)
@@ -3076,6 +3076,21 @@ inline internal::BindableMatcher sizeOfExpr(
/// \code
/// namespace a { namespace b { class X; } }
/// \endcode
+///
+/// Qualified names in templated classes can be matched explicitly or implicity
+/// by specifying the template type or using `<*>` to match any template.
AaronBallman wrote:
We should probably be clear that `<*>` is the *only* syntax we support. e.g.,
we don't let you search based on arity, as in `hasName("Foo::Bar")`.
Might be worth an example as well, definitely worth some test coverage.
https://github.com/llvm/llvm-project/pull/100349
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ASTMatchers] Extend hasName matcher when matching templates (PR #100349)
https://github.com/AaronBallman commented: The changes should also come with a release note in clang/docs/ReleaseNotes.rst. https://github.com/llvm/llvm-project/pull/100349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ASTMatchers] Extend hasName matcher when matching templates (PR #100349)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/100349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Finish deleting the le32/le64 targets (PR #98497)
AaronBallman wrote: > Note that Halide has removed its dependency on the le32/le64 targets as of > [halide/Halide#8344](https://github.com/halide/Halide/pull/8344). Thank you for letting us know! Does anyone know of any reasons we should not revert the revert (so le32/le64 is removed in Clang 20)? https://github.com/llvm/llvm-project/pull/98497 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix hasName matcher assertion with inline namespaces (PR #100975)
AaronBallman wrote: > > I can't get the assertion to reproduce, but I can see the behavioral change > > with the first matcher: https://godbolt.org/z/TcsYPoGEh > > So this should probably come with a release note for the fix? > > The assertions are only shown on debug build, but it comes from > [here](https://github.com/llvm/llvm-project/blob/9e3f5ae48fa75d5db8b132162d96004191618956/clang/lib/ASTMatchers/ASTMatchersInternal.cpp#L678) That should still trigger with an assertions build though, right? > Which section in the release notes do changes to AST matchers belong in Somewhere around here: https://github.com/llvm/llvm-project/blob/77655f42d58e85875c4b4e28a73208b64a653c2a/clang/docs/ReleaseNotes.rst?plain=1#L247 https://github.com/llvm/llvm-project/pull/100975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix hasName matcher assertion with inline namespaces (PR #100975)
https://github.com/AaronBallman commented: I can't get the assertion to reproduce, but I can see the behavioral change with the first matcher: https://godbolt.org/z/TcsYPoGEh So this should probably come with a release note for the fix? https://github.com/llvm/llvm-project/pull/100975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Disallow applying `onwership_returns` to functions that return non-pointers (PR #99564)
@@ -231,6 +231,10 @@ Crash and bug fixes Improvements +- Improved the handling of the `ownership_returns` attribute. Now, Clang reports an + error if the attribute is attached to a function that returns a non-pointer value. AaronBallman wrote: ```suggestion - Improved the handling of the ``ownership_returns`` attribute. Now, Clang reports an error if the attribute is attached to a function that returns a non-pointer value. ``` https://github.com/llvm/llvm-project/pull/99564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Disallow applying `onwership_returns` to functions that return non-pointers (PR #99564)
https://github.com/AaronBallman commented: Just a few minor nits, otherwise LGTM https://github.com/llvm/llvm-project/pull/99564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Disallow applying `onwership_returns` to functions that return non-pointers (PR #99564)
@@ -1481,6 +1481,15 @@ static void handleOwnershipAttr(Sema , Decl *D, const
ParsedAttr ) {
break;
}
+ // Allow only pointers to be return type for functions with ownership_returns
+ // attribute. This matches with current OwnershipAttr::Takes semantics
+ if (K == OwnershipAttr::Returns) {
+if (!getFunctionOrMethodResultType(D)->isPointerType()) {
+ S.Diag(AL.getLoc(), diag::err_ownership_takes_return_type) << AL;
+ return;
+}
+ }
AaronBallman wrote:
```suggestion
if (K == OwnershipAttr::Returns &&
!getFunctionOrMethodResultType(D)->isPointerType()) {
S.Diag(AL.getLoc(), diag::err_ownership_takes_return_type) << AL;
return;
}
```
(combining the two predicates, may need reformatting.)
https://github.com/llvm/llvm-project/pull/99564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Disallow applying `onwership_returns` to functions that return non-pointers (PR #99564)
@@ -3330,6 +3330,8 @@ def err_attribute_invalid_implicit_this_argument : Error<
"%0 attribute is invalid for the implicit this argument">;
def err_ownership_type : Error<
"%0 attribute only applies to %select{pointer|integer}1 arguments">;
+def err_ownership_takes_return_type : Error<
+ "'ownership_returns' attribute only applies to functions that return
pointers">;
AaronBallman wrote:
```suggestion
"'ownership_returns' attribute only applies to functions that return a
pointer">;
```
https://github.com/llvm/llvm-project/pull/99564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Disallow applying `onwership_returns` to functions that return non-pointers (PR #99564)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/99564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Handle tm mangling on Solaris in PPMacroExpansion.cpp (PR #100724)
AaronBallman wrote: > > > Whatever the case, we need at least a short-term solution now: the > > > Solaris buildbots have been broken for two days now. > > > > > > Agreed that we need something in the short term, I'm just trying to find > > the cleanest "something" under the assumption that this hack will live for > > a long time. That's why I was hoping we could shove this into CMake > > somewhere (it's less in everyone's face than the current approach). If we > > can't do that, then I can live with this approach. > > I just don't see how this can be done. And TBH rather than pile hack upon > hack, I'd rather spend some time on duplicating what GCC already does in > Clang. This will require tons and tons of guidance, though. I would recommend talking to @efriedma-quic and @rjmccall about potential approaches https://github.com/llvm/llvm-project/pull/100724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Handle tm mangling on Solaris in PPMacroExpansion.cpp (PR #100724)
https://github.com/AaronBallman approved this pull request. LGTM; if we find a better approach post-commit, this is easy enough to remove. Thanks! https://github.com/llvm/llvm-project/pull/100724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Handle tm mangling on Solaris in PPMacroExpansion.cpp (PR #100724)
AaronBallman wrote: > Whatever the case, we need at least a short-term solution now: the Solaris > buildbots have been broken for two days now. Agreed that we need something in the short term, I'm just trying to find the cleanest "something" under the assumption that this hack will live for a long time. That's why I was hoping we could shove this into CMake somewhere (it's less in everyone's face than the current approach). If we can't do that, then I can live with this approach. Note: precommit CI failures look unrelated. https://github.com/llvm/llvm-project/pull/100724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Remove `IDNS_Ordinary` flag in `IndirectFieldDecl::IdentifierNamespace` (PR #100525)
AaronBallman wrote: > > I don’t believe the standard supports anonymous structs in C++ > > Do we need tests about anonymous union? C++ standard supports it. Yes, we should add tests for C++ as well; good call @cor3ntin and @Sirraide! https://github.com/llvm/llvm-project/pull/100525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Remove `IDNS_Ordinary` flag in `IndirectFieldDecl::IdentifierNamespace` (PR #100525)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/100525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Remove `IDNS_Ordinary` flag in `IndirectFieldDecl::IdentifierNamespace` (PR #100525)
https://github.com/AaronBallman approved this pull request. The C standard does not make this particularly clear, but the changes are correct. Per C23 6.2.1p4 Each member of the structure has file scope (because why not) but per 6.2.3p1, each member is in a distinct name space and the member is disambiguated by the type of the expression used to access the member via the `.` or `->` operator. That last bit is why the member cannot be an ordinary identifier within the structure itself. So LGTM, thank you! https://github.com/llvm/llvm-project/pull/100525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Handle tm mangling on Solaris in PPMacroExpansion.cpp (PR #100724)
https://github.com/AaronBallman commented: Thank you for helping out with this odd edge case! I don't suppose there's a way we can shove this hack into CMake (basically, like a kind of linker script hack)? If so, can we apply it broadly enough so that any use of `time_put` in other TUs will also benefit? (If there are better alternatives than this approach, I'd love to hear about them, this is a pretty gnarly way to go about working around the issue.) CC @petrhosek @MaskRay @efriedma-quic https://github.com/llvm/llvm-project/pull/100724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][clang] Avoid unnecessary assignment (PR #100728)
https://github.com/AaronBallman approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/100728 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-cl] [Sema] Support MSVC non-const lvalue to user-defined temporary reference (PR #99833)
https://github.com/AaronBallman requested changes to this pull request. Thank you for this! I think there's some work left to be done though. 1) Please add an extension warning (and tests for it) so users know where the extension is being used even if they opt into it explicitly. This is important for `-pedantic` mode. 2) This should be controllable via `-fms-extensions`/`-fno-ms-extensions`; having its own dialect flag is a bit novel but I'm not strongly opposed. CC @MaskRay @jansvoboda11 for driver/options opinions 3) Please document the extension in `clang/docs/LanguageExtensions.rst` https://github.com/llvm/llvm-project/pull/99833 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wimplicit-fallthrough to -Wextra (PR #97926)
AaronBallman wrote: > I'm pretty sure I enabled the warning correctly. @AaronBallman are we good to > land? Not quite -- it looks like this change is causing libc++ test failures that were caught by precommit CI: ``` # | /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-jltnz-1/llvm-project/github-pull-requests/build-runtimes/include/c++/v1/regex:3925:5: note: insert '[[clang::fallthrough]];' to silence this warning # | 3925 | case 'x': # | | ^ # | | [[clang::fallthrough]]; # | /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-jltnz-1/llvm-project/github-pull-requests/build-runtimes/include/c++/v1/regex:3925:5: note: insert 'break;' to avoid fall-through # | 3925 | case 'x': # | | ^ # | | break; # | 1 error generated. ``` CC @ldionne https://github.com/llvm/llvm-project/pull/97926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wimplicit-fallthrough to -Wextra (PR #97926)
AaronBallman wrote:
> I'm not seeing any compile time impact from dropping the `DefaultIgnore`s on
> warn_unannotated_fallthrough/warn_unannotated_fallthrough_per_function. So
> either it's free or I'm still doing something wrong.
> (http://llvm-compile-time-tracker.com/compare.php?from=6d12b3f67df429b6e1953d9f55867d7e2469=4117b087cc607686c472f54f82f1222688b5f2bf=instructions:u)
Yeah, that logic should suffice:
```
bool FallThroughDiagFull =
!Diags.isIgnored(diag::warn_unannotated_fallthrough, D->getBeginLoc());
bool FallThroughDiagPerFunction = !Diags.isIgnored(
diag::warn_unannotated_fallthrough_per_function, D->getBeginLoc());
if (FallThroughDiagFull || FallThroughDiagPerFunction ||
fscope->HasFallthroughStmt) {
DiagnoseSwitchLabelsFallthrough(S, AC, !FallThroughDiagFull);
}
```
so unless something explicitly ignores the fallthrough warning in the setup, it
seems like a valid test.
https://github.com/llvm/llvm-project/pull/97926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AIX] Detect `#pragma mc_func` (PR #99888)
AaronBallman wrote: > I have not reviewed other pragmas that we do not support, but I think this > one is a bit special in that it defines functions (code) that can be > executed, and we are aware of use cases where detecting the error at link > time is insufficient. Ooh, maybe I misunderstood the situation. I didn't think this was something detected only at link time, I was thinking this added a symbol at compile time and so the user would get an "unknown identifier 'whatever'" diagnostic anyway if they tried to call the function. Is that not the case? https://github.com/llvm/llvm-project/pull/99888 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ASTMatchers] Extend hasName matcher when matching templates (PR #100349)
AaronBallman wrote: > > > @AaronBallman That's a good point, I didn't account for how defaulted > > > template arguments aren't printed. Would using `<*>` ~or a more rustic > > > approach of `<_>`~ be a good alternative? > > > > > > I was thinking about that, but I keep coming around to wondering whether it > > might make sense to support a regex there instead of a custom syntax. That > > seems like it gives users a bit more power without requiring them to write > > additional matchers, but perhaps that's a bad idea for performance > > reasons... > > We already have a `matchesName` matcher that would be able to handle any case > you want to throw at it, but there is definitely some performance concerns > with it. This is here to simply the basic case where you just want to match > any template which should be helpful for most cases. Okay, that's a good point! Hmm, then yeah, I think `*` makes sense as a glob; `_` is a loaded term because of https://wg21.link/P2169 https://github.com/llvm/llvm-project/pull/100349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libcxx] Reapply "[Clang] Implement resolution for CWG1835 (#92957, #98547)" (PR #100425)
AaronBallman wrote: There is discussion on the Core reflectors about this DR (https://lists.isocpp.org/core/2024/07/16028.php), so given the amount of difficulty we've had with this DR so far, I would recommend we hold off on landing any changes until Core has finished deliberation. https://github.com/llvm/llvm-project/pull/100425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Refine unused-member-function diagnostic message for constructors (PR #84515)
@@ -402,7 +402,7 @@ def warn_unused_function : Warning<"unused function %0">,
InGroup, DefaultIgnore;
def warn_unused_template : Warning<"unused %select{function|variable}0
template %1">,
InGroup, DefaultIgnore;
-def warn_unused_member_function : Warning<"unused member function %0">,
+def warn_unused_member_function : Warning<"unused %select{constructor|member
function %1}0">,
AaronBallman wrote:
I would recommend using `%sub{select_special_member_kind}1` instead of manually
spelling out constructor; see `note_member_synthesized_at` for an example of it
being used.
https://github.com/llvm/llvm-project/pull/84515
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Refine unused-member-function diagnostic message for constructors (PR #84515)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/84515 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Refine unused-member-function diagnostic message for constructors (PR #84515)
@@ -76,10 +76,33 @@ struct S {
struct SVS : public VS {
void vm() { }
};
+
+ struct CS {
AaronBallman wrote:
When switching the diagnostic approach, be sure to add test coverage for unused
copy/move constructors, copy/move assignment, and destructors (you can get an
unused destructor if you only use `new` to allocate the class rather than stack
allocate it).
https://github.com/llvm/llvm-project/pull/84515
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Refine unused-member-function diagnostic message for constructors (PR #84515)
https://github.com/AaronBallman commented: Thank you for the diagnostic improvement! https://github.com/llvm/llvm-project/pull/84515 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `std::put_time` (PR #99075)
AaronBallman wrote: Blech, thank you for trying! I guess that unless there's some way to hide this ugliness in the cmake scripts or someone else has better ideas to try, the `asm` aliasing (on Solaris only) may be our best path forward. https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libcxx] [Clang] Add __common_type builtin (PR #99473)
AaronBallman wrote: Should we prefix this with `__builtin_` as mentioned in https://github.com/llvm/llvm-project/issues/98310#issuecomment-2221105713 ? https://github.com/llvm/llvm-project/pull/99473 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] fix assertion failure in invalid delete operator declaration check (PR #99308)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/99308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] fix assertion failure in invalid delete operator declaration check (PR #99308)
AaronBallman wrote: > @AaronBallman If no further feedback is needed, could you please proceed with > the merge? Thanks Can do! Btw, you should feel free to obtain commit privileges yourself if you'd like: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access https://github.com/llvm/llvm-project/pull/99308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] fix assertion failure in invalid delete operator declaration check (PR #99308)
https://github.com/AaronBallman approved this pull request. LGTM! Thank you for the fix! https://github.com/llvm/llvm-project/pull/99308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `std::put_time` (PR #99075)
AaronBallman wrote: > > We have a mixture of both `std::tm` and `::tm` in here, try switching to > > using `::tm` and see if that helps. > > Unfortunately not: I always get the undefined reference to the `std::tm const > *` version, no matter what I tried. Drat! If you dump the symbols from the STL shared library on the system, is there one for `time_put` at all? If so, what is the mangled symbol it exports? (As a perhaps terrible idea, could we use the `alias` attribute on a redeclaration to try to force to link against the correct mangling for Solaris only?) https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ASTMatchers] Extend hasName matcher when matching templates (PR #100349)
AaronBallman wrote:
> Allow users to match all record instantiations by using <> as a wildcard.
I think this will change the behavior of existing matchers, consider:
```
template
struct S {
Ty Val;
void Call();
};
int main() {
S<> s;
s.Call(); // Currently matches only this
S another;
another.Call(); // Will now start to match this as well?
}
```
https://godbolt.org/z/Gr439c9v4
WDYT?
https://github.com/llvm/llvm-project/pull/100349
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ASTMatchers] Extend hasName matcher when matching templates (PR #100349)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/100349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `std::put_time` (PR #99075)
AaronBallman wrote: Aha! https://bugs.llvm.org/show_bug.cgi?id=33767 We have a mixture of both `std::tm` and `::tm` in here, try switching to using `::tm` and see if that helps. https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `std::put_time` (PR #99075)
AaronBallman wrote: > Confirmed: reverting the change locally restores the builds, although I don't > yet see why. The fact that there's a missing symbol suggests the STL on the machine is not conforming... the symbol that's missing (`_ZNKSt8time_putIcSt19ostreambuf_iteratorIcSt11char_traitsIcEEE3putES3_RSt8ios_basecPKSt2tmPKcSB_`) demangles to: `std::time_put > >::put(std::ostreambuf_iterator >, std::ios_base&, char, std::tm const*, char const*, char const*) const` which has been around since C++98. https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement `__is_virtual_base_of()` intrinsic (PR #100393)
@@ -6027,6 +6027,33 @@ static bool EvaluateBinaryTypeTrait(Sema ,
TypeTrait BTT, const TypeSourceI
return cast(rhsRecord->getDecl())
->isDerivedFrom(cast(lhsRecord->getDecl()));
}
+ case BTT_IsVirtualBaseOf: {
+const RecordType *BaseRecord = LhsT->getAs();
+const RecordType *DerivedRecord = RhsT->getAs();
+
+if (!BaseRecord || !DerivedRecord) {
+ DiagnoseVLAInCXXTypeTrait(Self, Lhs, tok::kw___is_virtual_base_of);
+ DiagnoseVLAInCXXTypeTrait(Self, Rhs, tok::kw___is_virtual_base_of);
+ return false;
+}
+
+if (BaseRecord->isUnionType() || DerivedRecord->isUnionType())
+ return false;
+
+if (!BaseRecord->isStructureOrClassType() ||
+!DerivedRecord->isStructureOrClassType())
+ return false;
+
+if (Self.RequireCompleteType(Rhs->getTypeLoc().getBeginLoc(), RhsT,
AaronBallman wrote:
I hadn't realized that `is_base_of` already has the same requirement, so I
don't insist on a change here. But that said, this presumes users don't have
typos they'd like to catch. e.g.,
```
struct Bee { };
struct Be;
struct Derived : virtual Bee {};
static_assert(!__is_virtual_base_of(Be, Derived)); // Oops, I can't spell very
well!
```
I think it would be *much* kinder to tell the user that `Be` is incomplete than
let them be surprised.
https://github.com/llvm/llvm-project/pull/100393
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement `__is_virtual_base_of()` intrinsic (PR #100393)
@@ -6027,6 +6027,33 @@ static bool EvaluateBinaryTypeTrait(Sema ,
TypeTrait BTT, const TypeSourceI
return cast(rhsRecord->getDecl())
->isDerivedFrom(cast(lhsRecord->getDecl()));
}
+ case BTT_IsVirtualBaseOf: {
+const RecordType *BaseRecord = LhsT->getAs();
+const RecordType *DerivedRecord = RhsT->getAs();
+
+if (!BaseRecord || !DerivedRecord) {
+ DiagnoseVLAInCXXTypeTrait(Self, Lhs, tok::kw___is_virtual_base_of);
+ DiagnoseVLAInCXXTypeTrait(Self, Rhs, tok::kw___is_virtual_base_of);
+ return false;
+}
+
+if (BaseRecord->isUnionType() || DerivedRecord->isUnionType())
+ return false;
+
+if (!BaseRecord->isStructureOrClassType() ||
+!DerivedRecord->isStructureOrClassType())
+ return false;
+
+if (Self.RequireCompleteType(Rhs->getTypeLoc().getBeginLoc(), RhsT,
+ diag::err_incomplete_type))
+ return false;
+
+if (Self.Context.hasSameUnqualifiedType(LhsT, RhsT))
+ return false;
AaronBallman wrote:
I think this actually does *more* work instead of less; most code will not pass
in the same type as both arguments, right? So that means this check won't catch
anything useful most of the time, but the below check still will catch this
case (I believe).
https://github.com/llvm/llvm-project/pull/100393
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement `__is_virtual_base_of()` intrinsic (PR #100393)
@@ -6027,6 +6027,33 @@ static bool EvaluateBinaryTypeTrait(Sema ,
TypeTrait BTT, const TypeSourceI
return cast(rhsRecord->getDecl())
->isDerivedFrom(cast(lhsRecord->getDecl()));
}
+ case BTT_IsVirtualBaseOf: {
+const RecordType *BaseRecord = LhsT->getAs();
+const RecordType *DerivedRecord = RhsT->getAs();
+
+if (!BaseRecord || !DerivedRecord) {
+ DiagnoseVLAInCXXTypeTrait(Self, Lhs, tok::kw___is_virtual_base_of);
+ DiagnoseVLAInCXXTypeTrait(Self, Rhs, tok::kw___is_virtual_base_of);
+ return false;
+}
+
+if (BaseRecord->isUnionType() || DerivedRecord->isUnionType())
+ return false;
+
+if (!BaseRecord->isStructureOrClassType() ||
+!DerivedRecord->isStructureOrClassType())
+ return false;
AaronBallman wrote:
ObjC types don't inherit from `RecordType`, so I don't think this check is
needed.
https://github.com/llvm/llvm-project/pull/100393
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement `__is_virtual_base_of()` intrinsic (PR #100393)
@@ -6027,6 +6027,33 @@ static bool EvaluateBinaryTypeTrait(Sema ,
TypeTrait BTT, const TypeSourceI
return cast(rhsRecord->getDecl())
->isDerivedFrom(cast(lhsRecord->getDecl()));
}
+ case BTT_IsVirtualBaseOf: {
+const RecordType *BaseRecord = LhsT->getAs();
+const RecordType *DerivedRecord = RhsT->getAs();
+
+if (!BaseRecord || !DerivedRecord) {
+ DiagnoseVLAInCXXTypeTrait(Self, Lhs, tok::kw___is_virtual_base_of);
+ DiagnoseVLAInCXXTypeTrait(Self, Rhs, tok::kw___is_virtual_base_of);
+ return false;
+}
+
+if (BaseRecord->isUnionType() || DerivedRecord->isUnionType())
+ return false;
+
+if (!BaseRecord->isStructureOrClassType() ||
+!DerivedRecord->isStructureOrClassType())
+ return false;
+
+if (Self.RequireCompleteType(Rhs->getTypeLoc().getBeginLoc(), RhsT,
AaronBallman wrote:
What does it mean for there to be a complete derived class type but an
incomplete base class type? Shouldn't this require a complete type in both
cases to give reasonable diagnostic behavior?
https://github.com/llvm/llvm-project/pull/100393
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement `__is_virtual_base_of()` intrinsic (PR #100393)
@@ -6027,6 +6027,33 @@ static bool EvaluateBinaryTypeTrait(Sema ,
TypeTrait BTT, const TypeSourceI
return cast(rhsRecord->getDecl())
->isDerivedFrom(cast(lhsRecord->getDecl()));
}
+ case BTT_IsVirtualBaseOf: {
+const RecordType *BaseRecord = LhsT->getAs();
+const RecordType *DerivedRecord = RhsT->getAs();
+
+if (!BaseRecord || !DerivedRecord) {
+ DiagnoseVLAInCXXTypeTrait(Self, Lhs, tok::kw___is_virtual_base_of);
+ DiagnoseVLAInCXXTypeTrait(Self, Rhs, tok::kw___is_virtual_base_of);
AaronBallman wrote:
I guess I'm surprised by this -- why are VLAs special?
https://github.com/llvm/llvm-project/pull/100393
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Define `ATOMIC_FLAG_INIT` correctly for C++. (PR #97534)
AaronBallman wrote: > > @AaronBallman Just FYI you can also add the Milestone to the PR directly > > and use the /cherry-pick command right here. This is a bit simpler. > > That's good to know! I was going off the documentation at: > https://www.llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches > because it says "issue" specifically. I'll post a patch to update those > docs, thanks! https://github.com/llvm/llvm-project/pull/100401 https://github.com/llvm/llvm-project/pull/97534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Define `ATOMIC_FLAG_INIT` correctly for C++. (PR #97534)
AaronBallman wrote: > @AaronBallman Just FYI you can also add the Milestone to the PR directly and > use the /cherry-pick command right here. This is a bit simpler. That's good to know! I was going off the documentation at: https://www.llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches because it says "issue" specifically. I'll post a patch to update those docs, thanks! https://github.com/llvm/llvm-project/pull/97534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] thread-safety: Support the new capability-based names for all related attributes. (PR #99919)
https://github.com/AaronBallman commented: Thank you for these changes! Can you also add a release note to `clang/docs/ReleaseNotes.rst` so users know about the new attribute spellings? https://github.com/llvm/llvm-project/pull/99919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
