[clang] [clang][Sema] Don't issue -Wcast-function-type-mismatch for enums with a matching underlying type (PR #87793)
https://github.com/AaronBallman approved this pull request. LGTM! Do you need someone to land this on your behalf? https://github.com/llvm/llvm-project/pull/87793 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Don't issue -Wcast-function-type-mismatch for enums with a matching underlying type (PR #87793)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/87793 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Make sure all C++ DR tests are running with `-pedantic-errors` (PR #94203)
https://github.com/AaronBallman approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/94203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Make sure all C++ DR tests are running with `-pedantic-errors` (PR #94203)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/94203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Make sure all C++ DR tests are running with `-pedantic-errors` (PR #94203)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/94203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Teach clang-repl how to load PCHs. (PR #94166)
@@ -282,6 +288,8 @@ namespace {
}
void HandleTranslationUnit(ASTContext ) override {
+ IRGenFinished = true;
AaronBallman wrote:
shouldn't this be set at the end of `HandleTranslationUnit` rather than the
beginning?
https://github.com/llvm/llvm-project/pull/94166
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Teach clang-repl how to load PCHs. (PR #94166)
@@ -138,6 +138,8 @@ namespace {
assert(!M && "Replacing existing Module?");
M.reset(new llvm::Module(ExpandModuleName(ModuleName, CodeGenOpts), C));
+ IRGenFinished = false;
AaronBallman wrote:
Why is IR gen *finished* when we're just starting the module?
https://github.com/llvm/llvm-project/pull/94166
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] remove goma support from clang (PR #93942)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/93942 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [C23] Fix typeof_unqual for qualified array types (PR #92767)
https://github.com/AaronBallman approved this pull request. LGTM thank you! https://github.com/llvm/llvm-project/pull/92767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 68a6481 - [C11] Claim conformance to N1464
Author: Aaron Ballman
Date: 2024-05-31T13:52:13-04:00
New Revision: 68a64812d7bac28412d43a0b4b19bae6db101c48
URL:
https://github.com/llvm/llvm-project/commit/68a64812d7bac28412d43a0b4b19bae6db101c48
DIFF:
https://github.com/llvm/llvm-project/commit/68a64812d7bac28412d43a0b4b19bae6db101c48.diff
LOG: [C11] Claim conformance to N1464
That's on the CMPLX macros which Clang supports via __builtin_complex.
Added:
clang/test/C/C11/n1464.c
Modified:
clang/www/c_status.html
Removed:
diff --git a/clang/test/C/C11/n1464.c b/clang/test/C/C11/n1464.c
new file mode 100644
index 0..eca00ed949a70
--- /dev/null
+++ b/clang/test/C/C11/n1464.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -verify %s
+// expected-no-diagnostics
+
+/* WG14 N1464: ???
+ * Creation of complex value
+ */
+
+// The paper is about the CMPLX macros, which Clang supports via the
+// __builtin_complex builtin function. Test the basic functionality.
+_Static_assert(__builtin_complex(5.0, 2.0) == 5.0 + 1.0j * 2.0, "");
+_Static_assert(__builtin_complex(5.0f, 2.0f) == 5.0f + 1.0j * 2.0f, "");
+_Static_assert(__builtin_complex(5.0L, 2.0L) == 5.0L + 1.0j * 2.0L, "");
+
+// Test the edge case involving NaN or infinity.
+#define INF(type) (type)__builtin_inf()
+#define NAN(type) (type)__builtin_nan("")
+_Static_assert(__builtin_complex(5.0f, INF(float)) != 5.0f + 1.0j *
INF(float), "");
+_Static_assert(__builtin_complex(5.0, INF(double)) != 5.0 + 1.0j *
INF(double), "");
+_Static_assert(__builtin_complex(5.0L, INF(long double)) != 5.0L + 1.0j *
INF(long double), "");
+_Static_assert(__builtin_complex(5.0f, NAN(float)) != 5.0f + 1.0j *
NAN(float), "");
+_Static_assert(__builtin_complex(5.0, NAN(double)) != 5.0 + 1.0j *
NAN(double), "");
+_Static_assert(__builtin_complex(5.0L, NAN(long double)) != 5.0L + 1.0j *
NAN(long double), "");
diff --git a/clang/www/c_status.html b/clang/www/c_status.html
index dfc1afefda245..a94c606c3244a 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -561,7 +561,7 @@ C11 implementation status
Creation of complex value
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1464.htm;>N1464
- Unknown
+ Clang 12
Recommendations for extended identifier characters for C and C++
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Allow raw string literals in C as an extension (PR #88265)
AaronBallman wrote: > In that case I think it might just make sense to ignore the flag in C++11 and > later then and allow it before C++11. I think that makes the most sense. https://github.com/llvm/llvm-project/pull/88265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Allow raw string literals in C as an extension (PR #88265)
AaronBallman wrote: C has Unicode string literals as well: https://godbolt.org/z/chdjYrK9v and so if we're allowing raw string literals, it makes sense to also allow raw unicode string literals IMO. I don't think we need to rename the flag though. https://github.com/llvm/llvm-project/pull/88265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)
https://github.com/AaronBallman approved this pull request. There are a few minor suggestions from @delcypher that should be addressed, but overall the changes LGTM (assuming there are no precommit CI surprises after fixing the merge conflicts). https://github.com/llvm/llvm-project/pull/93231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [BoundsSafety] Add `-fexperimental-bounds-safety` CC1 and language option and use it to tweak `counted_by`'s semantics (PR #92623)
https://github.com/AaronBallman commented: I'd like to understand why we need a flag for this rather than changing the behavior of `-fbounds-safety`. (I'd like to avoid a proliferation of flags unless this flag is going to gate more changes in the near future.) https://github.com/llvm/llvm-project/pull/92623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [BoundsSafety] Add `-fexperimental-bounds-safety` CC1 and language option and use it to tweak `counted_by`'s semantics (PR #92623)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/92623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Split up `SemaDeclAttr.cpp` (PR #93966)
@@ -32,5 +36,87 @@ inline bool isFunctionOrMethodOrBlockForAttrSubject(const
Decl *D) {
return isFuncOrMethodForAttrSubject(D) || llvm::isa(D);
}
+/// Return true if the given decl has a declarator that should have
+/// been processed by Sema::GetTypeForDeclarator.
+inline bool hasDeclarator(const Decl *D) {
+ // In some sense, TypedefDecl really *ought* to be a DeclaratorDecl.
+ return isa(D) || isa(D) ||
+ isa(D) || isa(D);
AaronBallman wrote:
```suggestion
return isa(D);
```
https://github.com/llvm/llvm-project/pull/93966
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Split up `SemaDeclAttr.cpp` (PR #93966)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Split up `SemaDeclAttr.cpp` (PR #93966)
@@ -32,5 +36,87 @@ inline bool isFunctionOrMethodOrBlockForAttrSubject(const
Decl *D) {
return isFuncOrMethodForAttrSubject(D) || llvm::isa(D);
}
+/// Return true if the given decl has a declarator that should have
+/// been processed by Sema::GetTypeForDeclarator.
+inline bool hasDeclarator(const Decl *D) {
+ // In some sense, TypedefDecl really *ought* to be a DeclaratorDecl.
+ return isa(D) || isa(D) ||
+ isa(D) || isa(D);
+}
+
+/// hasFunctionProto - Return true if the given decl has a argument
+/// information. This decl should have already passed
+/// isFuncOrMethodForAttrSubject or isFunctionOrMethodOrBlockForAttrSubject.
+inline bool hasFunctionProto(const Decl *D) {
+ if (const FunctionType *FnTy = D->getFunctionType())
+return isa(FnTy);
+ return isa(D) || isa(D);
AaronBallman wrote:
```suggestion
return isa(D);
```
https://github.com/llvm/llvm-project/pull/93966
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Split up `SemaDeclAttr.cpp` (PR #93966)
https://github.com/AaronBallman commented: Precommit CI failures seem relevant. https://github.com/llvm/llvm-project/pull/93966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Interp] Member Pointers (PR #91303)
Timm =?utf-8?q?Bäder?= ,
Timm =?utf-8?q?Bäder?= ,
Timm =?utf-8?q?Bäder?=
Message-ID:
In-Reply-To:
@@ -0,0 +1,112 @@
+//===- MemberPointer.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_INTERP_MEMBER_POINTER_H
+#define LLVM_CLANG_AST_INTERP_MEMBER_POINTER_H
+
+#include "Pointer.h"
+#include
+
+namespace clang {
+class ASTContext;
+namespace interp {
+
+class Context;
+class FunctionPointer;
+
+class MemberPointer final {
+private:
+ Pointer Base;
+ const Decl *Dcl = nullptr;
+ int32_t PtrOffset = 0;
+
+ MemberPointer(Pointer Base, const Decl *Dcl, int32_t PtrOffset)
+ : Base(Base), Dcl(Dcl), PtrOffset(PtrOffset) {}
+
+public:
+ MemberPointer() = default;
+ MemberPointer(Pointer Base, const Decl *Dcl) : Base(Base), Dcl(Dcl) {}
+ MemberPointer(uint32_t Address, const Descriptor *D) {
+// This should be impossible to hit, at least I've been unable
+// to write a test for it.
+ }
+
+ MemberPointer(const Decl *D) : Dcl(D) {
+assert(isa(D) || isa(D) ||
+ isa(D));
AaronBallman wrote:
Ah yeah, macros and commas bite yet again. I don't insist on a change, though I
do find the combined form to be a bit easier to read.
https://github.com/llvm/llvm-project/pull/91303
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Interp] Member Pointers (PR #91303)
Timm =?utf-8?q?B=C3=A4der?= , Timm =?utf-8?q?B=C3=A4der?= , Timm =?utf-8?q?B=C3=A4der?= Message-ID: In-Reply-To: https://github.com/AaronBallman approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/91303 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Interp] Member Pointers (PR #91303)
Timm =?utf-8?q?Bäder?= , Timm =?utf-8?q?Bäder?= , Timm =?utf-8?q?Bäder?= Message-ID: In-Reply-To: https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/91303 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [C23] Fix typeof_unqual for qualified array types (PR #92767)
@@ -92,3 +92,43 @@ extern __attribute__((address_space(0))) int
type_attr_test_2; // expec
void invalid_param_fn(__attribute__((address_space(1))) int i); //
expected-error {{parameter may not be qualified with an address space}}
typeof(invalid_param_fn) invalid_param_1;
typeof_unqual(invalid_param_fn) invalid_param_2;
+
+// Ensure restrict is stripped
+extern int *restrict p1;
+extern int *p2;
+extern typeof(p1) p1;
+extern typeof_unqual(p1) p2;
+
+// Ensure array qualifications are removed
+extern const int aci[2];
+extern const int acii[2][2];
+extern int ai[2];
+extern int aii[2][2];
+extern typeof(aci) aci;
+extern typeof_unqual(aci) ai;
+extern typeof(acii) acii;
+extern typeof_unqual(acii) aii;
+
+extern int *restrict arpi[2];
+extern int *restrict arpii[2][2];
+extern int *api[2];
+extern int *apii[2][2];
+extern typeof(arpi) arpi;
+extern typeof_unqual(arpi) api;
+extern typeof(arpii) arpii;
+extern typeof_unqual(arpii) apii;
+
+extern int _Atomic aAi[2];
+extern int _Atomic aAii[2][2];
+extern typeof(aAi) aAi;
+extern typeof_unqual(aAi) ai;
+extern typeof(aAii) aAii;
+extern typeof_unqual(aAii) aii;
AaronBallman wrote:
Ah, so yeah, I think I was confused above. Both `typeof(_Atomic(int)[2][2])`
and `typeof(_Atomit int[2][2])` would return a two-dimensional array of
atomic-qualified ints, and `typeof_unqual` is defined to return the non-atomic,
unqualified version of the type that would come out of `typeof`, so that would
mean both forms come out of `typeof_unqual` the same.
https://github.com/llvm/llvm-project/pull/92767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/www/get_started.html] Use newer `cmake` syntax (PR #93503)
https://github.com/AaronBallman commented: In general, I think this is heading in the right direction. I did have a suggestion that may be worth considering. https://github.com/llvm/llvm-project/pull/93503 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/www/get_started.html] Use newer `cmake` syntax (PR #93503)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93503 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/www/get_started.html] Use newer `cmake` syntax (PR #93503)
@@ -74,8 +74,8 @@ On Unix-like Systems https://llvm.org/docs/CMake.html#frequently-used-cmake-variables;>frequently used cmake variables for more options. AaronBallman wrote: [Re: lines 72 to 76] I think we should update this text at the same time to add something like: ``` This example uses the https://ninja-build.org/;>Ninja build system, but see https://cmake.org/cmake/help/latest/manual/cmake-generators.7.html"CMake's documentation about generators for more options. ``` See this comment inline on https://app.graphite.dev/github/pr/llvm/llvm-project/93503?utm_source=unchanged-line-comment;>Graphite. https://github.com/llvm/llvm-project/pull/93503 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
https://github.com/AaronBallman approved this pull request. I looked through the review and I think this is generally an improvement. We may find a need to relax some `const` later, but I did not spot any obvious concerns. That said, I'd like @ChuanqiXu9 to give the final sign-off on this as modules code owner. https://github.com/llvm/llvm-project/pull/93493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -501,6 +501,9 @@ Improvements to Clang's diagnostics - Clang emits a ``-Wparentheses`` warning for expressions with consecutive comparisons like ``x < y < z``. Fixes #GH20456. +- Clang now diagnoses missing format attributes for non-template functions and + class/struct/union members. Fixes #GH70024 AaronBallman wrote: ```suggestion class/struct/union members. Fixes #GH60718 ``` https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,277 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+
+typedef unsigned short char16_t;
+typedef unsigned int char32_t;
+typedef __WCHAR_TYPE__ wchar_t;
+typedef __SIZE_TYPE__ size_t;
+typedef __builtin_va_list va_list;
+
+__attribute__((__format__(__printf__, 1, 2)))
+int printf(const char *, ...); // #printf
+
+__attribute__((__format__(__scanf__, 1, 2)))
+int scanf(const char *, ...); // #scanf
+
+__attribute__((__format__(__printf__, 1, 0)))
+int vprintf(const char *, va_list); // #vprintf
+
+__attribute__((__format__(__scanf__, 1, 0)))
+int vscanf(const char *, va_list); // #vscanf
+
+__attribute__((__format__(__printf__, 2, 0)))
+int vsprintf(char *, const char *, va_list); // #vsprintf
+
+__attribute__((__format__(__printf__, 3, 0)))
+int vsnprintf(char *ch, size_t, const char *, va_list); // #vsnprintf
+
+__attribute__((__format__(__scanf__, 1, 4)))
+void f1(char *out, const size_t len, const char *format, ... /* args */) // #f1
+{
+va_list args;
+vsnprintf(out, len, format, args); // expected-warning@#f1 {{diagnostic
behavior may be improved by adding the 'printf' format attribute to the
declaration of 'f1'}}
+ // CHECK-FIXES:
__attribute__((format(printf, 3, 4)))
+}
+
+__attribute__((__format__(__printf__, 1, 4)))
+void f2(char *out, const size_t len, const char *format, ... /* args */) // #f2
+{
+va_list args;
+vsnprintf(out, len, format, args); // expected-warning@#f2 {{diagnostic
behavior may be improved by adding the 'printf' format attribute to the
declaration of 'f2'}}
+ // CHECK-FIXES:
__attribute__((format(printf, 3, 4)))
+}
+
+void f3(char *out, va_list args) // #f3
+{
+vprintf(out, args); // expected-warning@#f3 {{diagnostic behavior may be
improved by adding the 'printf' format attribute to the declaration of 'f3'}}
+// CHECK-FIXES: __attribute__((format(printf, 1, 0)))
+vscanf(out, args); // expected-warning@#f3 {{diagnostic behavior may be
improved by adding the 'scanf' format attribute to the declaration of 'f3'}}
+ // CHECK-FIXES: __attribute__((format(scanf, 1, 0)))
AaronBallman wrote:
This seems like a situation where it makes more sense to not emit the
diagnostic or the fix-it, right? The chances of this function being a format
function in practice are pretty small because the format specifiers mean
different things. e.g., `%d` is a specifier for an `int` for `printf` but `int
*` for `scanf`.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -7131,6 +7138,114 @@ static void handleSwiftAsyncAttr(Sema , Decl *D,
const ParsedAttr ) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// This function is called only if function call is not inside template body.
+// TODO: Add call for function calls inside template body.
+// Check if parent function misses format attribute. If misses, emit warning.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ const FunctionDecl *ParentFuncDecl = getCurFunctionDecl();
+ if (!ParentFuncDecl)
+return;
+
+ // If function is a member of struct/union/class and has implicit object
+ // parameter, format attribute argument indexing starts from 2. Otherwise, it
+ // starts from 1.
+ unsigned int FormatArgumentIndexOffset =
+ checkIfMethodHasImplicitObjectParameter(FDecl) ? 2 : 1;
+ unsigned int ParentFunctionFormatArgumentIndexOffset =
+ checkIfMethodHasImplicitObjectParameter(ParentFuncDecl) ? 2 : 1;
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ const ParmVarDecl *FormatArg;
+ if (!llvm::any_of(FDecl->specific_attrs(),
+[&](const FormatAttr *Attr) {
+ const int FormatIndexOffseted =
AaronBallman wrote:
```suggestion
int OffsetFormatIndex =
```
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,277 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+
+typedef unsigned short char16_t;
+typedef unsigned int char32_t;
+typedef __WCHAR_TYPE__ wchar_t;
+typedef __SIZE_TYPE__ size_t;
+typedef __builtin_va_list va_list;
+
+__attribute__((__format__(__printf__, 1, 2)))
+int printf(const char *, ...); // #printf
+
+__attribute__((__format__(__scanf__, 1, 2)))
+int scanf(const char *, ...); // #scanf
+
+__attribute__((__format__(__printf__, 1, 0)))
+int vprintf(const char *, va_list); // #vprintf
+
+__attribute__((__format__(__scanf__, 1, 0)))
+int vscanf(const char *, va_list); // #vscanf
+
+__attribute__((__format__(__printf__, 2, 0)))
+int vsprintf(char *, const char *, va_list); // #vsprintf
+
+__attribute__((__format__(__printf__, 3, 0)))
+int vsnprintf(char *ch, size_t, const char *, va_list); // #vsnprintf
+
+__attribute__((__format__(__scanf__, 1, 4)))
+void f1(char *out, const size_t len, const char *format, ... /* args */) // #f1
+{
+va_list args;
+vsnprintf(out, len, format, args); // expected-warning@#f1 {{diagnostic
behavior may be improved by adding the 'printf' format attribute to the
declaration of 'f1'}}
+ // CHECK-FIXES:
__attribute__((format(printf, 3, 4)))
AaronBallman wrote:
This is not testing anything; that's a magic comment for the static analyzer
cc1 command. You'll need to use `-fdiagnostics-parseable-fixits` and
`FileCheck` to check fixes in general (you can search for that command line
option and model after one of the existing tests).
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,303 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,beforeCxx2b
-Wmissing-format-attribute %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2b -Wmissing-format-attribute
%s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++23 -Wmissing-format-attribute
%s
+
+typedef __SIZE_TYPE__ size_t;
+typedef __builtin_va_list va_list;
+
+namespace std
+{
+template struct basic_string_view {};
+template struct basic_string {
+const Elem *c_str() const noexcept;
+basic_string(const basic_string_view SW);
+};
+
+using string = basic_string;
+using wstring = basic_string;
+using string_view = basic_string_view;
+using wstring_view = basic_string_view;
+}
+
+__attribute__((__format__(__printf__, 1, 2)))
+int printf(const char *, ...); // #printf
+
+__attribute__((__format__(__scanf__, 1, 2)))
+int scanf(const char *, ...); // #scanf
+
+__attribute__((__format__(__printf__, 1, 0)))
+int vprintf(const char *, va_list); // #vprintf
+
+__attribute__((__format__(__scanf__, 1, 0)))
+int vscanf(const char *, va_list); // #vscanf
+
+__attribute__((__format__(__printf__, 2, 0)))
+int vsprintf(char *, const char *, va_list); // #vsprintf
+
+__attribute__((__format__(__printf__, 3, 0)))
+int vsnprintf(char *ch, size_t, const char *, va_list); // #vsnprintf
+
+void f1(const std::string , ... /* args */) // #f1
+{
+va_list args;
+vscanf(str.c_str(), args); // no warning
+vprintf(str.c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f2(const std::string , ... /* args */); // #f2
+
+void f3(std::string_view str, ... /* args */) // #f3
+{
+va_list args;
+vscanf(std::string(str).c_str(), args); // no warning
+vprintf(std::string(str).c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f4(std::string_view str, ... /* args */); // #f4
+
+void f5(const std::wstring , ... /* args */) // #f5
+{
+va_list args;
+vscanf((const char *)str.c_str(), args); // no warning
+vprintf((const char *)str.c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f6(const std::wstring , ... /* args */); // #f6
+
+void f7(std::wstring_view str, ... /* args */) // #f7
+{
+va_list args;
+vscanf((const char *) std::wstring(str).c_str(), args); // no warning
+vprintf((const char *) std::wstring(str).c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f8(std::wstring_view str, ... /* args */); // #f8
+
+void f9(const wchar_t *out, ... /* args */) // #f9
+{
+va_list args;
+vprintf(out, args); // expected-error {{no matching function for call to
'vprintf'}}
+// expected-note@#vprintf {{candidate function not
viable: no known conversion from 'const wchar_t *' to 'const char *' for 1st
argument}}
+vscanf((const char *) out, args); // expected-warning@#f9 {{diagnostic
behavior may be improved by adding the 'scanf' format attribute to the
declaration of 'f9'}}
+ // CHECK-FIXES:
__attribute__((format(scanf, 1, 2)))
+vscanf((char *) out, args); // expected-warning@#f9 {{diagnostic behavior
may be improved by adding the 'scanf' format attribute to the declaration of
'f9'}}
+// CHECK-FIXES: __attribute__((format(scanf,
1, 2)))
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f10(const wchar_t *out, ... /* args */); // #f10
+
+void f11(const char16_t *out, ... /* args */) // #f11
AaronBallman wrote:
This doesn't seem like a particularly helpful test because it's catching a type
mismatch rather than anything with format attributes.
That said, I think functions that can be either valid C or C++ should be kept
in the .c file and that file can get a C++-specific `RUN` line to test those
situations. Then this .cpp file can be focused on constructs that exist in C++
but not C. WDYT?
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] require template arg list after template kw (PR #80801)
@@ -2995,13 +2996,23 @@ bool Parser::ParseUnqualifiedId(CXXScopeSpec ,
ParsedType ObjectType,
SS, ObjectType, ObjectHadErrors,
TemplateKWLoc ? *TemplateKWLoc : SourceLocation(), Id, IdLoc,
EnteringContext, Result, TemplateSpecified);
-else if (TemplateSpecified &&
- Actions.ActOnTemplateName(
- getCurScope(), SS, *TemplateKWLoc, Result, ObjectType,
- EnteringContext, Template,
- /*AllowInjectedClassName*/ true) == TNK_Non_template)
- return true;
+if (TemplateSpecified) {
+ TemplateNameKind TNK =
+ Actions.ActOnTemplateName(getCurScope(), SS, *TemplateKWLoc, Result,
+ObjectType, EnteringContext, Template,
+/*AllowInjectedClassName*/ true);
+ if (TNK == TNK_Non_template)
+return true;
+
+ // C++ [template.names]p6
+ // A name prefixed by the keyword template shall be followed by a
template
+ // argument list or refer to a class template or an alias template.
+ if ((TNK == TNK_Function_template || TNK == TNK_Dependent_template_name
||
AaronBallman wrote:
I think it's reasonable to handle in a follow-up, thanks!
https://github.com/llvm/llvm-project/pull/80801
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] require template arg list after template kw (PR #80801)
https://github.com/AaronBallman approved this pull request. I think the changes are reasonable, but I'd like to wait a bit before landing to give @erichkeane a chance to review as templates code owner. https://github.com/llvm/llvm-project/pull/80801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] require template arg list after template kw (PR #80801)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/80801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Conditionalize use of POSIX features missing on WASI/WebAssembly (PR #92677)
AaronBallman wrote: Ah thank you for pointing that out, I had missed some updates on that thread. I think `defined(__wasi__)` would address my concerns. @MaskRay asked a good question about whether we should tie this to `LLVM_ON_UNIX` as well and I don't have a strong intuition there, but my naive thinking is that it would help in some cases but be insufficient in others (e.g., within Support/Unix/Path.inc, we'd still need to have a conditional for WASI). https://github.com/llvm/llvm-project/pull/92677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Add option to generate additional debug info for expression dereferencing pointer to pointers. (PR #81545)
AaronBallman wrote: > How do I reopen the PR after fixing the test case? AIUI, you cannot reopen a merged PR, you have to start a new PR and mention the old one to link the two together. https://github.com/llvm/llvm-project/pull/81545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)
@@ -1329,6 +1341,100 @@ bool Preprocessor::LexAfterModuleImport(Token ) {
return true;
}
+/// Lex a token following the 'module' contextual keyword.
+///
+/// [cpp.module]/p2:
+/// The pp-tokens, if any, of a pp-module shall be of the form:
+/// pp-module-name pp-module-partition[opt] pp-tokens[opt]
+///
+/// where the pp-tokens (if any) shall not begin with a ( preprocessing token
+/// and the grammar non-terminals are defined as:
+/// pp-module-name:
+/// pp-module-name-qualifierp[opt] identifier
+/// pp-module-partition:
+/// : pp-module-name-qualifier[opt] identifier
+/// pp-module-name-qualifier:
+/// identifier .
+/// pp-module-name-qualifier identifier .
+/// No identifier in the pp-module-name or pp-module-partition shall currently
+/// be defined as an object-like macro.
+///
+/// [cpp.module]/p3:
+/// Any preprocessing tokens after the module preprocessing token in the module
+/// directive are processed just as in normal text.
+bool Preprocessor::LexAfterModuleDecl(Token ) {
+ // Figure out what kind of lexer we actually have.
+ recomputeCurLexerKind();
+ LexUnexpandedToken(Result);
+
+ auto EnterTokens = [this](ArrayRef Toks, bool DisableMacroExpansion) {
+auto ToksCopy = std::make_unique(Toks.size());
+std::copy(Toks.begin(), Toks.end(), ToksCopy.get());
+EnterTokenStream(std::move(ToksCopy), Toks.size(), DisableMacroExpansion,
+ /*IsReinject=*/false);
+ };
+
+ // If we don't expect an identifier but got an identifier, it's not a part of
+ // module name.
+ if (!ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+EnterTokens(Result, /*DisableMacroExpansion=*/false);
+return false;
+ }
+
+ // The token sequence
+ //
+ // export[opt] module identifier (. identifier)*
+ //
+ // indicates a module directive. We already saw the 'module'
+ // contextual keyword, so now we're looking for the identifiers.
+ if (ModuleDeclExpectsIdentifier && Result.is(tok::identifier)) {
+auto *MI = getMacroInfo(Result.getIdentifierInfo());
+if (MI && MI->isObjectLike()) {
+ Diag(Result, diag::err_module_decl_cannot_be_macros)
+ << Result.getLocation() << ModuleDeclLexingPartitionName
+ << Result.getIdentifierInfo();
+}
+ModuleDeclExpectsIdentifier = false;
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+ }
+
+ // If we're expecting a '.', a ':' or a ';', and we got a '.', then wait
until
+ // we see the next identifier.
+ if (!ModuleDeclExpectsIdentifier && Result.isOneOf(tok::period, tok::colon))
{
+ModuleDeclExpectsIdentifier = true;
+ModuleDeclLexingPartitionName = Result.is(tok::colon);
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return true;
+ }
+
+ // [cpp.module]/p2: where the pp-tokens (if any) shall not begin with a (
+ // preprocessing token [...]
+ if (!ModuleDeclExpectsIdentifier && Result.is(tok::l_paren)) {
+ModuleDeclExpectsIdentifier = false;
+Diag(Result, diag::err_unxepected_paren_in_module_decl)
+<< ModuleDeclLexingPartitionName;
+Token Tok;
+// We already have a '('.
+unsigned NumParens = 1;
+while (true) {
+ LexUnexpandedToken(Tok);
+ if (Tok.isOneOf(tok::eod, tok::eof, tok::semi, tok::period, tok::colon))
{
+EnterTokens(Tok, /*DisableMacroExpansion=*/true);
+break;
+ }
+ if (Tok.is(tok::l_paren))
+NumParens++;
+ else if (Tok.is(tok::r_paren) && --NumParens == 0)
+break;
+}
+CurLexerCallback = CLK_LexAfterModuleDecl;
+return false;
+ }
+
+ return true;
+}
+
AaronBallman wrote:
> I am not sure I under why we would actually consume ATTRS in either case, but
> I think I need to think about it more / play with the PR.
I'd like to understand this better, too. I would have (naively and without
checking in a debugger) that we would have peeked at `ATTRS` rather than
consumed it.
> @AaronBallman Any opinion?
I think your suggestion makes sense to me; that's a good use of annotation
tokens to squirrel information from the preprocessor into the parser, and
keeping the logic consolidated as much as possible is preferred.
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] ``README.txt``: Replace the link to the old bug tracker with the new one. (PR #93878)
https://github.com/AaronBallman approved this pull request. LGTM! Do you need someone to land this on your behalf? https://github.com/llvm/llvm-project/pull/93878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] remove goma support from clang (PR #93942)
https://github.com/AaronBallman approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/93942 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Conditionalize use of POSIX features missing on WASI/WebAssembly (PR #92677)
AaronBallman wrote: Comments in the config file isn't a harmful thing, but I'm also worried about folks who never see the config file. e.g., they're working in Path.inc on something, see `HAVE_PWD_H` in the code, and just make assumptions from there. So perhaps a two-pronged approach: a comment in config.h for the cases where we don't expect a graceful fallback, and a similar comment around the guarded #include in source for the folks who won't look in config.h? https://github.com/llvm/llvm-project/pull/92677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Conditionalize use of POSIX features missing on WASI/WebAssembly (PR #92677)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/92677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Conditionalize use of POSIX features missing on WASI/WebAssembly (PR #92677)
https://github.com/AaronBallman commented: Sorry for not responding sooner, but I think this approach makes sense to me. It's basically similar to autoconf where we're checking whether a feature is supported and guarding against it. One concern I have is that someone adding new code may think they need to come up with fallback behavior in case a feature isn't available (e.g., "how do I do this if I don't have `setjmp`?) and it would be nice if we could find a way to make it clear that they are not responsible for coming up with that fallback behavior. (Mostly worried about folks doing a bunch of heavy lifting before submitting a PR only to hear during review "you didn't need to do that, please pull it out because we don't have a way to test it".) https://github.com/llvm/llvm-project/pull/92677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [C23] Fix typeof_unqual for qualified array types (PR #92767)
@@ -92,3 +92,43 @@ extern __attribute__((address_space(0))) int
type_attr_test_2; // expec
void invalid_param_fn(__attribute__((address_space(1))) int i); //
expected-error {{parameter may not be qualified with an address space}}
typeof(invalid_param_fn) invalid_param_1;
typeof_unqual(invalid_param_fn) invalid_param_2;
+
+// Ensure restrict is stripped
+extern int *restrict p1;
+extern int *p2;
+extern typeof(p1) p1;
+extern typeof_unqual(p1) p2;
+
+// Ensure array qualifications are removed
+extern const int aci[2];
+extern const int acii[2][2];
+extern int ai[2];
+extern int aii[2][2];
+extern typeof(aci) aci;
+extern typeof_unqual(aci) ai;
+extern typeof(acii) acii;
+extern typeof_unqual(acii) aii;
+
+extern int *restrict arpi[2];
+extern int *restrict arpii[2][2];
+extern int *api[2];
+extern int *apii[2][2];
+extern typeof(arpi) arpi;
+extern typeof_unqual(arpi) api;
+extern typeof(arpii) arpii;
+extern typeof_unqual(arpii) apii;
+
+extern int _Atomic aAi[2];
+extern int _Atomic aAii[2][2];
+extern typeof(aAi) aAi;
+extern typeof_unqual(aAi) ai;
+extern typeof(aAii) aAii;
+extern typeof_unqual(aAii) aii;
AaronBallman wrote:
I think this is a more complicated situation, perhaps. `_Atomic` as a type
qualifier (as with `_Atomic type`) is excluded in terms of arrays and elements
being identically qualified (see footnote 42, which says "This does not apply
to the _Atomic qualifier.") so the array is not atomic to begin with. So
`typeof_unqual` would strip the non-existent qualifiers from the array in this
case but leave them alone in the element type. However, `_Atomic` as a type
specifier (as with `_Atomic(type)`) is an atomic-qualified type and so it would
be stripped in that case (see footnote 146 which also reminds us that
`_Atomic(type-name)` is an atomic-qualified type).
So I think the behavior of this case is correct but we need another test for
`_Atomic(int)`.
https://github.com/llvm/llvm-project/pull/92767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [C23] Fix typeof_unqual for qualified array types (PR #92767)
@@ -92,3 +92,43 @@ extern __attribute__((address_space(0))) int
type_attr_test_2; // expec
void invalid_param_fn(__attribute__((address_space(1))) int i); //
expected-error {{parameter may not be qualified with an address space}}
typeof(invalid_param_fn) invalid_param_1;
typeof_unqual(invalid_param_fn) invalid_param_2;
+
+// Ensure restrict is stripped
+extern int *restrict p1;
+extern int *p2;
+extern typeof(p1) p1;
+extern typeof_unqual(p1) p2;
+
+// Ensure array qualifications are removed
+extern const int aci[2];
+extern const int acii[2][2];
+extern int ai[2];
+extern int aii[2][2];
+extern typeof(aci) aci;
+extern typeof_unqual(aci) ai;
+extern typeof(acii) acii;
+extern typeof_unqual(acii) aii;
+
+extern int *restrict arpi[2];
+extern int *restrict arpii[2][2];
+extern int *api[2];
+extern int *apii[2][2];
+extern typeof(arpi) arpi;
+extern typeof_unqual(arpi) api;
+extern typeof(arpii) arpii;
+extern typeof_unqual(arpii) apii;
AaronBallman wrote:
GCC also disagrees on these `typeof_unqual`s, but I think that behavior is also
incorrect for the same reason as above.
https://github.com/llvm/llvm-project/pull/92767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [C23] Fix typeof_unqual for qualified array types (PR #92767)
@@ -92,3 +92,43 @@ extern __attribute__((address_space(0))) int
type_attr_test_2; // expec
void invalid_param_fn(__attribute__((address_space(1))) int i); //
expected-error {{parameter may not be qualified with an address space}}
typeof(invalid_param_fn) invalid_param_1;
typeof_unqual(invalid_param_fn) invalid_param_2;
+
+// Ensure restrict is stripped
+extern int *restrict p1;
+extern int *p2;
+extern typeof(p1) p1;
+extern typeof_unqual(p1) p2;
+
+// Ensure array qualifications are removed
+extern const int aci[2];
+extern const int acii[2][2];
+extern int ai[2];
+extern int aii[2][2];
+extern typeof(aci) aci;
+extern typeof_unqual(aci) ai;
+extern typeof(acii) acii;
+extern typeof_unqual(acii) aii;
AaronBallman wrote:
Curiously, this is different from the results in GCC:
https://godbolt.org/z/5rczxdGKn
Based on my understanding of "An array and its element type are always
considered to be identically qualified.", I think GCC's behavior is incorrect,
but it would be good to file an issue with them to let them know to ensure we
agree.
https://github.com/llvm/llvm-project/pull/92767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/www/get_started.html] Use newer `cmake` syntax (PR #93503)
@@ -67,15 +67,13 @@ On Unix-like Systems Build LLVM and Clang: cd llvm-project -mkdir build (in-tree build is not supported) -cd build This builds both LLVM and Clang in release mode. Alternatively, if you need a debug build, switch Release to Debug. See https://llvm.org/docs/CMake.html#frequently-used-cmake-variables;>frequently used cmake variables for more options. -cmake -DLLVM_ENABLE_PROJECTS=clang -DCMAKE_BUILD_TYPE=Release -G "Unix Makefiles" ../llvm AaronBallman wrote: I'm wondering what the benefits are to using `-S` and `-B`? Is it purely as shorthand for `cd` and `mkdir` effectively, or does it do more beyond that? (I think we want the docs to be as general as possible and we want to lean on CMake's documentation for bells and whistles.) https://github.com/llvm/llvm-project/pull/93503 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] Avoid object libraries in the VS IDE (PR #93519)
AaronBallman wrote: I applied the patch locally and tried it out, and I think this is an improvement for folks generating VS solutions. https://github.com/llvm/llvm-project/pull/93519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] fix ast-print of `extern ` with >=2 declarators (PR #93131)
@@ -1064,6 +1090,8 @@ void
DeclPrinter::VisitNamespaceAliasDecl(NamespaceAliasDecl *D) {
void DeclPrinter::VisitEmptyDecl(EmptyDecl *D) {
prettyPrintAttributes(D);
+ if (const char *lang = tryGetUnbracedLinkageLanguage(D))
+Out << "extern \"" << lang << "\";";
AaronBallman wrote:
```suggestion
if (const char *Lang = tryGetUnbracedLinkageLanguage(D))
Out << "extern \"" << Lang << "\";";
```
https://github.com/llvm/llvm-project/pull/93131
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] fix ast-print of `extern ` with >=2 declarators (PR #93131)
@@ -662,6 +678,11 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
CXXConversionDecl *ConversionDecl = dyn_cast(D);
CXXDeductionGuideDecl *GuideDecl = dyn_cast(D);
if (!Policy.SuppressSpecifiers) {
+if (const char *lang = tryGetUnbracedLinkageLanguage(D)) {
+ // the "extern" specifier is implicit
+ assert(D->getStorageClass() == SC_None);
+ Out << "extern \"" << lang << "\" ";
+}
AaronBallman wrote:
```suggestion
if (const char *Lang = tryGetUnbracedLinkageLanguage(D)) {
// the "extern" specifier is implicit
assert(D->getStorageClass() == SC_None);
Out << "extern \"" << Lang << "\" ";
}
```
https://github.com/llvm/llvm-project/pull/93131
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] fix ast-print of `extern ` with >=2 declarators (PR #93131)
@@ -932,6 +953,11 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
: D->getASTContext().getUnqualifiedObjCPointerType(D->getType());
if (!Policy.SuppressSpecifiers) {
+if (const char *lang = tryGetUnbracedLinkageLanguage(D)) {
+ // the "extern" specifier is implicit
+ assert(D->getStorageClass() == SC_None);
+ Out << "extern \"" << lang << "\" ";
+}
AaronBallman wrote:
```suggestion
if (const char *Lang = tryGetUnbracedLinkageLanguage(D)) {
// the "extern" specifier is implicit
assert(D->getStorageClass() == SC_None);
Out << "extern \"" << Lang << "\" ";
}
```
https://github.com/llvm/llvm-project/pull/93131
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] fix ast-print of `extern ` with >=2 declarators (PR #93131)
https://github.com/AaronBallman approved this pull request. LGTM aside from some coding style nits. Because AST printing is not part of the public interface for Clang, I don't think these changes need a release note. https://github.com/llvm/llvm-project/pull/93131 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] fix ast-print of `extern ` with >=2 declarators (PR #93131)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93131 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Make warning pragma override -Werror=foo and DefaultError warnings (PR #93647)
https://github.com/AaronBallman commented: Adding @jyknight because of our discussions on related behavior regarding `DefaultError` diagnostics and suppression. https://github.com/llvm/llvm-project/pull/93647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Make warning pragma override -Werror=foo and DefaultError warnings (PR #93647)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Make warning pragma override -Werror=foo and DefaultError warnings (PR #93647)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Make warning pragma override -Werror=foo and DefaultError warnings (PR #93647)
https://github.com/AaronBallman commented: Thank you for this! Please add a release note to clang/docs/ReleaseNotes.rst so users know about the behavioral change. Also, should we update some documentation as well? https://github.com/llvm/llvm-project/pull/93647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] Introduce target-specific `Sema` components (PR #93179)
https://github.com/AaronBallman approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/93179 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] Introduce target-specific `Sema` components (PR #93179)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93179 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] Introduce target-specific `Sema` components (PR #93179)
@@ -0,0 +1,36 @@
+//===- Attr.h --- Helper functions for attribute handling in Sema
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file provides helpers for Sema functions that handle attributes.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_SEMA_ATTR_H
+#define LLVM_CLANG_SEMA_ATTR_H
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "llvm/Support/Casting.h"
+
+namespace clang {
+
+/// isFunctionOrMethod - Return true if the given decl has function
+/// type (function or function-typed variable) or an Objective-C
+/// method.
+inline bool isFunctionOrMethod(const Decl *D) {
AaronBallman wrote:
We could go with `isFuncOrMethodForAttrSubject()` since it seems that we have
to expose the interface... I don't love it, but I could live with it?
https://github.com/llvm/llvm-project/pull/93179
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] require template arg list after template kw (PR #80801)
@@ -2995,13 +2996,23 @@ bool Parser::ParseUnqualifiedId(CXXScopeSpec ,
ParsedType ObjectType,
SS, ObjectType, ObjectHadErrors,
TemplateKWLoc ? *TemplateKWLoc : SourceLocation(), Id, IdLoc,
EnteringContext, Result, TemplateSpecified);
-else if (TemplateSpecified &&
- Actions.ActOnTemplateName(
- getCurScope(), SS, *TemplateKWLoc, Result, ObjectType,
- EnteringContext, Template,
- /*AllowInjectedClassName*/ true) == TNK_Non_template)
- return true;
+if (TemplateSpecified) {
+ TemplateNameKind TNK =
+ Actions.ActOnTemplateName(getCurScope(), SS, *TemplateKWLoc, Result,
+ObjectType, EnteringContext, Template,
+/*AllowInjectedClassName*/ true);
+ if (TNK == TNK_Non_template)
+return true;
+
+ // C++ [template.names]p6
+ // A name prefixed by the keyword template shall be followed by a
template
+ // argument list or refer to a class template or an alias template.
+ if ((TNK == TNK_Function_template || TNK == TNK_Dependent_template_name
||
AaronBallman wrote:
What about type templates: https://godbolt.org/z/1coGTT38P (hmm we may have a
wider issue there, from looking at Clang's current behavior, so a FIXME would
be reasonable too).
https://github.com/llvm/llvm-project/pull/80801
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] require template arg list after template kw (PR #80801)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/80801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] require template arg list after template kw (PR #80801)
https://github.com/AaronBallman commented: Thank you for this! The changes should also come with a release note in clang/docs/ReleaseNotes.rst so users know about the change. https://github.com/llvm/llvm-project/pull/80801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C] Disallow declarations where a statement is required (PR #92908)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/92908 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Disallow explicit object parameters in more contexts (PR #89078)
@@ -11463,6 +11463,23 @@ void
Sema::CheckExplicitObjectMemberFunction(Declarator ,
D.setInvalidType();
}
+ // Handle the following case:
+ //
+ // struct S {
+ // struct T {
+ // int f(this T);
+ // };
+ //
+ // friend int T::f(this T); // Allow this.
+ // friend int f(this S);// But disallow this.
+ // };
+ if (D.getDeclSpec().isFriendSpecified() && D.getCXXScopeSpec().isEmpty()) {
AaronBallman wrote:
Heh, another terrible test case to consider but should just fall out naturally
is:
```
struct S {
int friend func(this T);
};
```
https://github.com/llvm/llvm-project/pull/89078
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] Introduce target-specific `Sema` components (PR #93179)
@@ -3548,6 +3561,56 @@ class Sema final : public SemaBase {
BuiltinFunction
};
+ /// A helper function to provide Attribute Location for the Attr types
+ /// AND the ParsedAttr.
+ template
+ static std::enable_if_t, SourceLocation>
+ getAttrLoc(const AttrInfo ) {
+return AL.getLocation();
+ }
+ SourceLocation getAttrLoc(const ParsedAttr );
+
+ /// If Expr is a valid integer constant, get the value of the integer
+ /// expression and return success or failure. May output an error.
+ ///
+ /// Negative argument is implicitly converted to unsigned, unless
+ /// \p StrictlyUnsigned is true.
+ template
+ bool checkUInt32Argument(const AttrInfo , const Expr *Expr, uint32_t ,
+ unsigned Idx = UINT_MAX,
+ bool StrictlyUnsigned = false) {
+std::optional I = llvm::APSInt(32);
+if (Expr->isTypeDependent() ||
+!(I = Expr->getIntegerConstantExpr(Context))) {
+ if (Idx != UINT_MAX)
+Diag(getAttrLoc(AI), diag::err_attribute_argument_n_type)
+<< << Idx << AANT_ArgumentIntegerConstant
+<< Expr->getSourceRange();
+ else
+Diag(getAttrLoc(AI), diag::err_attribute_argument_type)
+<< << AANT_ArgumentIntegerConstant << Expr->getSourceRange();
+ return false;
+}
+
+if (!I->isIntN(32)) {
+ Diag(Expr->getExprLoc(), diag::err_ice_too_large)
+ << toString(*I, 10, false) << 32 << /* Unsigned */ 1;
+ return false;
+}
+
+if (StrictlyUnsigned && I->isSigned() && I->isNegative()) {
+ Diag(getAttrLoc(AI), diag::err_attribute_requires_positive_integer)
+ << << /*non-negative*/ 1;
+ return false;
+}
+
+Val = (uint32_t)I->getZExtValue();
+return true;
+ }
+
+ bool isFunctionOrMethod(const Decl *D);
+ bool isFunctionOrMethodOrBlock(const Decl *D);
AaronBallman wrote:
Ah I missed the other calls to `isFunctionOrMethod()`, sorry about that! I
think it might make sense to move the attribute helper functions to
`clang/include/clang/Sema/Attr.h` or somewhere similar.
https://github.com/llvm/llvm-project/pull/93179
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
@@ -817,11 +817,11 @@ class alignas(8) Decl {
"owned local decl but no local module storage");
return reinterpret_cast(this)[-1];
}
- void setLocalOwningModule(Module *M) {
+ void setLocalOwningModule(const Module *M) {
AaronBallman wrote:
This is an example where it's not clear to me that it's correct -- the caller
assumes "this will never be mutated" but that's not a safe assumption given
`Module *getLocalOwningModule() const;` (which itself is not const correct!
that should be a pair of functions where the `const` variant returns a `const
Module *` and the non-`const` variant returns a `Module *`)
https://github.com/llvm/llvm-project/pull/93493
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
AaronBallman wrote: > My philosophy on problems like that is that the first step to getting out of > a hole is to stop digging the hole. We aren't going to make all of clang > const-correct overnight, right? Yup! I think we're in agreement with the long-term goal and how we get there. The devil is in the details (as always). For example, one thing we do all over the place in Clang is `const_cast` away constness of AST nodes, safe in the knowledge that they have to have been dynamically allocated, and thus we can never hit the UB of "original declaration of the object was const". I'd like to see us move away from this sort of "technically correct" and towards "obviously correct", and this moves in the right direction IMO. > To me, this commit is considering the bigger picture. Rather than just saying > "Oh, I am going to change the interface of Module so that it can control its > invariants and to do that I need to make just these parts const-correct", I > noticed I needed to make that change in some parts so as a preparatory commit > (this one) I first make sure we are as const-correct as we can be with the > current Module interface. Then I'll be going through and adding more const > later on as I refactor more of Module. Okay, that's good to know, thank you! > I could agree on being const correct here, but mostly by removing const, not > adding more, in the general case. Errr, not certain I agree with this -- that basically is "admit defeat and stop aiming for const correctness." https://github.com/llvm/llvm-project/pull/93493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
AaronBallman wrote: You don't have to convince me that being const-correct is a good thing, I'm already in full support of that. :-) The push back isn't "no, don't do this", it's "we're not const correct in a *lot* of places and so adding const correctness to a part of the compiler without considering the bigger picture means increasing the chances of needing to add `const_cast` in places to avoid viral changes in the future." https://github.com/llvm/llvm-project/pull/93493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] Introduce target-specific `Sema` components (PR #93179)
@@ -3548,6 +3561,56 @@ class Sema final : public SemaBase {
BuiltinFunction
};
+ /// A helper function to provide Attribute Location for the Attr types
+ /// AND the ParsedAttr.
+ template
+ static std::enable_if_t, SourceLocation>
+ getAttrLoc(const AttrInfo ) {
+return AL.getLocation();
+ }
+ SourceLocation getAttrLoc(const ParsedAttr );
+
+ /// If Expr is a valid integer constant, get the value of the integer
+ /// expression and return success or failure. May output an error.
+ ///
+ /// Negative argument is implicitly converted to unsigned, unless
+ /// \p StrictlyUnsigned is true.
+ template
+ bool checkUInt32Argument(const AttrInfo , const Expr *Expr, uint32_t ,
+ unsigned Idx = UINT_MAX,
+ bool StrictlyUnsigned = false) {
+std::optional I = llvm::APSInt(32);
+if (Expr->isTypeDependent() ||
+!(I = Expr->getIntegerConstantExpr(Context))) {
+ if (Idx != UINT_MAX)
+Diag(getAttrLoc(AI), diag::err_attribute_argument_n_type)
+<< << Idx << AANT_ArgumentIntegerConstant
+<< Expr->getSourceRange();
+ else
+Diag(getAttrLoc(AI), diag::err_attribute_argument_type)
+<< << AANT_ArgumentIntegerConstant << Expr->getSourceRange();
+ return false;
+}
+
+if (!I->isIntN(32)) {
+ Diag(Expr->getExprLoc(), diag::err_ice_too_large)
+ << toString(*I, 10, false) << 32 << /* Unsigned */ 1;
+ return false;
+}
+
+if (StrictlyUnsigned && I->isSigned() && I->isNegative()) {
+ Diag(getAttrLoc(AI), diag::err_attribute_requires_positive_integer)
+ << << /*non-negative*/ 1;
+ return false;
+}
+
+Val = (uint32_t)I->getZExtValue();
+return true;
+ }
+
+ bool isFunctionOrMethod(const Decl *D);
+ bool isFunctionOrMethodOrBlock(const Decl *D);
AaronBallman wrote:
It's not clear to me that these should be public interfaces in Sema. We have
`DeclBase::isFunctionOrMethod()` which is subtly different. When these were
static functions in SemaDeclAttr.cpp, they were a bit more reasonable (if still
somewhat poorly named).
It looks like `isFunctionOrMethodOrBlock()` doesn't need to move out of
SemaDeclAttr.cpp, and the only reason `isFunctionOrMethod()` is exposed is
because of WebAssembly. Perhaps a horrible hack would be to leave the interface
in SemaDeclAttr.cpp, make it `extern`, and forward declare for the use in wasm?
https://github.com/llvm/llvm-project/pull/93179
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
https://github.com/AaronBallman commented: In general, I'm in favor of improving const-correctness like this. That said, I share the concerns of other reviewers that we should be sure we're adding const-correctness where it makes sense to do so. e.g., functions named `getSomething()` that return a pointer should probably be returning a const pointer. But for a function named `setSomething()` taking a pointer, it might not make sense to take a const pointer even if that's permissible currently. https://github.com/llvm/llvm-project/pull/93493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 273777e - clang:: to llvm::; NFC
Author: Aaron Ballman
Date: 2024-05-28T12:05:55-04:00
New Revision: 273777ead296c9ab2c157d16b750e3ee1ace08ec
URL:
https://github.com/llvm/llvm-project/commit/273777ead296c9ab2c157d16b750e3ee1ace08ec
DIFF:
https://github.com/llvm/llvm-project/commit/273777ead296c9ab2c157d16b750e3ee1ace08ec.diff
LOG: clang:: to llvm::; NFC
These interfaces are LLVM interfaces, not Clang ones; but this worked
because of LLVM.h adding the interfaces to the clang namespace.
Added:
Modified:
clang/lib/AST/APValue.cpp
clang/lib/Analysis/MacroExpansionContext.cpp
Removed:
diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index 8c77b563657d9..d8e33ff421c06 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -90,7 +90,7 @@ QualType APValue::LValueBase::getType() const {
// For a materialized temporary, the type of the temporary we materialized
// may not be the type of the expression.
if (const MaterializeTemporaryExpr *MTE =
- clang::dyn_cast(Base)) {
+ llvm::dyn_cast(Base)) {
SmallVector CommaLHSs;
SmallVector Adjustments;
const Expr *Temp = MTE->getSubExpr();
diff --git a/clang/lib/Analysis/MacroExpansionContext.cpp
b/clang/lib/Analysis/MacroExpansionContext.cpp
index 564e359668a51..b212b7f245792 100644
--- a/clang/lib/Analysis/MacroExpansionContext.cpp
+++ b/clang/lib/Analysis/MacroExpansionContext.cpp
@@ -12,7 +12,7 @@
#define DEBUG_TYPE "macro-expansion-context"
-static void dumpTokenInto(const clang::Preprocessor , clang::raw_ostream
,
+static void dumpTokenInto(const clang::Preprocessor , llvm::raw_ostream ,
clang::Token Tok);
namespace clang {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ASTMatchers] forCallable should not erase binding on success (PR #89657)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/89657 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ASTMatchers] forCallable should not erase binding on success (PR #89657)
https://github.com/AaronBallman commented: The changes look reasonable to me, but can you also add a release note to clang/docs/ReleaseNotes.rst so that users of AST matchers know about the fix? https://github.com/llvm/llvm-project/pull/89657 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] fix ast-print of `extern ` with >=2 declarators (PR #93131)
https://github.com/AaronBallman commented: I think we're going to be printing incorrect code either way, but with this patch, we're more likely to be closer to the original intent of the code than without the patch. So even though this isn't perfect, it's forward progress and I think we're fine to land it as-is. However, if we can fix it correctly, that would be preferred. https://github.com/llvm/llvm-project/pull/93131 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] fix ast-print of `extern ` with >=2 declarators (PR #93131)
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -ast-print %s -o - | FileCheck %s
+
+// CHECK: extern "C" int printf(const char *, ...);
+extern "C" int printf(const char *...);
+
+// CHECK: extern "C++" {
AaronBallman wrote:
Oofda, there are differences between the braced and non-braced versions, so
ideally, we want to reproduce exactly what was in the user's source to avoid
accidentally changing the meaning of code. Consider cases like:
```
extern "C" int a, b, c;
extern "C" {
int x;
int y;
int z;
}
```
`z`, `y`, and `z` are external, `a`, `b`, and `c` are not:
https://godbolt.org/z/evG9KPKMM
https://github.com/llvm/llvm-project/pull/93131
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] fix ast-print of `extern ` with >=2 declarators (PR #93131)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93131 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Disallow explicit object parameters in more contexts (PR #89078)
@@ -4845,6 +4845,55 @@ static TypeSourceInfo
*GetFullTypeForDeclarator(TypeProcessingState ,
// Check for auto functions and trailing return type and adjust the
// return type accordingly.
if (!D.isInvalidType()) {
+auto isClassType = [&](CXXScopeSpec ) {
AaronBallman wrote:
```suggestion
auto IsClassType = [&](CXXScopeSpec ) {
```
Our goofy naming style...
https://github.com/llvm/llvm-project/pull/89078
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Disallow explicit object parameters in more contexts (PR #89078)
@@ -7525,6 +7525,8 @@ def err_explicit_object_parameter_mutable: Error< def err_invalid_explicit_object_type_in_lambda: Error< "invalid explicit object parameter type %0 in lambda with capture; " "the type must be the same as, or derived from, the lambda">; +def err_explicit_object_parameter_invalid: Error< + "an explicit object parameter is not allowed here">; AaronBallman wrote: "here" is pretty ambiguous. Any chance we can reuse the `an explicit object parameter cannot appear in a non-member function` diagnostic, add a `%select` to it, and give specific names for what "here" is? https://github.com/llvm/llvm-project/pull/89078 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Disallow explicit object parameters in more contexts (PR #89078)
@@ -4845,6 +4845,55 @@ static TypeSourceInfo
*GetFullTypeForDeclarator(TypeProcessingState ,
// Check for auto functions and trailing return type and adjust the
// return type accordingly.
if (!D.isInvalidType()) {
+auto isClassType = [&](CXXScopeSpec ) {
+ // If there already was an problem with the scope, don’t issue
another
+ // error about the explicit object parameter.
+ return SS.isInvalid() ||
+ isa_and_present(S.computeDeclContext(SS));
+};
+
+// [dcl.fct]p6:
+//
+// An explicit-object-parameter-declaration is a parameter-declaration
+// with a this specifier. An explicit-object-parameter-declaration
shall
+// appear only as the first parameter-declaration of a
+// parameter-declaration-list of either:
+//
+// - a member-declarator that declares a member function [class.mem],
or
+// - a lambda-declarator [expr.prim.lambda].
+DeclaratorContext C = D.getContext();
+ParmVarDecl *First =
+FTI.NumParams
+? dyn_cast_if_present(FTI.Params[0].Param)
+: nullptr;
+
+auto IsFunctionDecl = D.getInnermostNonParenChunk() ==
AaronBallman wrote:
```suggestion
bool IsFunctionDecl = D.getInnermostNonParenChunk() ==
```
https://github.com/llvm/llvm-project/pull/89078
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [C23] Fix typeof_unqual for qualified array types (PR #92767)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/92767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [C23] Fix typeof_unqual for qualified array types (PR #92767)
https://github.com/AaronBallman approved this pull request. LGTM! Precommit CI failure is unrelated to your changes. https://github.com/llvm/llvm-project/pull/92767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Improve ast-dumper text printing of TemplateArgument (PR #93431)
@@ -1086,45 +1106,100 @@ void TextNodeDumper::VisitNullTemplateArgument(const
TemplateArgument &) {
void TextNodeDumper::VisitTypeTemplateArgument(const TemplateArgument ) {
OS << " type";
- dumpType(TA.getAsType());
+ dumpTemplateArgument(TA);
}
void TextNodeDumper::VisitDeclarationTemplateArgument(
const TemplateArgument ) {
OS << " decl";
+ dumpTemplateArgument(TA);
dumpDeclRef(TA.getAsDecl());
}
-void TextNodeDumper::VisitNullPtrTemplateArgument(const TemplateArgument &) {
+void TextNodeDumper::VisitNullPtrTemplateArgument(const TemplateArgument ) {
OS << " nullptr";
+ dumpTemplateArgument(TA);
}
void TextNodeDumper::VisitIntegralTemplateArgument(const TemplateArgument )
{
- OS << " integral " << TA.getAsIntegral();
+ OS << " integral";
+ dumpTemplateArgument(TA);
+}
+
+void TextNodeDumper::dumpTemplateName(TemplateName TN) {
+ switch (TN.getKind()) {
+ case TemplateName::Template:
+AddChild([=] { Visit(TN.getAsTemplateDecl()); });
+return;
+ case TemplateName::UsingTemplate: {
+const UsingShadowDecl *USD = TN.getAsUsingShadowDecl();
+AddChild([=] { Visit(USD); });
+AddChild("target", [=] { Visit(USD->getTargetDecl()); });
+return;
+ }
+ case TemplateName::QualifiedTemplate: {
+OS << " qualified";
+const QualifiedTemplateName *QTN = TN.getAsQualifiedTemplateName();
+if (QTN->hasTemplateKeyword())
+ OS << " keyword";
+dumpNestedNameSpecifier(QTN->getQualifier());
+dumpTemplateName(QTN->getUnderlyingTemplate());
+return;
+ }
+ case TemplateName::DependentTemplate: {
+OS << " dependent";
+const DependentTemplateName *DTN = TN.getAsDependentTemplateName();
+dumpNestedNameSpecifier(DTN->getQualifier());
+return;
+ }
+ case TemplateName::SubstTemplateTemplateParm: {
+const SubstTemplateTemplateParmStorage *STS =
+TN.getAsSubstTemplateTemplateParm();
+OS << " subst index " << STS->getIndex();
+if (std::optional PackIndex = STS->getPackIndex())
+ OS << " pack_index " << *PackIndex;
AaronBallman wrote:
Yeah, I'd say let's go with `subst_index` instead.
https://github.com/llvm/llvm-project/pull/93431
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Improve ast-dumper text printing of TemplateArgument (PR #93431)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Improve ast-dumper text printing of TemplateArgument (PR #93431)
https://github.com/AaronBallman commented: Should we also be updating JSONNodeDumper.cpp at the same time? https://github.com/llvm/llvm-project/pull/93431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Improve ast-dumper text printing of TemplateArgument (PR #93431)
@@ -947,6 +947,26 @@ void TextNodeDumper::dumpDeclRef(const Decl *D, StringRef
Label) {
});
}
+void TextNodeDumper::dumpTemplateArgument(const TemplateArgument ) {
+ llvm::SmallString<128> Str;
+ {
+llvm::raw_svector_ostream SS(Str);
+TA.print(PrintPolicy, SS, /*IncludeType=*/true);
+ }
+ OS << " '" << Str << "'";
+
+ if (TemplateArgument CanonTA = Context->getCanonicalTemplateArgument(TA);
AaronBallman wrote:
```suggestion
if (const TemplateArgument =
Context->getCanonicalTemplateArgument(TA);
```
https://github.com/llvm/llvm-project/pull/93431
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C] Disallow declarations where a statement is required (PR #92908)
@@ -239,7 +239,10 @@ StmtResult
Parser::ParseStatementOrDeclarationAfterAttributes(
auto IsStmtAttr = [](ParsedAttr ) { return Attr.isStmtAttr(); };
bool AllAttrsAreStmtAttrs = llvm::all_of(CXX11Attrs, IsStmtAttr) &&
llvm::all_of(GNUAttrs, IsStmtAttr);
-if (((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
+if ((getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt ||
+ (StmtCtx & ParsedStmtContext::AllowDeclarationsInC) !=
+ ParsedStmtContext()) &&
+((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
AaronBallman wrote:
I've added one, let me know if you're happy with it.
https://github.com/llvm/llvm-project/pull/92908
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C] Disallow declarations where a statement is required (PR #92908)
https://github.com/AaronBallman updated
https://github.com/llvm/llvm-project/pull/92908
>From 219dae02c3235c17bc4568496a7df9763d798e2a Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Mon, 20 May 2024 13:40:28 -0400
Subject: [PATCH 1/5] [C] Fix declaration parsing
8bd06d5b65845e5e01dd899a2deb773580460b89 removed the notion of parsed
statement contexts that track whether a declaration is valid for C or
not. This resurrects the concept, because C still does not allow a
declaration as a substatement.
---
clang/include/clang/Parse/Parser.h | 9 ++---
clang/lib/Parse/ParseStmt.cpp | 5 -
clang/test/C/C99/block-scopes.c| 3 ++-
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Parse/Parser.h
b/clang/include/clang/Parse/Parser.h
index 5f04664141d29..5e455b16fb8c0 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -467,15 +467,18 @@ class Parser : public CodeCompletionHandler {
/// Flags describing a context in which we're parsing a statement.
enum class ParsedStmtContext {
+/// This context permits declarations in language modes where declarations
+/// are not statements.
+AllowDeclarationsInC = 0x1,
/// This context permits standalone OpenMP directives.
-AllowStandaloneOpenMPDirectives = 0x1,
+AllowStandaloneOpenMPDirectives = 0x2,
/// This context is at the top level of a GNU statement expression.
-InStmtExpr = 0x2,
+InStmtExpr = 0x4,
/// The context of a regular substatement.
SubStmt = 0,
/// The context of a compound-statement.
-Compound = AllowStandaloneOpenMPDirectives,
+Compound = AllowDeclarationsInC | AllowStandaloneOpenMPDirectives,
LLVM_MARK_AS_BITMASK_ENUM(InStmtExpr)
};
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index b0af04451166c..57b9fb10a1e08 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -239,7 +239,10 @@ StmtResult
Parser::ParseStatementOrDeclarationAfterAttributes(
auto IsStmtAttr = [](ParsedAttr ) { return Attr.isStmtAttr(); };
bool AllAttrsAreStmtAttrs = llvm::all_of(CXX11Attrs, IsStmtAttr) &&
llvm::all_of(GNUAttrs, IsStmtAttr);
-if (((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
+if ((getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt ||
+ (StmtCtx & ParsedStmtContext::AllowDeclarationsInC) !=
+ ParsedStmtContext()) &&
+((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
isDeclarationStatement())) {
SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
DeclGroupPtrTy Decl;
diff --git a/clang/test/C/C99/block-scopes.c b/clang/test/C/C99/block-scopes.c
index 589047df3e52b..116e5d922593e 100644
--- a/clang/test/C/C99/block-scopes.c
+++ b/clang/test/C/C99/block-scopes.c
@@ -18,8 +18,9 @@
enum {a, b};
void different(void) {
- if (sizeof(enum {b, a}) != sizeof(int))
+ if (sizeof(enum {b, a}) != sizeof(int)) {
_Static_assert(a == 1, "");
+ }
/* In C89, the 'b' found here would have been from the enum declaration in
* the controlling expression of the selection statement, not from the global
* declaration. In C99 and later, that enumeration is scoped to the 'if'
>From c67502ddeb0c1db5c71fe1eb115250bd238c0d9d Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Tue, 21 May 2024 08:34:01 -0400
Subject: [PATCH 2/5] Update OpenACC test for slight behavioral change
---
clang/test/SemaOpenACC/parallel-loc-and-stmt.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
b/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
index ba29f6da8ba25..bbcdd823483a5 100644
--- a/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
+++ b/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
@@ -33,9 +33,11 @@ int foo3;
void func() {
// FIXME: Should we disallow this on declarations, or consider this to be on
- // the initialization?
+ // the initialization? This is currently rejected in C because
+ // Parser::ParseOpenACCDirectiveStmt() calls ParseStatement() and passes the
+ // statement context as "SubStmt" which does not allow for a declaration in
C.
#pragma acc parallel
- int foo;
+ int foo; // expected-error {{expected expression}}
#pragma acc parallel
{
>From 6e2e9c015ba5c60204f3ef71ebd63db8d986835a Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Tue, 21 May 2024 08:40:46 -0400
Subject: [PATCH 3/5] Add test coverage for changes
---
clang/test/Parser/decls.c | 39 +++
1 file changed, 39 insertions(+)
create mode 100644 clang/test/Parser/decls.c
diff --git a/clang/test/Parser/decls.c b/clang/test/Parser/decls.c
new file mode 100644
index 0..39ef05bf4bd99
--- /dev/null
+++ b/clang/test/Parser/decls.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
[clang] [C] Disallow declarations where a statement is required (PR #92908)
https://github.com/AaronBallman updated
https://github.com/llvm/llvm-project/pull/92908
>From 219dae02c3235c17bc4568496a7df9763d798e2a Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Mon, 20 May 2024 13:40:28 -0400
Subject: [PATCH 1/4] [C] Fix declaration parsing
8bd06d5b65845e5e01dd899a2deb773580460b89 removed the notion of parsed
statement contexts that track whether a declaration is valid for C or
not. This resurrects the concept, because C still does not allow a
declaration as a substatement.
---
clang/include/clang/Parse/Parser.h | 9 ++---
clang/lib/Parse/ParseStmt.cpp | 5 -
clang/test/C/C99/block-scopes.c| 3 ++-
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Parse/Parser.h
b/clang/include/clang/Parse/Parser.h
index 5f04664141d29..5e455b16fb8c0 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -467,15 +467,18 @@ class Parser : public CodeCompletionHandler {
/// Flags describing a context in which we're parsing a statement.
enum class ParsedStmtContext {
+/// This context permits declarations in language modes where declarations
+/// are not statements.
+AllowDeclarationsInC = 0x1,
/// This context permits standalone OpenMP directives.
-AllowStandaloneOpenMPDirectives = 0x1,
+AllowStandaloneOpenMPDirectives = 0x2,
/// This context is at the top level of a GNU statement expression.
-InStmtExpr = 0x2,
+InStmtExpr = 0x4,
/// The context of a regular substatement.
SubStmt = 0,
/// The context of a compound-statement.
-Compound = AllowStandaloneOpenMPDirectives,
+Compound = AllowDeclarationsInC | AllowStandaloneOpenMPDirectives,
LLVM_MARK_AS_BITMASK_ENUM(InStmtExpr)
};
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index b0af04451166c..57b9fb10a1e08 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -239,7 +239,10 @@ StmtResult
Parser::ParseStatementOrDeclarationAfterAttributes(
auto IsStmtAttr = [](ParsedAttr ) { return Attr.isStmtAttr(); };
bool AllAttrsAreStmtAttrs = llvm::all_of(CXX11Attrs, IsStmtAttr) &&
llvm::all_of(GNUAttrs, IsStmtAttr);
-if (((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
+if ((getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt ||
+ (StmtCtx & ParsedStmtContext::AllowDeclarationsInC) !=
+ ParsedStmtContext()) &&
+((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
isDeclarationStatement())) {
SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
DeclGroupPtrTy Decl;
diff --git a/clang/test/C/C99/block-scopes.c b/clang/test/C/C99/block-scopes.c
index 589047df3e52b..116e5d922593e 100644
--- a/clang/test/C/C99/block-scopes.c
+++ b/clang/test/C/C99/block-scopes.c
@@ -18,8 +18,9 @@
enum {a, b};
void different(void) {
- if (sizeof(enum {b, a}) != sizeof(int))
+ if (sizeof(enum {b, a}) != sizeof(int)) {
_Static_assert(a == 1, "");
+ }
/* In C89, the 'b' found here would have been from the enum declaration in
* the controlling expression of the selection statement, not from the global
* declaration. In C99 and later, that enumeration is scoped to the 'if'
>From c67502ddeb0c1db5c71fe1eb115250bd238c0d9d Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Tue, 21 May 2024 08:34:01 -0400
Subject: [PATCH 2/4] Update OpenACC test for slight behavioral change
---
clang/test/SemaOpenACC/parallel-loc-and-stmt.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
b/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
index ba29f6da8ba25..bbcdd823483a5 100644
--- a/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
+++ b/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
@@ -33,9 +33,11 @@ int foo3;
void func() {
// FIXME: Should we disallow this on declarations, or consider this to be on
- // the initialization?
+ // the initialization? This is currently rejected in C because
+ // Parser::ParseOpenACCDirectiveStmt() calls ParseStatement() and passes the
+ // statement context as "SubStmt" which does not allow for a declaration in
C.
#pragma acc parallel
- int foo;
+ int foo; // expected-error {{expected expression}}
#pragma acc parallel
{
>From 6e2e9c015ba5c60204f3ef71ebd63db8d986835a Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Tue, 21 May 2024 08:40:46 -0400
Subject: [PATCH 3/4] Add test coverage for changes
---
clang/test/Parser/decls.c | 39 +++
1 file changed, 39 insertions(+)
create mode 100644 clang/test/Parser/decls.c
diff --git a/clang/test/Parser/decls.c b/clang/test/Parser/decls.c
new file mode 100644
index 0..39ef05bf4bd99
--- /dev/null
+++ b/clang/test/Parser/decls.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
[clang] [clang] Diagnose problematic diagnostic messages (PR #93229)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/93229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] allow `` `@$ `` in raw string delimiters in C++26 (PR #93216)
https://github.com/AaronBallman approved this pull request. LGTM, thank you! https://github.com/llvm/llvm-project/pull/93216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] allow `` `@$ `` in raw string delimiters in C++26 (PR #93216)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/93216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Diagnose problematic diagnostic messages (PR #93229)
https://github.com/AaronBallman updated
https://github.com/llvm/llvm-project/pull/93229
>From 38d6d9b809e1cf9d6a8f577630c838421486cd04 Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Thu, 23 May 2024 14:55:16 -0400
Subject: [PATCH 1/5] Diagnose problematic diagnostic messages
Clang has some unwritten rules about diagnostic wording regarding
things like punctuation and capitalization. This patch documents those
rules and adds some tablegen support for checking diagnostics follow
the rules.
Specifically: tablegen now checks that a diagnostic does not start with
a capital letter or end with punctuation, except for the usual
exceptions like proper nouns or ending with a question.
---
clang/docs/InternalsManual.rst| 38
clang/test/TableGen/deferred-diag.td | 10 +-
clang/test/TableGen/text-substitution.td | 4 +-
clang/test/TableGen/wording-errors.td | 55 +
.../TableGen/ClangDiagnosticsEmitter.cpp | 191 ++
5 files changed, 291 insertions(+), 7 deletions(-)
create mode 100644 clang/test/TableGen/wording-errors.td
diff --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst
index b3e2b870ae5f9..3d21e37784b36 100644
--- a/clang/docs/InternalsManual.rst
+++ b/clang/docs/InternalsManual.rst
@@ -123,6 +123,44 @@ severe that error recovery won't be able to recover
sensibly from them (thus
spewing a ton of bogus errors). One example of this class of error are failure
to ``#include`` a file.
+Diagnostic Wording
+^^
+The wording used for a diagnostic is critical because it is the only way for a
+user to know how to correct their code. Use the following suggestions when
+wording a diagnostic.
+
+* Diagnostics in Clang do not start with a capital letter and do not end with
+ punctuation.
+
+* This does not apply to proper nouns like ``Clang`` or ``OpenMP``, to
+ acronyms like ``GCC`` or ``ARC``, or to language standards like ``C23``
+ or ``C++17``.
+* A trailing question mark is allowed. e.g., ``unknown identifier %0; did
+ you mean %1?``.
+
+* Appropriately capitalize proper nouns like ``Clang``, ``OpenCL``, ``GCC``,
+ ``Objective-C``, etc and language standard versions like ``C11`` or
``C++11``.
+* The wording should be succinct. If necessary, use a semicolon to combine
+ sentence fragments instead of using complete sentences. e.g., prefer wording
+ like ``'%0' is deprecated; it will be removed in a future release of Clang``
+ over wording like ``'%0' is deprecated. It will be removed in a future
release
+ of Clang``.
+* The wording should be actionable and avoid using standards terms or grammar
+ productions that a new user would not be familiar with. e.g., prefer wording
+ like ``missing semicolon`` over wording like ``syntax error`` (which is not
+ actionable) or ``expected unqualified-id`` (which uses standards
terminology).
+* The wording should clearly explain what is wrong with the code rather than
+ restating what the code does. e.g., prefer wording like ``type %0 requires a
+ value in the range %1 to %2`` over wording like ``%0 is invalid``.
+* The wording should have enough contextual information to help the user
+ identify the issue in a complex expression. e.g., prefer wording like
+ ``both sides of the %0 binary operator are identical`` over wording like
+ ``identical operands to binary operator``.
+* Use single quotes to denote syntactic constructs or command line arguments
+ named in a diagnostic message. e.g., prefer wording like ``'this' pointer
+ cannot be null in well-defined C++ code`` over wording like ``this pointer
+ cannot be null in well-defined C++ code``.
+
The Format String
^
diff --git a/clang/test/TableGen/deferred-diag.td
b/clang/test/TableGen/deferred-diag.td
index c1906d4a9e45e..d7e8e694c7b3e 100644
--- a/clang/test/TableGen/deferred-diag.td
+++ b/clang/test/TableGen/deferred-diag.td
@@ -4,24 +4,24 @@ include "DiagnosticBase.inc"
// Test usage of Deferrable and NonDeferrable in diagnostics.
-def test_default : Error<"This error is non-deferrable by default">;
+def test_default : Error<"this error is non-deferrable by default">;
// CHECK-DAG: DIAG(test_default, {{.*}}SFINAE_SubstitutionFailure, false,
true, true, false, 0)
-def test_deferrable : Error<"This error is deferrable">, Deferrable;
+def test_deferrable : Error<"this error is deferrable">, Deferrable;
// CHECK-DAG: DIAG(test_deferrable, {{.*}} SFINAE_SubstitutionFailure, false,
true, true, true, 0)
-def test_non_deferrable : Error<"This error is non-deferrable">, NonDeferrable;
+def test_non_deferrable : Error<"this error is non-deferrable">, NonDeferrable;
// CHECK-DAG: DIAG(test_non_deferrable, {{.*}} SFINAE_SubstitutionFailure,
false, true, true, false, 0)
let Deferrable = 1 in {
-def test_let : Error<"This error is deferrable by let">;
+def test_let : Error<"this error is deferrable by let">;
// CHECK-DAG:
[clang] [clang] Diagnose problematic diagnostic messages (PR #93229)
https://github.com/AaronBallman updated
https://github.com/llvm/llvm-project/pull/93229
>From 38d6d9b809e1cf9d6a8f577630c838421486cd04 Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Thu, 23 May 2024 14:55:16 -0400
Subject: [PATCH 1/4] Diagnose problematic diagnostic messages
Clang has some unwritten rules about diagnostic wording regarding
things like punctuation and capitalization. This patch documents those
rules and adds some tablegen support for checking diagnostics follow
the rules.
Specifically: tablegen now checks that a diagnostic does not start with
a capital letter or end with punctuation, except for the usual
exceptions like proper nouns or ending with a question.
---
clang/docs/InternalsManual.rst| 38
clang/test/TableGen/deferred-diag.td | 10 +-
clang/test/TableGen/text-substitution.td | 4 +-
clang/test/TableGen/wording-errors.td | 55 +
.../TableGen/ClangDiagnosticsEmitter.cpp | 191 ++
5 files changed, 291 insertions(+), 7 deletions(-)
create mode 100644 clang/test/TableGen/wording-errors.td
diff --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst
index b3e2b870ae5f9..3d21e37784b36 100644
--- a/clang/docs/InternalsManual.rst
+++ b/clang/docs/InternalsManual.rst
@@ -123,6 +123,44 @@ severe that error recovery won't be able to recover
sensibly from them (thus
spewing a ton of bogus errors). One example of this class of error are failure
to ``#include`` a file.
+Diagnostic Wording
+^^
+The wording used for a diagnostic is critical because it is the only way for a
+user to know how to correct their code. Use the following suggestions when
+wording a diagnostic.
+
+* Diagnostics in Clang do not start with a capital letter and do not end with
+ punctuation.
+
+* This does not apply to proper nouns like ``Clang`` or ``OpenMP``, to
+ acronyms like ``GCC`` or ``ARC``, or to language standards like ``C23``
+ or ``C++17``.
+* A trailing question mark is allowed. e.g., ``unknown identifier %0; did
+ you mean %1?``.
+
+* Appropriately capitalize proper nouns like ``Clang``, ``OpenCL``, ``GCC``,
+ ``Objective-C``, etc and language standard versions like ``C11`` or
``C++11``.
+* The wording should be succinct. If necessary, use a semicolon to combine
+ sentence fragments instead of using complete sentences. e.g., prefer wording
+ like ``'%0' is deprecated; it will be removed in a future release of Clang``
+ over wording like ``'%0' is deprecated. It will be removed in a future
release
+ of Clang``.
+* The wording should be actionable and avoid using standards terms or grammar
+ productions that a new user would not be familiar with. e.g., prefer wording
+ like ``missing semicolon`` over wording like ``syntax error`` (which is not
+ actionable) or ``expected unqualified-id`` (which uses standards
terminology).
+* The wording should clearly explain what is wrong with the code rather than
+ restating what the code does. e.g., prefer wording like ``type %0 requires a
+ value in the range %1 to %2`` over wording like ``%0 is invalid``.
+* The wording should have enough contextual information to help the user
+ identify the issue in a complex expression. e.g., prefer wording like
+ ``both sides of the %0 binary operator are identical`` over wording like
+ ``identical operands to binary operator``.
+* Use single quotes to denote syntactic constructs or command line arguments
+ named in a diagnostic message. e.g., prefer wording like ``'this' pointer
+ cannot be null in well-defined C++ code`` over wording like ``this pointer
+ cannot be null in well-defined C++ code``.
+
The Format String
^
diff --git a/clang/test/TableGen/deferred-diag.td
b/clang/test/TableGen/deferred-diag.td
index c1906d4a9e45e..d7e8e694c7b3e 100644
--- a/clang/test/TableGen/deferred-diag.td
+++ b/clang/test/TableGen/deferred-diag.td
@@ -4,24 +4,24 @@ include "DiagnosticBase.inc"
// Test usage of Deferrable and NonDeferrable in diagnostics.
-def test_default : Error<"This error is non-deferrable by default">;
+def test_default : Error<"this error is non-deferrable by default">;
// CHECK-DAG: DIAG(test_default, {{.*}}SFINAE_SubstitutionFailure, false,
true, true, false, 0)
-def test_deferrable : Error<"This error is deferrable">, Deferrable;
+def test_deferrable : Error<"this error is deferrable">, Deferrable;
// CHECK-DAG: DIAG(test_deferrable, {{.*}} SFINAE_SubstitutionFailure, false,
true, true, true, 0)
-def test_non_deferrable : Error<"This error is non-deferrable">, NonDeferrable;
+def test_non_deferrable : Error<"this error is non-deferrable">, NonDeferrable;
// CHECK-DAG: DIAG(test_non_deferrable, {{.*}} SFINAE_SubstitutionFailure,
false, true, true, false, 0)
let Deferrable = 1 in {
-def test_let : Error<"This error is deferrable by let">;
+def test_let : Error<"this error is deferrable by let">;
// CHECK-DAG:
[clang] [clang] Diagnose problematic diagnostic messages (PR #93229)
https://github.com/AaronBallman updated
https://github.com/llvm/llvm-project/pull/93229
>From 38d6d9b809e1cf9d6a8f577630c838421486cd04 Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Thu, 23 May 2024 14:55:16 -0400
Subject: [PATCH 1/3] Diagnose problematic diagnostic messages
Clang has some unwritten rules about diagnostic wording regarding
things like punctuation and capitalization. This patch documents those
rules and adds some tablegen support for checking diagnostics follow
the rules.
Specifically: tablegen now checks that a diagnostic does not start with
a capital letter or end with punctuation, except for the usual
exceptions like proper nouns or ending with a question.
---
clang/docs/InternalsManual.rst| 38
clang/test/TableGen/deferred-diag.td | 10 +-
clang/test/TableGen/text-substitution.td | 4 +-
clang/test/TableGen/wording-errors.td | 55 +
.../TableGen/ClangDiagnosticsEmitter.cpp | 191 ++
5 files changed, 291 insertions(+), 7 deletions(-)
create mode 100644 clang/test/TableGen/wording-errors.td
diff --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst
index b3e2b870ae5f9..3d21e37784b36 100644
--- a/clang/docs/InternalsManual.rst
+++ b/clang/docs/InternalsManual.rst
@@ -123,6 +123,44 @@ severe that error recovery won't be able to recover
sensibly from them (thus
spewing a ton of bogus errors). One example of this class of error are failure
to ``#include`` a file.
+Diagnostic Wording
+^^
+The wording used for a diagnostic is critical because it is the only way for a
+user to know how to correct their code. Use the following suggestions when
+wording a diagnostic.
+
+* Diagnostics in Clang do not start with a capital letter and do not end with
+ punctuation.
+
+* This does not apply to proper nouns like ``Clang`` or ``OpenMP``, to
+ acronyms like ``GCC`` or ``ARC``, or to language standards like ``C23``
+ or ``C++17``.
+* A trailing question mark is allowed. e.g., ``unknown identifier %0; did
+ you mean %1?``.
+
+* Appropriately capitalize proper nouns like ``Clang``, ``OpenCL``, ``GCC``,
+ ``Objective-C``, etc and language standard versions like ``C11`` or
``C++11``.
+* The wording should be succinct. If necessary, use a semicolon to combine
+ sentence fragments instead of using complete sentences. e.g., prefer wording
+ like ``'%0' is deprecated; it will be removed in a future release of Clang``
+ over wording like ``'%0' is deprecated. It will be removed in a future
release
+ of Clang``.
+* The wording should be actionable and avoid using standards terms or grammar
+ productions that a new user would not be familiar with. e.g., prefer wording
+ like ``missing semicolon`` over wording like ``syntax error`` (which is not
+ actionable) or ``expected unqualified-id`` (which uses standards
terminology).
+* The wording should clearly explain what is wrong with the code rather than
+ restating what the code does. e.g., prefer wording like ``type %0 requires a
+ value in the range %1 to %2`` over wording like ``%0 is invalid``.
+* The wording should have enough contextual information to help the user
+ identify the issue in a complex expression. e.g., prefer wording like
+ ``both sides of the %0 binary operator are identical`` over wording like
+ ``identical operands to binary operator``.
+* Use single quotes to denote syntactic constructs or command line arguments
+ named in a diagnostic message. e.g., prefer wording like ``'this' pointer
+ cannot be null in well-defined C++ code`` over wording like ``this pointer
+ cannot be null in well-defined C++ code``.
+
The Format String
^
diff --git a/clang/test/TableGen/deferred-diag.td
b/clang/test/TableGen/deferred-diag.td
index c1906d4a9e45e..d7e8e694c7b3e 100644
--- a/clang/test/TableGen/deferred-diag.td
+++ b/clang/test/TableGen/deferred-diag.td
@@ -4,24 +4,24 @@ include "DiagnosticBase.inc"
// Test usage of Deferrable and NonDeferrable in diagnostics.
-def test_default : Error<"This error is non-deferrable by default">;
+def test_default : Error<"this error is non-deferrable by default">;
// CHECK-DAG: DIAG(test_default, {{.*}}SFINAE_SubstitutionFailure, false,
true, true, false, 0)
-def test_deferrable : Error<"This error is deferrable">, Deferrable;
+def test_deferrable : Error<"this error is deferrable">, Deferrable;
// CHECK-DAG: DIAG(test_deferrable, {{.*}} SFINAE_SubstitutionFailure, false,
true, true, true, 0)
-def test_non_deferrable : Error<"This error is non-deferrable">, NonDeferrable;
+def test_non_deferrable : Error<"this error is non-deferrable">, NonDeferrable;
// CHECK-DAG: DIAG(test_non_deferrable, {{.*}} SFINAE_SubstitutionFailure,
false, true, true, false, 0)
let Deferrable = 1 in {
-def test_let : Error<"This error is deferrable by let">;
+def test_let : Error<"this error is deferrable by let">;
// CHECK-DAG:
[clang] [clang] Diagnose problematic diagnostic messages (PR #93229)
https://github.com/AaronBallman updated
https://github.com/llvm/llvm-project/pull/93229
>From 38d6d9b809e1cf9d6a8f577630c838421486cd04 Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Thu, 23 May 2024 14:55:16 -0400
Subject: [PATCH 1/3] Diagnose problematic diagnostic messages
Clang has some unwritten rules about diagnostic wording regarding
things like punctuation and capitalization. This patch documents those
rules and adds some tablegen support for checking diagnostics follow
the rules.
Specifically: tablegen now checks that a diagnostic does not start with
a capital letter or end with punctuation, except for the usual
exceptions like proper nouns or ending with a question.
---
clang/docs/InternalsManual.rst| 38
clang/test/TableGen/deferred-diag.td | 10 +-
clang/test/TableGen/text-substitution.td | 4 +-
clang/test/TableGen/wording-errors.td | 55 +
.../TableGen/ClangDiagnosticsEmitter.cpp | 191 ++
5 files changed, 291 insertions(+), 7 deletions(-)
create mode 100644 clang/test/TableGen/wording-errors.td
diff --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst
index b3e2b870ae5f9..3d21e37784b36 100644
--- a/clang/docs/InternalsManual.rst
+++ b/clang/docs/InternalsManual.rst
@@ -123,6 +123,44 @@ severe that error recovery won't be able to recover
sensibly from them (thus
spewing a ton of bogus errors). One example of this class of error are failure
to ``#include`` a file.
+Diagnostic Wording
+^^
+The wording used for a diagnostic is critical because it is the only way for a
+user to know how to correct their code. Use the following suggestions when
+wording a diagnostic.
+
+* Diagnostics in Clang do not start with a capital letter and do not end with
+ punctuation.
+
+* This does not apply to proper nouns like ``Clang`` or ``OpenMP``, to
+ acronyms like ``GCC`` or ``ARC``, or to language standards like ``C23``
+ or ``C++17``.
+* A trailing question mark is allowed. e.g., ``unknown identifier %0; did
+ you mean %1?``.
+
+* Appropriately capitalize proper nouns like ``Clang``, ``OpenCL``, ``GCC``,
+ ``Objective-C``, etc and language standard versions like ``C11`` or
``C++11``.
+* The wording should be succinct. If necessary, use a semicolon to combine
+ sentence fragments instead of using complete sentences. e.g., prefer wording
+ like ``'%0' is deprecated; it will be removed in a future release of Clang``
+ over wording like ``'%0' is deprecated. It will be removed in a future
release
+ of Clang``.
+* The wording should be actionable and avoid using standards terms or grammar
+ productions that a new user would not be familiar with. e.g., prefer wording
+ like ``missing semicolon`` over wording like ``syntax error`` (which is not
+ actionable) or ``expected unqualified-id`` (which uses standards
terminology).
+* The wording should clearly explain what is wrong with the code rather than
+ restating what the code does. e.g., prefer wording like ``type %0 requires a
+ value in the range %1 to %2`` over wording like ``%0 is invalid``.
+* The wording should have enough contextual information to help the user
+ identify the issue in a complex expression. e.g., prefer wording like
+ ``both sides of the %0 binary operator are identical`` over wording like
+ ``identical operands to binary operator``.
+* Use single quotes to denote syntactic constructs or command line arguments
+ named in a diagnostic message. e.g., prefer wording like ``'this' pointer
+ cannot be null in well-defined C++ code`` over wording like ``this pointer
+ cannot be null in well-defined C++ code``.
+
The Format String
^
diff --git a/clang/test/TableGen/deferred-diag.td
b/clang/test/TableGen/deferred-diag.td
index c1906d4a9e45e..d7e8e694c7b3e 100644
--- a/clang/test/TableGen/deferred-diag.td
+++ b/clang/test/TableGen/deferred-diag.td
@@ -4,24 +4,24 @@ include "DiagnosticBase.inc"
// Test usage of Deferrable and NonDeferrable in diagnostics.
-def test_default : Error<"This error is non-deferrable by default">;
+def test_default : Error<"this error is non-deferrable by default">;
// CHECK-DAG: DIAG(test_default, {{.*}}SFINAE_SubstitutionFailure, false,
true, true, false, 0)
-def test_deferrable : Error<"This error is deferrable">, Deferrable;
+def test_deferrable : Error<"this error is deferrable">, Deferrable;
// CHECK-DAG: DIAG(test_deferrable, {{.*}} SFINAE_SubstitutionFailure, false,
true, true, true, 0)
-def test_non_deferrable : Error<"This error is non-deferrable">, NonDeferrable;
+def test_non_deferrable : Error<"this error is non-deferrable">, NonDeferrable;
// CHECK-DAG: DIAG(test_non_deferrable, {{.*}} SFINAE_SubstitutionFailure,
false, true, true, false, 0)
let Deferrable = 1 in {
-def test_let : Error<"This error is deferrable by let">;
+def test_let : Error<"this error is deferrable by let">;
// CHECK-DAG:
[clang] [clang] Diagnose problematic diagnostic messages (PR #93229)
@@ -1355,7 +1357,7 @@ static void verifyDiagnosticWording(const Record ) {
if (isDigit(FullDiagText.back()) && *(FullDiagText.end() - 2) == '}') {
// Scan backwards to find the opening curly brace.
size_t BraceCount = 1;
-auto Iter = FullDiagText.end() - /*}0*/ 3;
+auto Iter = FullDiagText.end() - sizeof("}0");
AaronBallman wrote:
It's because we want to get to the iterator just *before* the `}0`, so -2 gets
us to `}` while -3 gets us to the character before the `}`.
https://github.com/llvm/llvm-project/pull/93229
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
