[clang] [llvm] Recommit changes to global checks (PR #71171)
hnrklssn wrote: >I'd run ./llvm/utils/update_any_test_checks.py once and see if the tests pass >afterwards. Then do it again to ensure the nasty ordering and duplication issues are gone for good. All tests that fail after running update_any_test_checks.py also fail when running it without this patch, so I don't see any regressions there. Running it a second time doesn't result in any changes. https://github.com/llvm/llvm-project/pull/71171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Recommit changes to global checks (PR #71171)
https://github.com/hnrklssn closed https://github.com/llvm/llvm-project/pull/71171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
hnrklssn wrote: What differentiates the DecompositionDecl such that we need to mark the decl invalid when there's an error in the RHS, while for other decls we don't? It seems inconsistent. https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Sema::isSimpleTypeSpecifier return true for _Bool in c99 (PR #72204)
hnrklssn wrote: There's a dup of `Sema::isSimpleTypeSpecifier` in `FormatToken.cpp` that you may want to update also: `FormatToken::isSimpleTypeSpecifier`. Perhaps that'll make testing easier? https://github.com/llvm/llvm-project/pull/72204 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][InstrProf] Allow mix-up of absolute path with relative path on command line when using -fprofile-list= (PR #67519)
https://github.com/hnrklssn requested changes to this pull request. https://github.com/llvm/llvm-project/pull/67519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][InstrProf] Allow mix-up of absolute path with relative path on command line when using -fprofile-list= (PR #67519)
@@ -4,7 +4,8 @@ // RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -fprofile-list=%t-func.list -emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC // RUN: echo "src:%s" | sed -e 's/\\//g' > %t-file.list -// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -fprofile-list=%t-file.list -emit-llvm %s -o - | FileCheck %s --check-prefix=FILE +// RUN: cd %S +// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -fprofile-list=%t-file.list -emit-llvm profile-filter.c -o - | FileCheck %s --check-prefix=FILE hnrklssn wrote: Is `%s` an absolute or relative path? Either way, this only tests one of the cases. We should test both. I.e. if `%s` is the absolute path, there should also be a test for `"src:profile-filter.c"`. However, it `%s` is the absolute path, wasn't this already working before this patch? https://github.com/llvm/llvm-project/pull/67519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][InstrProf] Allow mix-up of absolute path with relative path on command line when using -fprofile-list= (PR #67519)
https://github.com/hnrklssn edited https://github.com/llvm/llvm-project/pull/67519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
hnrklssn wrote: > > What differentiates the DecompositionDecl such that we need to mark the > > decl invalid when there's an error in the RHS, while for other decls we > > don't? It seems inconsistent. > > > > DecompositionDecl (aka structure binding) is special here, a legal > DecompositionDecl must be declared with a deduced `auto` type, so the its > initializer should play part of the decl's invalid bit. For the error case > where a DecompositionDecl with a non-deduced type, clang still builds the > Decl AST node (with invalid bit) for better recovery. There are some > oversight cases. This patch is fixing those. Ah that makes sense then. I didn't realise it had to be `auto`. https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Interp] Handle std::move etc. builtins (PR #70772)
Timm =?utf-8?q?B=C3=A4der?= Message-ID: In-Reply-To: hnrklssn wrote: LGTM, but someone else should approve also. https://github.com/llvm/llvm-project/pull/70772 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang]get non-injected-class before finding instantiated decl (PR #70886)
@@ -429,3 +429,19 @@ namespace qualified_friend_no_match {
friend void Y::f(double); // expected-error {{friend declaration of 'f'
does not match any declaration in 'qualified_friend_no_match::Y'}}
};
}
+
+namespace gh21483 {
+template
+struct B {
+ struct mixin {
+int type;
+friend void foo(B::mixin it) {
+ (void)it.mixin::type;
hnrklssn wrote:
Why is `it.mixin` not an error? I thought `it` was of type `B::mixin`,
which has no `mixin` field.
https://github.com/llvm/llvm-project/pull/70886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix false positive -Wmissing-field-initializer for anonymous unions (PR #70829)
@@ -4,7 +4,7 @@ // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,reorder -Wno-c99-designator -Werror=reorder-init-list -Wno-initializer-overrides // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,override -Wno-c99-designator -Wno-reorder-init-list -Werror=initializer-overrides // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides -// RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,wmissing -Wmissing-field-initializers -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides +// RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,wmissing -Wmissing-field-initializers -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides -D NON_PEDANTIC hnrklssn wrote: Imo it's clearer to unconditionally compile the same code for each test case, and instead introduce another `-verify` prefix for the diagnostics that aren't emitted by this invocation. https://github.com/llvm/llvm-project/pull/70829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix false positive -Wmissing-field-initializer for anonymous unions (PR #70829)
@@ -2537,19 +2555,13 @@ class FieldInitializerValidatorCCC final : public
CorrectionCandidateCallback {
/// actually be initialized.
hnrklssn wrote:
The comment above looks like it needs to be updated with `@param`s for
`FinishSubobjectInit`, `TopLevelObject` and `InitializedFields`.
https://github.com/llvm/llvm-project/pull/70829
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang]get non-injected-class before finding instantiated decl (PR #70886)
@@ -429,3 +429,19 @@ namespace qualified_friend_no_match {
friend void Y::f(double); // expected-error {{friend declaration of 'f'
does not match any declaration in 'qualified_friend_no_match::Y'}}
};
}
+
+namespace gh21483 {
+template
+struct B {
+ struct mixin {
+int type;
+friend void foo(B::mixin it) {
+ (void)it.mixin::type;
hnrklssn wrote:
Ah I see it now after playing around with it a bit. In this case line 439 could
have accessed the same value through `(void)it.type` since `mixin` doesn't
inherit anything, but `it.mixin::type` is allowed regardless.
https://github.com/llvm/llvm-project/pull/70886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Interp] Handle std::move etc. builtins (PR #70772)
hnrklssn wrote: Would making a class with custom move and copy constructors with side effects, and moving/copying an instance around a bit to affect the results of static_asserts, work for testing? https://github.com/llvm/llvm-project/pull/70772 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Interp] Handle std::move etc. builtins (PR #70772)
Timm =?utf-8?q?Bäder?= Message-ID: In-Reply-To: hnrklssn wrote: Nice. I realise you're following the convention already established in the test file, but have you considered compiling with `-verify=new,both` and `-verify=ref,both`, respectively, and combining the shared diagnostics into `both-error` etc. to make it easier to see when they do and don't align? https://github.com/llvm/llvm-project/pull/70772 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang]get non-injected-class before finding instantiated decl (PR #70886)
https://github.com/hnrklssn commented: LGTM, but I'm not very familiar with the C++ specific parts of clang, so someone else ought to take a look also https://github.com/llvm/llvm-project/pull/70886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Recommit changes to global checks (PR #71171)
hnrklssn wrote: > I think if the issues with the original commit are resolved, this is good to > go. > > Did you verify we can properly auto-generate files, e.g., in > llvm/test/Transforms/Attributor and clang/test/OpenMP? > > Ah no I did not, I'll do that on Monday. https://github.com/llvm/llvm-project/pull/71171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Turn 'counted_by' into a type attribute and parse it into 'CountAttributedType' (PR #78000)
@@ -2000,6 +2001,21 @@ class alignas(TypeAlignment) Type : public
ExtQualsTypeCommonBase {
unsigned NumExpansions;
};
+ class CountAttributedTypeBitfields {
+friend class CountAttributedType;
+
+LLVM_PREFERRED_TYPE(TypeBitfields)
+unsigned : NumTypeBits;
+
+/// The limit is 15.
hnrklssn wrote:
Why is the limit 15? From what I can tell `NumTypeBits` is 22, so with
`CountInBytes` and `OrNull` that's 24 bits, which makes 15 seem very arbitrary.
How about a static assert that `sizeof(CountAttributedTypeBitfields)` is less
than or equal to some maximum, if that's the goal here.
https://github.com/llvm/llvm-project/pull/78000
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][InstrProf] Allow absolute path in fun.list of -fprofile-list= (PR #67519)
@@ -139,9 +139,23 @@ std::optional
ProfileList::isFileExcluded(StringRef FileName,
CodeGenOptions::ProfileInstrKind Kind) const {
StringRef Section = getSectionName(Kind);
- // Check for "source:="
+
+ // Convert the input file path to its canonical (absolute) form
+ llvm::SmallString<128> CanonicalFileName(FileName);
+ llvm::sys::fs::make_absolute(CanonicalFileName);
hnrklssn wrote:
> IIRC you mean first check the relative file name and then use the relative
> filename to get the absolute file name and then finally check for the
> absolute filename.
>
> Like this -
>
> ```
> diff --git a/clang/lib/Basic/ProfileList.cpp b/clang/lib/Basic/ProfileList.cpp
> index 1853a3e34ec3..a561db4e7e49 100644
> --- a/clang/lib/Basic/ProfileList.cpp
> +++ b/clang/lib/Basic/ProfileList.cpp
> @@ -139,21 +139,21 @@ std::optional
> ProfileList::isFileExcluded(StringRef FileName,
> CodeGenOptions::ProfileInstrKind Kind) const {
>StringRef Section = getSectionName(Kind);
> - // Convert the input file path to its canonical (absolute) form
> - llvm::SmallString<128> CanonicalFileName(FileName);
> - llvm::sys::fs::make_absolute(CanonicalFileName);
> -
>// Check for "source:="
>if (auto V = inSection(Section, "source", FileName))
> return V;
> - if (auto V = inSection(Section, "source", CanonicalFileName))
> -return V;
>if (SCL->inSection(Section, "!src", FileName))
> return Forbid;
> - if (SCL->inSection(Section, "!src", CanonicalFileName))
> -return Forbid;
>if (SCL->inSection(Section, "src", FileName))
> return Allow;
> +
> + // Convert the input file path to its canonical (absolute) form
> + llvm::SmallString<128> CanonicalFileName(FileName);
> + llvm::sys::fs::make_absolute(CanonicalFileName);
> + if (auto V = inSection(Section, "source", CanonicalFileName))
> +return V;
> + if (SCL->inSection(Section, "!src", CanonicalFileName))
> +return Forbid;
>if (SCL->inSection(Section, "src", CanonicalFileName))
> return Allow;
>return std::nullopt;
> ```
>
> For me both are fine, let me know if you think the same as I understand and
> want this version.
Yeah that's what I meant. I won't block on it though, because the performance
aspect probably doesn't matter too much. But please do add tests.
https://github.com/llvm/llvm-project/pull/67519
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [C23] Fix crash with _BitInt running clang-tidy (PR #65889)
@@ -1333,7 +1333,13 @@ void StmtProfiler::VisitPredefinedExpr(const
PredefinedExpr *S) {
void StmtProfiler::VisitIntegerLiteral(const IntegerLiteral *S) {
VisitExpr(S);
S->getValue().Profile(ID);
- ID.AddInteger(S->getType()->castAs()->getKind());
+
+ QualType T = S->getType();
+ ID.AddInteger(T->getTypeClass());
hnrklssn wrote:
This addition of the type class doesn't observe the `Canonical` attribute,
leading to IntegerLiterals with different types but same canonical types
mismatching even when `Canonical` is true.
https://github.com/llvm/llvm-project/pull/65889
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] [C23] Fix crash with _BitInt running clang-tidy (PR #65889)
@@ -1333,7 +1333,13 @@ void StmtProfiler::VisitPredefinedExpr(const
PredefinedExpr *S) {
void StmtProfiler::VisitIntegerLiteral(const IntegerLiteral *S) {
VisitExpr(S);
S->getValue().Profile(ID);
- ID.AddInteger(S->getType()->castAs()->getKind());
+
+ QualType T = S->getType();
+ ID.AddInteger(T->getTypeClass());
hnrklssn wrote:
Sure thing. I haven't been able to find a reproducer for upstream clang, so I
don't have a test case, but it doesn't seem to break any tests at least.
Waiting for it to push to github now.
https://github.com/llvm/llvm-project/pull/65889
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add missing canonicalization in int literal profile (PR #67822)
https://github.com/hnrklssn created
https://github.com/llvm/llvm-project/pull/67822
The addition of the type kind to the profile ID of IntegerLiterals results in
e.g. size_t and unsigned long literals mismatch even on platforms where they
are canonically the same type. This patch checks the Canonical field to
determine whether to canonicalize the type first.
rdar://116063468
>From 8ebdc7b829a3c2f59ef2773332511c81a40a8e73 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Fri, 29 Sep 2023 17:33:57 +0200
Subject: [PATCH] [clang] Add missing canonicalization in int literal profile
The addition of the type kind to the profile ID of IntegerLiterals
results in e.g. size_t and unsigned long literals mismatch even on
platforms where they are canonically the same type. This patch checks
the Canonical field to determine whether to canonicalize the type first.
rdar://116063468
---
clang/lib/AST/StmtProfile.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 2e4f15f83ac26ef..763d3d612698095 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -1335,6 +1335,8 @@ void StmtProfiler::VisitIntegerLiteral(const
IntegerLiteral *S) {
S->getValue().Profile(ID);
QualType T = S->getType();
+ if (Canonical)
+T = T.getCanonicalType();
ID.AddInteger(T->getTypeClass());
if (auto BitIntT = T->getAs())
BitIntT->Profile(ID);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add missing canonicalization in int literal profile (PR #67822)
https://github.com/hnrklssn closed https://github.com/llvm/llvm-project/pull/67822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][InstrProf] Allow absolute path in fun.list of -fprofile-list= (PR #67519)
https://github.com/hnrklssn edited https://github.com/llvm/llvm-project/pull/67519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][InstrProf] Allow absolute path in fun.list of -fprofile-list= (PR #67519)
https://github.com/hnrklssn requested changes to this pull request. Please update `clang/test/CodeGen/profile-filter.c` with some cases that exercise the new code path. https://github.com/llvm/llvm-project/pull/67519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][InstrProf] Allow absolute path in fun.list of -fprofile-list= (PR #67519)
@@ -139,9 +139,23 @@ std::optional
ProfileList::isFileExcluded(StringRef FileName,
CodeGenOptions::ProfileInstrKind Kind) const {
StringRef Section = getSectionName(Kind);
- // Check for "source:="
+
+ // Convert the input file path to its canonical (absolute) form
+ llvm::SmallString<128> CanonicalFileName(FileName);
+ llvm::sys::fs::make_absolute(CanonicalFileName);
hnrklssn wrote:
Any specific reason why you convert the filename to absolute form before
checking the relative name?
https://github.com/llvm/llvm-project/pull/67519
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 8a3fdf7 - [UTC] Add fallback support for specific metadata, and check their defs
Author: Henrik G. Olsson
Date: 2023-07-05T14:04:50+02:00
New Revision: 8a3fdf7b908978625e9a7e57fbb443e4e6f98976
URL:
https://github.com/llvm/llvm-project/commit/8a3fdf7b908978625e9a7e57fbb443e4e6f98976
DIFF:
https://github.com/llvm/llvm-project/commit/8a3fdf7b908978625e9a7e57fbb443e4e6f98976.diff
LOG: [UTC] Add fallback support for specific metadata, and check their defs
This prevents update_cc_tests.py from emitting hard-coded identifiers
for metadata (global variable checkers still check hard-coded
identifiers). Instead it emits regex checkers that match even if the
identifiers change. Also adds a new mode for --check-globals: instead of
simply being on or off, it now has the options 'none', 'smart' and
'all', with 'none' and 'all' corresponding to the previous modes.
The 'smart' mode only emits checks for global definitions referenced
in the IR or other metadata that itself has a definition checker
emitted, making the rule transitive. It does not emit checks for
attribute sets, since that is better checked by --check-attributes. This
mode is made the new default. To make the change in default mode
backwards compatible a version bump is introduced (to v3), and the
default remains 'none' in v1 & v2.
This will result in metadata checks being emitted more often, so filters
are added to not check absolute file paths and compiler version git
hashes.
rdar://105239218
Added:
clang/test/utils/update_cc_test_checks/Inputs/annotations.c
clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.generated.all.expected
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.no-generated.all.expected
clang/test/utils/update_cc_test_checks/annotations.test
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values.ll.funcsig.noglobals.expected
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values.ll.funcsig.transitiveglobals.expected
Modified:
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.generated.expected
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.no-generated.expected
clang/test/utils/update_cc_test_checks/check-globals.test
clang/test/utils/update_cc_test_checks/generated-funcs.test
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.generated.globals.expected
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.nogenerated.globals.expected
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.generated.globals.expected
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.nogenerated.globals.expected
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values.ll.funcsig.globals.expected
llvm/test/tools/UpdateTestChecks/update_test_checks/various_ir_values.test
llvm/utils/UpdateTestChecks/common.py
llvm/utils/update_cc_test_checks.py
llvm/utils/update_test_checks.py
Removed:
diff --git a/clang/test/utils/update_cc_test_checks/Inputs/annotations.c
b/clang/test/utils/update_cc_test_checks/Inputs/annotations.c
new file mode 100644
index 00..f09b0f788e9b12
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/Inputs/annotations.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fblocks
-ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s
+
+int foo() {
+int x = x + 1;
+return x;
+}
diff --git
a/clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected
b/clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected
new file mode 100644
index 00..b4eb9b0091900c
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected
@@ -0,0 +1,21 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
UTC_ARGS: --version 3
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fblocks
-ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s
+
+// CHECK-LABEL: define dso_local i32 @foo
+// CHECK-SAME: () #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:[[X:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i32 0, ptr [[X]], align 4, !annotation
[[META2:![0-9]+]]
+// CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[X]], align 4
+// CHECK-NEXT:[[ADD:%.*]] = add nsw i32 [[TMP0]], 1
+// CHECK-NEXT:store i32 [[ADD]], ptr [[X]], align 4
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4
+// CHECK-NEXT:ret i32 [[TMP1]]
+//
+int foo() {
+int x = x + 1;
+return x;
+}
+//.
+// CHECK: [[META2]] = !{!"auto-init"}
+//.
diff --git
a/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.generated.all.expected
b/clang/test/utils/update_cc_test_checks/In
[clang] 8fa2e93 - [clang] Do not merge traps in functions annotated optnone
Author: Henrik G. Olsson
Date: 2022-11-30T15:06:32+01:00
New Revision: 8fa2e93538595e1ff973110cb3f301b65bc9d2eb
URL:
https://github.com/llvm/llvm-project/commit/8fa2e93538595e1ff973110cb3f301b65bc9d2eb
DIFF:
https://github.com/llvm/llvm-project/commit/8fa2e93538595e1ff973110cb3f301b65bc9d2eb.diff
LOG: [clang] Do not merge traps in functions annotated optnone
This aligns the behaviour with that of disabling optimisations for the
translation unit entirely. Not merging the traps allows us to keep
separate debug information for each, improving the debugging experience
when finding the cause for a ubsan trap.
Differential Revision: https://reviews.llvm.org/D137714
Added:
Modified:
clang/lib/CodeGen/CGExpr.cpp
clang/test/CodeGen/ubsan-trap-debugloc.c
Removed:
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index d2d2ace66cee3..5021fc77e7b9b 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3550,7 +3550,8 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
TrapBBs.resize(CheckHandlerID + 1);
llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID];
- if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
+ if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB ||
+ (CurCodeDecl && CurCodeDecl->hasAttr())) {
TrapBB = createBasicBlock("trap");
Builder.CreateCondBr(Checked, Cont, TrapBB);
EmitBlock(TrapBB);
diff --git a/clang/test/CodeGen/ubsan-trap-debugloc.c
b/clang/test/CodeGen/ubsan-trap-debugloc.c
index d73cf73fc7c39..4cad708b0ca50 100644
--- a/clang/test/CodeGen/ubsan-trap-debugloc.c
+++ b/clang/test/CodeGen/ubsan-trap-debugloc.c
@@ -2,9 +2,23 @@
void foo(volatile int a) {
+ // CHECK-LABEL: @foo
// CHECK: call void @llvm.ubsantrap(i8 0){{.*}} !dbg [[LOC:![0-9]+]]
a = a + 1;
a = a + 1;
}
+void bar(volatile int a) __attribute__((optnone)) {
+ // CHECK-LABEL: @bar
+ // CHECK: call void @llvm.ubsantrap(i8 0){{.*}} !dbg [[LOC2:![0-9]+]]
+ // CHECK: call void @llvm.ubsantrap(i8 0){{.*}} !dbg [[LOC3:![0-9]+]]
+ a = a + 1;
+ a = a + 1;
+}
+
+// With optimisations enabled the traps are merged and need to share a debug
location
// CHECK: [[LOC]] = !DILocation(line: 0
+
+// With optimisations disabled the traps are not merged and retain accurate
debug locations
+// CHECK: [[LOC2]] = !DILocation(line: 15, column: 9
+// CHECK: [[LOC3]] = !DILocation(line: 16, column: 9
___
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)
hnrklssn wrote: @delcypher @rapidsna updated __sized_by(_or_null) to only allow types with unknown size if incomplete 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] [UVT] add update-verify-tests.py (PR #97369)
https://github.com/hnrklssn created
https://github.com/llvm/llvm-project/pull/97369
Adds a python script to automatically take output from a failed clang -verify
test and update the test case(s) to expect the new behaviour.
>From bc2f0757cfa08a8f26b9934929a0045d5e0ffd93 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Mon, 1 Jul 2024 18:19:09 -0700
Subject: [PATCH] [UVT] add update-verify-tests.py
Adds a python script to automatically take output from a failed clang
-verify test and update the test case(s) to expect the new behaviour.
---
clang/utils/update-verify-tests.py | 321 +
1 file changed, 321 insertions(+)
create mode 100644 clang/utils/update-verify-tests.py
diff --git a/clang/utils/update-verify-tests.py
b/clang/utils/update-verify-tests.py
new file mode 100644
index 0..95eca3af61a72
--- /dev/null
+++ b/clang/utils/update-verify-tests.py
@@ -0,0 +1,321 @@
+import sys
+import re
+
+"""
+ Pipe output from clang's -verify into this script to have the test case
updated to expect the actual diagnostic output.
+ When inserting new expected-* checks it will place them on the line before
the location of the diagnostic, with an @+1,
+ or @+N for some N if there are multiple diagnostics emitted on the same line.
If the current checks are using @-N for
+ this line, the new check will follow that convention also.
+ Existing checks will be left untouched as much as possible, including their
location and whitespace content, to minimize
+ diffs. If inaccurate their count will be updated, or the check removed
entirely.
+
+ Missing features:
+ - custom prefix support (-verify=my-prefix)
+ - multiple prefixes on the same line (-verify=my-prefix,my-other-prefix)
+ - multiple prefixes on separate RUN lines (RUN: -verify=my-prefix\nRUN:
-verify my-other-prefix)
+ - regexes with expected-*-re: existing ones will be left untouched if
accurate, but the script will abort if there are any
+diagnostic mismatches on the same line.
+ - multiple checks targeting the same line are supported, but a line may only
contain one check
+ - if multiple checks targeting the same line are failing the script is not
guaranteed to produce a minimal diff
+
+Example usage:
+ build/bin/llvm-lit clang/test/Sema/ --no-progress-bar -v | python3
update-verify-tests.py
+"""
+
+class KnownException(Exception):
+pass
+
+def parse_error_category(s):
+parts = s.split("diagnostics")
+diag_category = parts[0]
+category_parts = parts[0].strip().strip("'").split("-")
+expected = category_parts[0]
+if expected != "expected":
+raise Exception(f"expected 'expected', but found '{expected}'. Custom
verify prefixes are not supported.")
+diag_category = category_parts[1]
+if "seen but not expected" in parts[1]:
+seen = True
+elif "expected but not seen" in parts[1]:
+seen = False
+else:
+raise KnownException(f"unexpected category '{parts[1]}'")
+return (diag_category, seen)
+
+diag_error_re = re.compile(r"File (\S+) Line (\d+): (.+)")
+diag_error_re2 = re.compile(r"File \S+ Line \d+ \(directive at (\S+):(\d+)\):
(.+)")
+def parse_diag_error(s):
+m = diag_error_re2.match(s)
+if not m:
+m = diag_error_re.match(s)
+if not m:
+return None
+return (m.group(1), int(m.group(2)), m.group(3))
+
+class Line:
+def __init__(self, content, line_n):
+self.content = content
+self.diag = None
+self.line_n = line_n
+self.related_diags = []
+self.targeting_diags = []
+def update_line_n(self, n):
+if self.diag and not self.diag.line_is_absolute:
+self.diag.orig_target_line_n += n - self.line_n
+self.line_n = n
+for diag in self.targeting_diags:
+if diag.line_is_absolute:
+diag.orig_target_line_n = n
+else:
+diag.orig_target_line_n = n - diag.line.line_n
+for diag in self.related_diags:
+if not diag.line_is_absolute:
+pass
+def render(self):
+if not self.diag:
+return self.content
+assert("{{DIAG}}" in self.content)
+res = self.content.replace("{{DIAG}}", self.diag.render())
+if not res.strip():
+return ""
+return res
+
+class Diag:
+def __init__(self, diag_content, category, targeted_line_n,
line_is_absolute, count, line, is_re, whitespace_strings):
+self.diag_content = diag_content
+self.category = category
+self.orig_target_line_n = targeted_line_n
+self.line_is_absolute = line_is_absolute
+self.count = count
+self.line = line
+self.target = None
+self.is_re = is_re
+self.absolute_target()
+self.whitespace_strings = whitespace_strings
+
+def add(self):
+if targeted_line > 0:
+targeted_line += 1
+elif targeted_line < 0:
+targe
[clang] [UVT] add update-verify-tests.py (PR #97369)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/97369
>From bc2f0757cfa08a8f26b9934929a0045d5e0ffd93 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Mon, 1 Jul 2024 18:19:09 -0700
Subject: [PATCH 1/2] [UVT] add update-verify-tests.py
Adds a python script to automatically take output from a failed clang
-verify test and update the test case(s) to expect the new behaviour.
---
clang/utils/update-verify-tests.py | 321 +
1 file changed, 321 insertions(+)
create mode 100644 clang/utils/update-verify-tests.py
diff --git a/clang/utils/update-verify-tests.py
b/clang/utils/update-verify-tests.py
new file mode 100644
index 0..95eca3af61a72
--- /dev/null
+++ b/clang/utils/update-verify-tests.py
@@ -0,0 +1,321 @@
+import sys
+import re
+
+"""
+ Pipe output from clang's -verify into this script to have the test case
updated to expect the actual diagnostic output.
+ When inserting new expected-* checks it will place them on the line before
the location of the diagnostic, with an @+1,
+ or @+N for some N if there are multiple diagnostics emitted on the same line.
If the current checks are using @-N for
+ this line, the new check will follow that convention also.
+ Existing checks will be left untouched as much as possible, including their
location and whitespace content, to minimize
+ diffs. If inaccurate their count will be updated, or the check removed
entirely.
+
+ Missing features:
+ - custom prefix support (-verify=my-prefix)
+ - multiple prefixes on the same line (-verify=my-prefix,my-other-prefix)
+ - multiple prefixes on separate RUN lines (RUN: -verify=my-prefix\nRUN:
-verify my-other-prefix)
+ - regexes with expected-*-re: existing ones will be left untouched if
accurate, but the script will abort if there are any
+diagnostic mismatches on the same line.
+ - multiple checks targeting the same line are supported, but a line may only
contain one check
+ - if multiple checks targeting the same line are failing the script is not
guaranteed to produce a minimal diff
+
+Example usage:
+ build/bin/llvm-lit clang/test/Sema/ --no-progress-bar -v | python3
update-verify-tests.py
+"""
+
+class KnownException(Exception):
+pass
+
+def parse_error_category(s):
+parts = s.split("diagnostics")
+diag_category = parts[0]
+category_parts = parts[0].strip().strip("'").split("-")
+expected = category_parts[0]
+if expected != "expected":
+raise Exception(f"expected 'expected', but found '{expected}'. Custom
verify prefixes are not supported.")
+diag_category = category_parts[1]
+if "seen but not expected" in parts[1]:
+seen = True
+elif "expected but not seen" in parts[1]:
+seen = False
+else:
+raise KnownException(f"unexpected category '{parts[1]}'")
+return (diag_category, seen)
+
+diag_error_re = re.compile(r"File (\S+) Line (\d+): (.+)")
+diag_error_re2 = re.compile(r"File \S+ Line \d+ \(directive at (\S+):(\d+)\):
(.+)")
+def parse_diag_error(s):
+m = diag_error_re2.match(s)
+if not m:
+m = diag_error_re.match(s)
+if not m:
+return None
+return (m.group(1), int(m.group(2)), m.group(3))
+
+class Line:
+def __init__(self, content, line_n):
+self.content = content
+self.diag = None
+self.line_n = line_n
+self.related_diags = []
+self.targeting_diags = []
+def update_line_n(self, n):
+if self.diag and not self.diag.line_is_absolute:
+self.diag.orig_target_line_n += n - self.line_n
+self.line_n = n
+for diag in self.targeting_diags:
+if diag.line_is_absolute:
+diag.orig_target_line_n = n
+else:
+diag.orig_target_line_n = n - diag.line.line_n
+for diag in self.related_diags:
+if not diag.line_is_absolute:
+pass
+def render(self):
+if not self.diag:
+return self.content
+assert("{{DIAG}}" in self.content)
+res = self.content.replace("{{DIAG}}", self.diag.render())
+if not res.strip():
+return ""
+return res
+
+class Diag:
+def __init__(self, diag_content, category, targeted_line_n,
line_is_absolute, count, line, is_re, whitespace_strings):
+self.diag_content = diag_content
+self.category = category
+self.orig_target_line_n = targeted_line_n
+self.line_is_absolute = line_is_absolute
+self.count = count
+self.line = line
+self.target = None
+self.is_re = is_re
+self.absolute_target()
+self.whitespace_strings = whitespace_strings
+
+def add(self):
+if targeted_line > 0:
+targeted_line += 1
+elif targeted_line < 0:
+targeted_line -= 1
+
+def absolute_target(self):
+if self.line_is_absolute:
+res = self.orig_target_line_n
+els
[clang] [UVT] add update-verify-tests.py (PR #97369)
hnrklssn wrote: > This is a nice addition, but I think it should follow the conventions > established by the existing update_*_test_checks.py scripts as much as > possible, at least: > > * Ability to parse RUN lines, re-execute them autonomously, and modify > test files in place based on their output > > * Proper argument parsing via the argparse library > > * Name of the script itself > > > You should integrate with the UpdateTestChecks/common.py infrastructure as > much as possible. At a minimum, the collection of test cases to update, the > RUN line parsing, and stuff like `common.invoke_tool` should almost certainly > be shared. This may require some minor updates to that common infrastructure, > but that's a good thing. I did consider this at first - here's my reasoning why I didn't, please let me know what you think: I think that the added complexity of RUN line parsing + `common.invoke_tool` would cost more than it's worth for this tool. It's already a reimplementation of something that `lit` does excellently. `lit` also has automatic test discovery, instead of having to manually provide each test case to the script. Any test case relying on RUN lines is going to be tested with `lit` anyways, so it's not like we're adding any extra coupling. The `update_*_test_checks.py` scripts use `common.invoke_tool` to prevent the output from being piped into `FileCheck`, but this script uses the raw output of the `-verify` flag (which is rarely if ever piped into anything), so invoking the lines manually doesn't provide the same extra value. If anything I think `lit` should have a plugin system that allows a plugin to intercept the test invocations and control how RUN lines are executed. I didn't implement that now though, because I didn't want to expand the scope of this patch too much. > The actual generating and updating of CHECK lines is probably sufficiently > different that sharing might not make sense. That said, I wonder if this > script can be generalized to automatically generate CHECK lines for other > tools that produce diagnostic output. This script relies on the format of the output of the `-verify` flag to get the ground truth of which diagnostics are mismatching, and how. FileCheck doesn't provide as comprehensive information because it's much more flexible, so it'd be harder for it to confidently say "this check doesn't match anything and this check should be inserted on this line". E.g. this script doesn't even try to handle checks with regexes, while any FileCheck check can contain a regex unless they specify `LITERAL`. For the checks themselves I think they are sufficiently different from FileCheck checks that it'd be hard to generalise the output: multiple `expected-*` checks can point to the same line, and at the same time we have no information about the order the diagnostics are actually emitted to the use, e.g. a later diagnostic can point to an earlier line. https://github.com/llvm/llvm-project/pull/97369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] add --update-tests flag to llvm-lit (PR #97369)
https://github.com/hnrklssn edited https://github.com/llvm/llvm-project/pull/97369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] add --update-tests flag to llvm-lit (PR #97369)
hnrklssn wrote: Added tests and updated the script. Instead of only being a free-standing script it's now (along with the UTC scripts) integrated into llvm-lit to automatically detect which script can update a failing test, using the new flag `--update-tests`. https://github.com/llvm/llvm-project/pull/97369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] add --update-tests flag to llvm-lit (PR #97369)
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: diff %s %s.expected
hnrklssn wrote:
The files in the `Inputs` directory are not discovered by lit, so this doesn't
actually run the RUN lines. Instead a copy is made, and a new lit instance is
launched to test the run lines.
This is in `duplicate-diag.test`, the RUN lines of `duplicate-diag.c` are not
executed at all for this case because it tests the `update-verify-tests.py`
script:
```
# RUN: cp %S/Inputs/duplicate-diag.c %t.c && not %clang_cc1 -verify %t.c 2>&1 |
%update-verify-tests
# RUN: diff -u %S/Inputs/duplicate-diag.c.expected %t.c
# RUN: %clang_cc1 -verify %t.c
```
This is in `lit-plugin.test`, which copies the files to a separate directory
and launches llvm-lit there to test the lit integration:
```
# RUN: cp %S/Inputs/*.c %S/LitTests
# RUN: cp %S/Inputs/*.c.expected %S/LitTests
# RUN: not %{lit} %S/LitTests
# RUN: not %{lit} %S/LitTests --update-tests
# RUN: %{lit} %S/LitTests
```
https://github.com/llvm/llvm-project/pull/97369
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] add --update-tests flag to llvm-lit (PR #97369)
hnrklssn wrote: > I think the integration into lit is okay. The diff updater seems neat, but I > think it could be taken into a separate change, see also the inline comment. > In general, I think this PR really does three separate things, that ought to > be in separate changes: > > * Add update-verify-tests.py > > * Add a way to update tests from within llvm-lit > > * Add the diff-test-updater to llvm-lit Makes sense, I'll separate the changes into 3 PRs. https://github.com/llvm/llvm-project/pull/97369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
@@ -3330,6 +3340,118 @@ void Parser::DistributeCLateParsedAttrs(Decl *Dcl,
}
}
+/// GuardedBy attributes (e.g., guarded_by):
+/// AttrName '(' expression ')'
+void Parser::ParseGuardedByAttribute(
+IdentifierInfo &AttrName, SourceLocation AttrNameLoc,
+ParsedAttributes &Attrs, IdentifierInfo *ScopeName, SourceLocation
ScopeLoc,
+SourceLocation *EndLoc, ParsedAttr::Form Form) {
+ assert(Tok.is(tok::l_paren) && "Attribute arg list not starting with '('");
+
+ BalancedDelimiterTracker Parens(*this, tok::l_paren);
+ Parens.consumeOpen();
+
+ if (Tok.is(tok::r_paren)) {
+Diag(Tok.getLocation(), diag::err_argument_required_after_attribute);
+Parens.consumeClose();
+return;
+ }
+
+ ArgsVector ArgExprs;
+ // Don't evaluate argument when the attribute is ignored.
+ using ExpressionKind =
+ Sema::ExpressionEvaluationContextRecord::ExpressionKind;
+ EnterExpressionEvaluationContext EC(
+ Actions,
+ getLangOpts().CPlusPlus
+ ? Sema::ExpressionEvaluationContext::Unevaluated
+ : Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
hnrklssn wrote:
Why does C need to parse using `PotentiallyEvaluated` while C++ needs
`Unevaluated`? What happens if we parse using `Unevaluted` in C?
https://github.com/llvm/llvm-project/pull/95455
___
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/hnrklssn updated https://github.com/llvm/llvm-project/pull/93231 >From 5c5a28415f2cc10525f07784e6896718cc38624f Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Thu, 23 May 2024 11:44:41 -0700 Subject: [PATCH] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null The attributes `sized_by`, `counted_by_or_null` and `sized_by_or_null` have been added as variants on `counted_by`, each with slightly different semantics. `sized_by` takes a byte size parameter instead of an element count, allowing pointees with unknown size. The `counted_by_or_null` and `sized_by_or_null` variants are equivalent to their base variants, except the pointer can be null regardless of count/size value. If the pointer is null the size is effectively 0. rdar://125400354 --- clang/docs/ReleaseNotes.rst | 6 + clang/include/clang/Basic/Attr.td | 30 +++ .../clang/Basic/DiagnosticSemaKinds.td| 16 +- clang/include/clang/Sema/Sema.h | 4 +- clang/lib/AST/TypePrinter.cpp | 3 + clang/lib/Parse/ParseDecl.cpp | 7 +- clang/lib/Sema/SemaDeclAttr.cpp | 70 +++-- clang/lib/Sema/SemaType.cpp | 8 +- clang/lib/Sema/TreeTransform.h| 3 +- ...unted-by-or-null-late-parsed-struct-ptrs.c | 45 .../AST/attr-counted-by-or-null-struct-ptrs.c | 117 .../attr-sized-by-late-parsed-struct-ptrs.c | 45 ...sized-by-or-null-late-parsed-struct-ptrs.c | 45 .../AST/attr-sized-by-or-null-struct-ptrs.c | 117 clang/test/AST/attr-sized-by-struct-ptrs.c| 117 .../attr-counted-by-or-null-late-parsed-off.c | 26 ++ ...unted-by-or-null-late-parsed-struct-ptrs.c | 255 ++ ...ed-by-or-null-struct-ptrs-sizeless-types.c | 17 ++ .../attr-counted-by-or-null-struct-ptrs.c | 225 ...tr-counted-by-or-null-vla-sizeless-types.c | 11 + .../test/Sema/attr-sized-by-late-parsed-off.c | 26 ++ .../attr-sized-by-late-parsed-struct-ptrs.c | 243 + .../attr-sized-by-or-null-late-parsed-off.c | 26 ++ ...sized-by-or-null-late-parsed-struct-ptrs.c | 243 + ...ed-by-or-null-struct-ptrs-sizeless-types.c | 16 ++ .../Sema/attr-sized-by-or-null-struct-ptrs.c | 211 +++ ...attr-sized-by-or-null-vla-sizeless-types.c | 11 + ...attr-sized-by-struct-ptrs-sizeless-types.c | 16 ++ clang/test/Sema/attr-sized-by-struct-ptrs.c | 211 +++ .../Sema/attr-sized-by-vla-sizeless-types.c | 11 + 30 files changed, 2150 insertions(+), 31 deletions(-) create mode 100644 clang/test/AST/attr-counted-by-or-null-late-parsed-struct-ptrs.c create mode 100644 clang/test/AST/attr-counted-by-or-null-struct-ptrs.c create mode 100644 clang/test/AST/attr-sized-by-late-parsed-struct-ptrs.c create mode 100644 clang/test/AST/attr-sized-by-or-null-late-parsed-struct-ptrs.c create mode 100644 clang/test/AST/attr-sized-by-or-null-struct-ptrs.c create mode 100644 clang/test/AST/attr-sized-by-struct-ptrs.c create mode 100644 clang/test/Sema/attr-counted-by-or-null-late-parsed-off.c create mode 100644 clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c create mode 100644 clang/test/Sema/attr-counted-by-or-null-struct-ptrs-sizeless-types.c create mode 100644 clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c create mode 100644 clang/test/Sema/attr-counted-by-or-null-vla-sizeless-types.c create mode 100644 clang/test/Sema/attr-sized-by-late-parsed-off.c create mode 100644 clang/test/Sema/attr-sized-by-late-parsed-struct-ptrs.c create mode 100644 clang/test/Sema/attr-sized-by-or-null-late-parsed-off.c create mode 100644 clang/test/Sema/attr-sized-by-or-null-late-parsed-struct-ptrs.c create mode 100644 clang/test/Sema/attr-sized-by-or-null-struct-ptrs-sizeless-types.c create mode 100644 clang/test/Sema/attr-sized-by-or-null-struct-ptrs.c create mode 100644 clang/test/Sema/attr-sized-by-or-null-vla-sizeless-types.c create mode 100644 clang/test/Sema/attr-sized-by-struct-ptrs-sizeless-types.c create mode 100644 clang/test/Sema/attr-sized-by-struct-ptrs.c create mode 100644 clang/test/Sema/attr-sized-by-vla-sizeless-types.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 22b4dc172c840..d1e414c99029b 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -451,6 +451,12 @@ Attribute Changes in Clang size_t count; }; +- The attributes ``sized_by``, ``counted_by_or_null`` and ``sized_by_or_null``` + have been added as variants on ``counted_by``, each with slightly different semantics. + ``sized_by`` takes a byte size parameter instead of an element count, allowing pointees + with unknown size. The ``counted_by_or_null`` and ``sized_by_or_null`` variants are equivalent + to their base variants, except the pointer can be null regardless of count/size value. + If the poin
[clang] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)
hnrklssn wrote: Rebased on main after @delcypher's patch was relanded. Addressing comments now. 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] [Bounds Safety][NFC] Add `SemaBoundsSafety` class and move existing Sema checks there (PR #98954)
https://github.com/hnrklssn approved this pull request. LGTM, might want to give the rest some time to have a look though https://github.com/llvm/llvm-project/pull/98954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Bounds Safety][NFC] Add `SemaBoundsSafety` class and move existing Sema checks there (PR #98954)
hnrklssn wrote: > If you take a close look at the existing set of `Sema` parts, it's almost > entirely comprised of other languages or language extensions (from C and C++ > standpoint) and backends. They were considered natural enough to be split off > `Sema`, which, among other things, means that it's easy to explain their > place in the big picture, like I did in the previous sentence. I don't have a strong opinion on whether this should be split up, but I just wanted to point out that `-fbounds-safety` is also a language extension, so it does kind of fit your description of other things separated out. It's currently in the process of being upstreamed which is why it's small, but it'd be easier to split now than wait until it's reached a certain size, if we do want to split it eventually. https://github.com/llvm/llvm-project/pull/98954 ___ 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)
@@ -8320,6 +8320,15 @@ static const RecordDecl
*GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD) {
return RD;
}
+static CountAttributedType::DynamicCountPointerKind
+getCountAttrKind(bool CountInBytes, bool OrNull) {
hnrklssn wrote:
I guess it could be a static method. It can't be an instance method since it's
used before the object is constructed. Since it's only used in one place I
thought it'd improve readability to keep it close to the call site.
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] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)
https://github.com/hnrklssn closed 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] Allow 'counted_by' attribute on pointers in structs in C (PR #90786)
@@ -8588,31 +8588,71 @@ static const RecordDecl
*GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD) {
return RD;
}
-static bool
-CheckCountExpr(Sema &S, FieldDecl *FD, Expr *E,
- llvm::SmallVectorImpl &Decls) {
+enum class CountedByInvalidPointeeTypeKind {
+ INCOMPLETE,
+ SIZELESS,
+ FUNCTION,
+ FLEXIBLE_ARRAY_MEMBER,
+ VALID,
+};
+
+static bool CheckCountedByAttrOnField(
+Sema &S, FieldDecl *FD, Expr *E,
+llvm::SmallVectorImpl &Decls) {
+ // Check the context the attribute is used in
+
if (FD->getParent()->isUnion()) {
S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_in_union)
<< FD->getSourceRange();
return true;
}
- if (!E->getType()->isIntegerType() || E->getType()->isBooleanType()) {
-S.Diag(E->getBeginLoc(), diag::err_counted_by_attr_argument_not_integer)
-<< E->getSourceRange();
+ const auto FieldTy = FD->getType();
+ if (!FieldTy->isArrayType() && !FieldTy->isPointerType()) {
+S.Diag(FD->getBeginLoc(),
+ diag::err_counted_by_attr_not_on_ptr_or_flexible_array_member)
+<< FD->getLocation();
return true;
}
LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
LangOptions::StrictFlexArraysLevelKind::IncompleteOnly;
-
- if (!Decl::isFlexibleArrayMemberLike(S.getASTContext(), FD, FD->getType(),
+ if (FieldTy->isArrayType() &&
+ !Decl::isFlexibleArrayMemberLike(S.getASTContext(), FD, FieldTy,
StrictFlexArraysLevel, true)) {
-// The "counted_by" attribute must be on a flexible array member.
-SourceRange SR = FD->getLocation();
-S.Diag(SR.getBegin(),
- diag::err_counted_by_attr_not_on_flexible_array_member)
-<< SR;
+S.Diag(FD->getBeginLoc(),
+ diag::err_counted_by_attr_on_array_not_flexible_array_member)
+<< FD->getLocation();
+return true;
+ }
+
+ if (FieldTy->isPointerType()) {
hnrklssn wrote:
Does this need to be limited to pointers? Seems to me that it could apply to
arrays also.
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)
@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
}
}
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList &LAs, bool EnterScope,
+ ParsedAttributes *OutAttrs) {
+ assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+ auto LangStd = getLangOpts().LangStd;
+ if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);
hnrklssn wrote:
Do we have any tests triggering the OpenCL path?
https://github.com/llvm/llvm-project/pull/93121
___
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/hnrklssn created https://github.com/llvm/llvm-project/pull/93231 The attributes `sized_by`, `counted_by_or_null` and `sized_by_or_null` have been added as variants on `counted_by`, each with slightly different semantics. `sized_by` takes a byte size parameter instead of an element count, allowing pointees with unknown size. The `counted_by_or_null` and `sized_by_or_null` variants are equivalent to their base variants, except the pointer can be null regardless of count/size value. If the pointer is null the size is effectively 0. rdar://125400354 >From fa82a978536906d330fb0fa6c67f2589f76df5e1 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Thu, 23 May 2024 11:44:41 -0700 Subject: [PATCH] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null The attributes `sized_by`, `counted_by_or_null` and `sized_by_or_null` have been added as variants on `counted_by`, each with slightly different semantics. `sized_by` takes a byte size parameter instead of an element count, allowing pointees with unknown size. The `counted_by_or_null` and `sized_by_or_null` variants are equivalent to their base variants, except the pointer can be null regardless of count/size value. If the pointer is null the size is effectively 0. rdar://125400354 --- clang/docs/ReleaseNotes.rst | 6 + clang/include/clang/Basic/Attr.td | 30 +++ .../clang/Basic/DiagnosticSemaKinds.td| 16 +- clang/include/clang/Sema/Sema.h | 4 +- clang/lib/AST/TypePrinter.cpp | 3 + clang/lib/Parse/ParseDecl.cpp | 10 +- clang/lib/Sema/SemaDeclAttr.cpp | 70 +++-- clang/lib/Sema/SemaType.cpp | 8 +- clang/lib/Sema/TreeTransform.h| 3 +- ...unted-by-or-null-late-parsed-struct-ptrs.c | 45 .../AST/attr-counted-by-or-null-struct-ptrs.c | 117 .../attr-sized-by-late-parsed-struct-ptrs.c | 45 ...sized-by-or-null-late-parsed-struct-ptrs.c | 45 .../AST/attr-sized-by-or-null-struct-ptrs.c | 117 clang/test/AST/attr-sized-by-struct-ptrs.c| 117 .../attr-counted-by-or-null-late-parsed-off.c | 26 ++ ...unted-by-or-null-late-parsed-struct-ptrs.c | 255 ++ ...ed-by-or-null-struct-ptrs-sizeless-types.c | 17 ++ .../attr-counted-by-or-null-struct-ptrs.c | 225 ...tr-counted-by-or-null-vla-sizeless-types.c | 11 + .../test/Sema/attr-sized-by-late-parsed-off.c | 26 ++ .../attr-sized-by-late-parsed-struct-ptrs.c | 243 + .../attr-sized-by-or-null-late-parsed-off.c | 26 ++ ...sized-by-or-null-late-parsed-struct-ptrs.c | 243 + ...ed-by-or-null-struct-ptrs-sizeless-types.c | 16 ++ .../Sema/attr-sized-by-or-null-struct-ptrs.c | 211 +++ ...attr-sized-by-or-null-vla-sizeless-types.c | 11 + ...attr-sized-by-struct-ptrs-sizeless-types.c | 16 ++ clang/test/Sema/attr-sized-by-struct-ptrs.c | 211 +++ .../Sema/attr-sized-by-vla-sizeless-types.c | 11 + 30 files changed, 2153 insertions(+), 31 deletions(-) create mode 100644 clang/test/AST/attr-counted-by-or-null-late-parsed-struct-ptrs.c create mode 100644 clang/test/AST/attr-counted-by-or-null-struct-ptrs.c create mode 100644 clang/test/AST/attr-sized-by-late-parsed-struct-ptrs.c create mode 100644 clang/test/AST/attr-sized-by-or-null-late-parsed-struct-ptrs.c create mode 100644 clang/test/AST/attr-sized-by-or-null-struct-ptrs.c create mode 100644 clang/test/AST/attr-sized-by-struct-ptrs.c create mode 100644 clang/test/Sema/attr-counted-by-or-null-late-parsed-off.c create mode 100644 clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c create mode 100644 clang/test/Sema/attr-counted-by-or-null-struct-ptrs-sizeless-types.c create mode 100644 clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c create mode 100644 clang/test/Sema/attr-counted-by-or-null-vla-sizeless-types.c create mode 100644 clang/test/Sema/attr-sized-by-late-parsed-off.c create mode 100644 clang/test/Sema/attr-sized-by-late-parsed-struct-ptrs.c create mode 100644 clang/test/Sema/attr-sized-by-or-null-late-parsed-off.c create mode 100644 clang/test/Sema/attr-sized-by-or-null-late-parsed-struct-ptrs.c create mode 100644 clang/test/Sema/attr-sized-by-or-null-struct-ptrs-sizeless-types.c create mode 100644 clang/test/Sema/attr-sized-by-or-null-struct-ptrs.c create mode 100644 clang/test/Sema/attr-sized-by-or-null-vla-sizeless-types.c create mode 100644 clang/test/Sema/attr-sized-by-struct-ptrs-sizeless-types.c create mode 100644 clang/test/Sema/attr-sized-by-struct-ptrs.c create mode 100644 clang/test/Sema/attr-sized-by-vla-sizeless-types.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 2f83f5c6d54e9..841ef07f2e99b 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -425,6 +425,12 @@ Attribute Changes in Clang
[clang] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)
hnrklssn wrote: Blocked by pointer support for `counted_by` reverted. It's being relanded in #93121 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] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)
@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
}
}
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList &LAs, bool EnterScope,
+ ParsedAttributes *OutAttrs) {
+ assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+ auto LangStd = getLangOpts().LangStd;
+ if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);
hnrklssn wrote:
> In general I think the approach of thinking that this function should be for
> C only is actually flawed and I'm going to look into removing this entirely
> after landing this.
At that point, why not just call `ParseLexedAttributeList` and add a non-C++
case to `ParseLexedAttribute` instead of creating separate C versions that seem
to serve basically the same purpose?
https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)
@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
}
}
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList &LAs, bool EnterScope,
+ ParsedAttributes *OutAttrs) {
+ assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+ auto LangStd = getLangOpts().LangStd;
+ if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);
hnrklssn wrote:
> However, I think I'll remove this assert for now because I intend (provided
> I can make it to work with our internal code) to remove this code entirely in
> favor of re-using the late parsing code that's used for C++
Ah so that's the same thing I suggested then. I thought you meant to just
remove the assert. Overall I think this is a good idea 👍
https://github.com/llvm/llvm-project/pull/93121
___
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)
@@ -8697,9 +8708,10 @@ static bool CheckCountedByAttrOnField(
InvalidTypeKind = CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER;
}
- if (InvalidTypeKind != CountedByInvalidPointeeTypeKind::VALID) {
+ if (InvalidTypeKind != CountedByInvalidPointeeTypeKind::VALID &&
+ !CountInBytes) {
hnrklssn wrote:
I think that is a reasonable restriction to add in `-fbounds-safety`, but for
simply saying "this is the size", is it really an issue? It's also feasible to
allow the attribute, but not allow indexing into an unsized type. That would
make it possible to cast to `char * sized_by(sz)` and retain the size bounds
before indexing into it.
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] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)
@@ -425,6 +425,12 @@ Attribute Changes in Clang size_t count; }; +- The attributes ``sized_by``, ``counted_by_or_null`` and ``sized_by_or_null``` + have been added as variants on ``counted_by``, each with slightly different semantics. + ``sized_by`` takes a byte size parameter instead of an element count, allowing pointees + with unknown size. The ``counted_by_or_null`` and ``sized_by_or_null`` variants are equivalent + to their base variants, except the pointer can be null regardless of count/size value. hnrklssn wrote: Hmm yeah there's nothing preventing you from assigning null I guess, but I don't believe `__bdos` CG has any support for returning 0 when the pointer is null. 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] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)
@@ -8641,22 +8641,33 @@ enum class CountedByInvalidPointeeTypeKind {
VALID,
};
-static bool CheckCountedByAttrOnField(
-Sema &S, FieldDecl *FD, Expr *E,
-llvm::SmallVectorImpl &Decls) {
+static bool
+CheckCountedByAttrOnField(Sema &S, FieldDecl *FD, Expr *E,
+ llvm::SmallVectorImpl &Decls,
+ bool CountInBytes, bool OrNull) {
// Check the context the attribute is used in
+ unsigned Kind = CountInBytes;
+ if (OrNull)
+Kind += 2;
+
if (FD->getParent()->isUnion()) {
S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_in_union)
-<< FD->getSourceRange();
+<< Kind << FD->getSourceRange();
return true;
}
const auto FieldTy = FD->getType();
+ if (FieldTy->isArrayType() && (CountInBytes || OrNull)) {
+S.Diag(FD->getBeginLoc(),
+ diag::err_counted_by_attr_not_on_ptr_or_flexible_array_member)
hnrklssn wrote:
> The OrNull case probably deserves its own special diagnostic because in that
> case the diagnostic should explain that they cannot use the attribute on
> arrays and that they need to use __counted_by instead.
That is also true for `CountInBytes`. But I agree that it would be good for the
diagnostic to suggest `counted_by`.
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] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)
@@ -8641,22 +8641,33 @@ enum class CountedByInvalidPointeeTypeKind {
VALID,
};
-static bool CheckCountedByAttrOnField(
-Sema &S, FieldDecl *FD, Expr *E,
-llvm::SmallVectorImpl &Decls) {
+static bool
+CheckCountedByAttrOnField(Sema &S, FieldDecl *FD, Expr *E,
+ llvm::SmallVectorImpl &Decls,
+ bool CountInBytes, bool OrNull) {
// Check the context the attribute is used in
+ unsigned Kind = CountInBytes;
+ if (OrNull)
+Kind += 2;
+
if (FD->getParent()->isUnion()) {
S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_in_union)
-<< FD->getSourceRange();
+<< Kind << FD->getSourceRange();
return true;
}
const auto FieldTy = FD->getType();
+ if (FieldTy->isArrayType() && (CountInBytes || OrNull)) {
+S.Diag(FD->getBeginLoc(),
+ diag::err_counted_by_attr_not_on_ptr_or_flexible_array_member)
hnrklssn wrote:
>The diagnostic name is a little misleading here because CountInBytes suggested
>__sized_by but the diagnostic name has counted_by in its name
I kept it because it's the same family of attributes. Do you have a suggestion
for a name that would imply that it's not just for `counted_by`, but more
specific than `bounds_attribute`?
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] [Clang] Add wraps attribute (for granular integer overflow handling) (PR #86618)
hnrklssn wrote: Regarding there not being a `no-wraps` attribute. What happens with code like this? Is the attribute lost or casted away during the addition? ``` wrapping_int a = INT_MAX; a = (int) a + 1; ``` Does it affect converting a number too large to fit in the target type? ``` wrapping_int a = INT_MAX; wrapping_short b = a; short c = a; ``` https://github.com/llvm/llvm-project/pull/86618 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Utils] add --update-tests flag to llvm-lit (PR #97369)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/97369
>From d93d77e193f235d12d4de4a4b184c458508fa8df Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Mon, 1 Jul 2024 18:19:09 -0700
Subject: [PATCH 1/4] [Utils] add update-verify-tests.py
Adds a python script to automatically take output from a failed clang
-verify test and update the test case(s) to expect the new behaviour.
---
clang/utils/update-verify-tests.py | 404 +
1 file changed, 404 insertions(+)
create mode 100644 clang/utils/update-verify-tests.py
diff --git a/clang/utils/update-verify-tests.py
b/clang/utils/update-verify-tests.py
new file mode 100644
index 00..cfcfefc85e576a
--- /dev/null
+++ b/clang/utils/update-verify-tests.py
@@ -0,0 +1,404 @@
+import sys
+import re
+
+"""
+ Pipe output from clang's -verify into this script to have the test case
updated to expect the actual diagnostic output.
+ When inserting new expected-* checks it will place them on the line before
the location of the diagnostic, with an @+1,
+ or @+N for some N if there are multiple diagnostics emitted on the same line.
If the current checks are using @-N for
+ this line, the new check will follow that convention also.
+ Existing checks will be left untouched as much as possible, including their
location and whitespace content, to minimize
+ diffs. If inaccurate their count will be updated, or the check removed
entirely.
+
+ Missing features:
+ - custom prefix support (-verify=my-prefix)
+ - multiple prefixes on the same line (-verify=my-prefix,my-other-prefix)
+ - multiple prefixes on separate RUN lines (RUN: -verify=my-prefix\nRUN:
-verify my-other-prefix)
+ - regexes with expected-*-re: existing ones will be left untouched if
accurate, but the script will abort if there are any
+diagnostic mismatches on the same line.
+ - multiple checks targeting the same line are supported, but a line may only
contain one check
+ - if multiple checks targeting the same line are failing the script is not
guaranteed to produce a minimal diff
+
+Example usage:
+ build/bin/llvm-lit clang/test/Sema/ --no-progress-bar -v | python3
update-verify-tests.py
+"""
+
+
+class KnownException(Exception):
+pass
+
+
+def parse_error_category(s):
+parts = s.split("diagnostics")
+diag_category = parts[0]
+category_parts = parts[0].strip().strip("'").split("-")
+expected = category_parts[0]
+if expected != "expected":
+raise Exception(
+f"expected 'expected', but found '{expected}'. Custom verify
prefixes are not supported."
+)
+diag_category = category_parts[1]
+if "seen but not expected" in parts[1]:
+seen = True
+elif "expected but not seen" in parts[1]:
+seen = False
+else:
+raise KnownException(f"unexpected category '{parts[1]}'")
+return (diag_category, seen)
+
+
+diag_error_re = re.compile(r"File (\S+) Line (\d+): (.+)")
+diag_error_re2 = re.compile(r"File \S+ Line \d+ \(directive at (\S+):(\d+)\):
(.+)")
+
+
+def parse_diag_error(s):
+m = diag_error_re2.match(s)
+if not m:
+m = diag_error_re.match(s)
+if not m:
+return None
+return (m.group(1), int(m.group(2)), m.group(3))
+
+
+class Line:
+def __init__(self, content, line_n):
+self.content = content
+self.diag = None
+self.line_n = line_n
+self.related_diags = []
+self.targeting_diags = []
+
+def update_line_n(self, n):
+if self.diag and not self.diag.line_is_absolute:
+self.diag.orig_target_line_n += n - self.line_n
+self.line_n = n
+for diag in self.targeting_diags:
+if diag.line_is_absolute:
+diag.orig_target_line_n = n
+else:
+diag.orig_target_line_n = n - diag.line.line_n
+for diag in self.related_diags:
+if not diag.line_is_absolute:
+pass
+
+def render(self):
+if not self.diag:
+return self.content
+assert "{{DIAG}}" in self.content
+res = self.content.replace("{{DIAG}}", self.diag.render())
+if not res.strip():
+return ""
+return res
+
+
+class Diag:
+def __init__(
+self,
+diag_content,
+category,
+targeted_line_n,
+line_is_absolute,
+count,
+line,
+is_re,
+whitespace_strings,
+):
+self.diag_content = diag_content
+self.category = category
+self.orig_target_line_n = targeted_line_n
+self.line_is_absolute = line_is_absolute
+self.count = count
+self.line = line
+self.target = None
+self.is_re = is_re
+self.absolute_target()
+self.whitespace_strings = whitespace_strings
+
+def add(self):
+if targeted_line > 0:
+targeted_line += 1
+elif targeted_line < 0:
+targeted_li
[clang] [Utils] add update-verify-tests.py (PR #97369)
https://github.com/hnrklssn edited https://github.com/llvm/llvm-project/pull/97369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
hnrklssn wrote: Note that the first 4 commits are from https://github.com/llvm/llvm-project/pull/97369; only the last 2 are new. I'll merge that tomorrow when I have more time to be ready to revert in case something should happen. https://github.com/llvm/llvm-project/pull/108425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Utils] add update-verify-tests.py (PR #97369)
https://github.com/hnrklssn closed https://github.com/llvm/llvm-project/pull/97369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Utils] add update-verify-tests.py (PR #97369)
hnrklssn wrote: Very sorry about the CI oversight on my part! Thanks for reverting, I was in a meeting. Looks like I was bitten by Windows line endings again... https://github.com/llvm/llvm-project/pull/97369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Utils] add update-verify-tests.py" (#108630)" (PR #108658)
https://github.com/hnrklssn created https://github.com/llvm/llvm-project/pull/108658 This relands commit d4f41befb7256f8e8378ae358b2b3d802454d6a4 which was reverted by b7e585b95e241d0506b6f71d53ff5b6e72a9c8f4. This version ignores differences in line endings in the diff tests to make sure the tests work as intended on Windows. Original description below: Adds a python script to automatically take output from a failed clang -verify test and update the test case(s) to expect the new behaviour. >From dcf20a69dc51a17c71c6f7ad378a67ce818c06f5 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Fri, 13 Sep 2024 13:17:05 -0700 Subject: [PATCH] Reland "[Utils] add update-verify-tests.py" (#108630)" This relands commit d4f41befb7256f8e8378ae358b2b3d802454d6a4 which was reverted by b7e585b95e241d0506b6f71d53ff5b6e72a9c8f4. This version ignores differences in line endings in the diff tests to make sure the tests work as intended on Windows. Original description below: Adds a python script to automatically take output from a failed clang -verify test and update the test case(s) to expect the new behaviour. --- .../Inputs/duplicate-diag.c | 8 + .../Inputs/duplicate-diag.c.expected | 8 + .../Inputs/infer-indentation.c| 8 + .../Inputs/infer-indentation.c.expected | 11 + .../Inputs/leave-existing-diags.c | 11 + .../Inputs/leave-existing-diags.c.expected| 12 + .../Inputs/multiple-errors.c | 6 + .../Inputs/multiple-errors.c.expected | 9 + .../multiple-missing-errors-same-line.c | 8 + ...ltiple-missing-errors-same-line.c.expected | 13 + .../update-verify-tests/Inputs/no-checks.c| 3 + .../Inputs/no-checks.c.expected | 4 + .../update-verify-tests/Inputs/no-diags.c | 5 + .../Inputs/no-diags.c.expected| 5 + .../Inputs/no-expected-diags.c| 4 + .../Inputs/no-expected-diags.c.expected | 4 + .../Inputs/non-default-prefix.c | 5 + .../Inputs/non-default-prefix.c.expected | 5 + .../Inputs/update-same-line.c | 4 + .../Inputs/update-same-line.c.expected| 4 + .../Inputs/update-single-check.c | 4 + .../Inputs/update-single-check.c.expected | 4 + .../update-verify-tests/duplicate-diag.test | 4 + .../infer-indentation.test| 3 + .../leave-existing-diags.test | 4 + .../utils/update-verify-tests/lit.local.cfg | 25 + .../update-verify-tests/multiple-errors.test | 3 + .../multiple-missing-errors-same-line.test| 3 + .../utils/update-verify-tests/no-checks.test | 3 + .../utils/update-verify-tests/no-diags.test | 4 + .../no-expected-diags.test| 4 + .../non-default-prefix.test | 4 + .../update-verify-tests/update-same-line.test | 4 + .../update-single-check.test | 3 + clang/utils/UpdateVerifyTests/core.py | 452 ++ clang/utils/update-verify-tests.py| 38 ++ 36 files changed, 699 insertions(+) create mode 100644 clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c create mode 100644 clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected create mode 100644 clang/test/utils/update-verify-tests/Inputs/infer-indentation.c create mode 100644 clang/test/utils/update-verify-tests/Inputs/infer-indentation.c.expected create mode 100644 clang/test/utils/update-verify-tests/Inputs/leave-existing-diags.c create mode 100644 clang/test/utils/update-verify-tests/Inputs/leave-existing-diags.c.expected create mode 100644 clang/test/utils/update-verify-tests/Inputs/multiple-errors.c create mode 100644 clang/test/utils/update-verify-tests/Inputs/multiple-errors.c.expected create mode 100644 clang/test/utils/update-verify-tests/Inputs/multiple-missing-errors-same-line.c create mode 100644 clang/test/utils/update-verify-tests/Inputs/multiple-missing-errors-same-line.c.expected create mode 100644 clang/test/utils/update-verify-tests/Inputs/no-checks.c create mode 100644 clang/test/utils/update-verify-tests/Inputs/no-checks.c.expected create mode 100644 clang/test/utils/update-verify-tests/Inputs/no-diags.c create mode 100644 clang/test/utils/update-verify-tests/Inputs/no-diags.c.expected create mode 100644 clang/test/utils/update-verify-tests/Inputs/no-expected-diags.c create mode 100644 clang/test/utils/update-verify-tests/Inputs/no-expected-diags.c.expected create mode 100644 clang/test/utils/update-verify-tests/Inputs/non-default-prefix.c create mode 100644 clang/test/utils/update-verify-tests/Inputs/non-default-prefix.c.expected create mode 100644 clang/test/utils/update-verify-tests/Inputs/update-same-line.c create mode 100644 clang/test/utils/update-verify-tests/Inputs/update-same-line.c.expected create mode 100644 clang/test/utils/update-verif
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/108425
>From 11c5fe752a880dfa220abdf46ad9ba1a8be66b37 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Wed, 28 Aug 2024 23:30:49 -0700
Subject: [PATCH 1/2] [Utils] Add --update-tests to lit
This adds a flag to lit for detecting and updating failing tests when
possible to do so automatically. The flag uses a plugin architecture
where config files can add additional auto-updaters for the types of
tests in the test suite. When a test fails with --update-tests enabled
lit passes the test RUN invocation and output to each registered test
updater until one of them signals that it updated the test. As such it
is the responsibility of the test updater to only update tests where it
is reasonably certain that it will actually fix the test, or come close
to doing so.
Also adds one initial test updater specific to clang's Sema test suite,
which updates the expected-[diag-kind] lines in tests using the -verify
flag.
---
clang/test/Sema/lit.local.cfg | 12 ++
.../Inputs/duplicate-diag.c | 2 +
.../Inputs/duplicate-diag.c.expected | 2 +
.../Inputs/infer-indentation.c| 2 +
.../Inputs/infer-indentation.c.expected | 2 +
.../Inputs/leave-existing-diags.c | 4 +-
.../Inputs/leave-existing-diags.c.expected| 4 +-
.../Inputs/multiple-errors.c | 2 +
.../Inputs/multiple-errors.c.expected | 2 +
.../multiple-missing-errors-same-line.c | 2 +
...ltiple-missing-errors-same-line.c.expected | 2 +
.../update-verify-tests/Inputs/no-checks.c| 2 +
.../Inputs/no-checks.c.expected | 2 +
.../update-verify-tests/Inputs/no-diags.c | 2 +
.../Inputs/no-diags.c.expected| 2 +
.../Inputs/no-expected-diags.c| 2 +
.../Inputs/no-expected-diags.c.expected | 2 +
.../Inputs/non-default-prefix.c | 2 +
.../Inputs/non-default-prefix.c.expected | 2 +
.../Inputs/update-same-line.c | 2 +
.../Inputs/update-same-line.c.expected| 2 +
.../Inputs/update-single-check.c | 2 +
.../Inputs/update-single-check.c.expected | 2 +
.../update-verify-tests/LitTests/.gitignore | 4 ++
.../update-verify-tests/LitTests/lit.cfg | 31
.../utils/update-verify-tests/lit-plugin.test | 5 +++
.../utils/update-verify-tests/lit.local.cfg | 11 ++
clang/utils/UpdateVerifyTests/__init__.py | 1 +
clang/utils/UpdateVerifyTests/litplugin.py| 37 +++
clang/utils/update-verify-tests.py| 3 ++
llvm/utils/lit/lit/LitConfig.py | 3 ++
llvm/utils/lit/lit/TestRunner.py | 12 ++
llvm/utils/lit/lit/cl_arguments.py| 6 +++
llvm/utils/lit/lit/llvm/config.py | 5 +++
llvm/utils/lit/lit/main.py| 1 +
35 files changed, 177 insertions(+), 2 deletions(-)
create mode 100644 clang/test/utils/update-verify-tests/LitTests/.gitignore
create mode 100644 clang/test/utils/update-verify-tests/LitTests/lit.cfg
create mode 100644 clang/test/utils/update-verify-tests/lit-plugin.test
create mode 100644 clang/utils/UpdateVerifyTests/__init__.py
create mode 100644 clang/utils/UpdateVerifyTests/litplugin.py
diff --git a/clang/test/Sema/lit.local.cfg b/clang/test/Sema/lit.local.cfg
index baf1b39ef238cd..b385fd92098a8e 100644
--- a/clang/test/Sema/lit.local.cfg
+++ b/clang/test/Sema/lit.local.cfg
@@ -2,3 +2,15 @@ config.substitutions = list(config.substitutions)
config.substitutions.insert(
0, (r"%clang\b", """*** Do not use the driver in Sema tests. ***""")
)
+
+if lit_config.update_tests:
+import sys
+import os
+
+curdir = os.path.dirname(os.path.realpath(__file__))
+testdir = os.path.dirname(curdir)
+clangdir = os.path.dirname(testdir)
+utilspath = os.path.join(clangdir, "utils")
+sys.path.append(utilspath)
+from UpdateVerifyTests.litplugin import verify_test_updater
+lit_config.test_updaters.append(verify_test_updater)
diff --git a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
index 8c7e46c6eca9c1..d4a92eb4a7874a 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
@@ -1,3 +1,5 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: diff %s %s.expected
void foo() {
// expected-error@+1{{use of undeclared identifier 'a'}}
a = 2; a = 2;
diff --git
a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
index 6214ff382f4495..5fd72709e487f5 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
@@ -1,3 +1,5 @@
+// RUN:
[clang] Reland "[Utils] add update-verify-tests.py" (#108630)" (PR #108658)
https://github.com/hnrklssn closed https://github.com/llvm/llvm-project/pull/108658 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/108425
>From 451a178dbb461e6b3dd264be6ede0aa26283dbbe Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Wed, 28 Aug 2024 23:30:49 -0700
Subject: [PATCH 1/2] [Utils] Add --update-tests to lit
This adds a flag to lit for detecting and updating failing tests when
possible to do so automatically. The flag uses a plugin architecture
where config files can add additional auto-updaters for the types of
tests in the test suite. When a test fails with --update-tests enabled
lit passes the test RUN invocation and output to each registered test
updater until one of them signals that it updated the test. As such it
is the responsibility of the test updater to only update tests where it
is reasonably certain that it will actually fix the test, or come close
to doing so.
Also adds one initial test updater specific to clang's Sema test suite,
which updates the expected-[diag-kind] lines in tests using the -verify
flag.
---
clang/test/Sema/lit.local.cfg | 12 ++
.../Inputs/duplicate-diag.c | 2 +
.../Inputs/duplicate-diag.c.expected | 2 +
.../Inputs/infer-indentation.c| 2 +
.../Inputs/infer-indentation.c.expected | 2 +
.../Inputs/leave-existing-diags.c | 4 +-
.../Inputs/leave-existing-diags.c.expected| 4 +-
.../Inputs/multiple-errors.c | 2 +
.../Inputs/multiple-errors.c.expected | 2 +
.../multiple-missing-errors-same-line.c | 2 +
...ltiple-missing-errors-same-line.c.expected | 2 +
.../update-verify-tests/Inputs/no-checks.c| 2 +
.../Inputs/no-checks.c.expected | 2 +
.../update-verify-tests/Inputs/no-diags.c | 2 +
.../Inputs/no-diags.c.expected| 2 +
.../Inputs/no-expected-diags.c| 2 +
.../Inputs/no-expected-diags.c.expected | 2 +
.../Inputs/non-default-prefix.c | 2 +
.../Inputs/non-default-prefix.c.expected | 2 +
.../Inputs/update-same-line.c | 2 +
.../Inputs/update-same-line.c.expected| 2 +
.../Inputs/update-single-check.c | 2 +
.../Inputs/update-single-check.c.expected | 2 +
.../update-verify-tests/LitTests/.gitignore | 4 ++
.../update-verify-tests/LitTests/lit.cfg | 31
.../utils/update-verify-tests/lit-plugin.test | 5 +++
.../utils/update-verify-tests/lit.local.cfg | 11 ++
clang/utils/UpdateVerifyTests/__init__.py | 1 +
clang/utils/UpdateVerifyTests/litplugin.py| 37 +++
clang/utils/update-verify-tests.py| 3 ++
llvm/utils/lit/lit/LitConfig.py | 3 ++
llvm/utils/lit/lit/TestRunner.py | 12 ++
llvm/utils/lit/lit/cl_arguments.py| 6 +++
llvm/utils/lit/lit/llvm/config.py | 5 +++
llvm/utils/lit/lit/main.py| 1 +
35 files changed, 177 insertions(+), 2 deletions(-)
create mode 100644 clang/test/utils/update-verify-tests/LitTests/.gitignore
create mode 100644 clang/test/utils/update-verify-tests/LitTests/lit.cfg
create mode 100644 clang/test/utils/update-verify-tests/lit-plugin.test
create mode 100644 clang/utils/UpdateVerifyTests/__init__.py
create mode 100644 clang/utils/UpdateVerifyTests/litplugin.py
diff --git a/clang/test/Sema/lit.local.cfg b/clang/test/Sema/lit.local.cfg
index baf1b39ef238cd..b385fd92098a8e 100644
--- a/clang/test/Sema/lit.local.cfg
+++ b/clang/test/Sema/lit.local.cfg
@@ -2,3 +2,15 @@ config.substitutions = list(config.substitutions)
config.substitutions.insert(
0, (r"%clang\b", """*** Do not use the driver in Sema tests. ***""")
)
+
+if lit_config.update_tests:
+import sys
+import os
+
+curdir = os.path.dirname(os.path.realpath(__file__))
+testdir = os.path.dirname(curdir)
+clangdir = os.path.dirname(testdir)
+utilspath = os.path.join(clangdir, "utils")
+sys.path.append(utilspath)
+from UpdateVerifyTests.litplugin import verify_test_updater
+lit_config.test_updaters.append(verify_test_updater)
diff --git a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
index 8c7e46c6eca9c1..d4a92eb4a7874a 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
@@ -1,3 +1,5 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: diff %s %s.expected
void foo() {
// expected-error@+1{{use of undeclared identifier 'a'}}
a = 2; a = 2;
diff --git
a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
index 6214ff382f4495..5fd72709e487f5 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
@@ -1,3 +1,5 @@
+// RUN:
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/108425
>From 451a178dbb461e6b3dd264be6ede0aa26283dbbe Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Wed, 28 Aug 2024 23:30:49 -0700
Subject: [PATCH 1/2] [Utils] Add --update-tests to lit
This adds a flag to lit for detecting and updating failing tests when
possible to do so automatically. The flag uses a plugin architecture
where config files can add additional auto-updaters for the types of
tests in the test suite. When a test fails with --update-tests enabled
lit passes the test RUN invocation and output to each registered test
updater until one of them signals that it updated the test. As such it
is the responsibility of the test updater to only update tests where it
is reasonably certain that it will actually fix the test, or come close
to doing so.
Also adds one initial test updater specific to clang's Sema test suite,
which updates the expected-[diag-kind] lines in tests using the -verify
flag.
---
clang/test/Sema/lit.local.cfg | 12 ++
.../Inputs/duplicate-diag.c | 2 +
.../Inputs/duplicate-diag.c.expected | 2 +
.../Inputs/infer-indentation.c| 2 +
.../Inputs/infer-indentation.c.expected | 2 +
.../Inputs/leave-existing-diags.c | 4 +-
.../Inputs/leave-existing-diags.c.expected| 4 +-
.../Inputs/multiple-errors.c | 2 +
.../Inputs/multiple-errors.c.expected | 2 +
.../multiple-missing-errors-same-line.c | 2 +
...ltiple-missing-errors-same-line.c.expected | 2 +
.../update-verify-tests/Inputs/no-checks.c| 2 +
.../Inputs/no-checks.c.expected | 2 +
.../update-verify-tests/Inputs/no-diags.c | 2 +
.../Inputs/no-diags.c.expected| 2 +
.../Inputs/no-expected-diags.c| 2 +
.../Inputs/no-expected-diags.c.expected | 2 +
.../Inputs/non-default-prefix.c | 2 +
.../Inputs/non-default-prefix.c.expected | 2 +
.../Inputs/update-same-line.c | 2 +
.../Inputs/update-same-line.c.expected| 2 +
.../Inputs/update-single-check.c | 2 +
.../Inputs/update-single-check.c.expected | 2 +
.../update-verify-tests/LitTests/.gitignore | 4 ++
.../update-verify-tests/LitTests/lit.cfg | 31
.../utils/update-verify-tests/lit-plugin.test | 5 +++
.../utils/update-verify-tests/lit.local.cfg | 11 ++
clang/utils/UpdateVerifyTests/__init__.py | 1 +
clang/utils/UpdateVerifyTests/litplugin.py| 37 +++
clang/utils/update-verify-tests.py| 3 ++
llvm/utils/lit/lit/LitConfig.py | 3 ++
llvm/utils/lit/lit/TestRunner.py | 12 ++
llvm/utils/lit/lit/cl_arguments.py| 6 +++
llvm/utils/lit/lit/llvm/config.py | 5 +++
llvm/utils/lit/lit/main.py| 1 +
35 files changed, 177 insertions(+), 2 deletions(-)
create mode 100644 clang/test/utils/update-verify-tests/LitTests/.gitignore
create mode 100644 clang/test/utils/update-verify-tests/LitTests/lit.cfg
create mode 100644 clang/test/utils/update-verify-tests/lit-plugin.test
create mode 100644 clang/utils/UpdateVerifyTests/__init__.py
create mode 100644 clang/utils/UpdateVerifyTests/litplugin.py
diff --git a/clang/test/Sema/lit.local.cfg b/clang/test/Sema/lit.local.cfg
index baf1b39ef238cd..b385fd92098a8e 100644
--- a/clang/test/Sema/lit.local.cfg
+++ b/clang/test/Sema/lit.local.cfg
@@ -2,3 +2,15 @@ config.substitutions = list(config.substitutions)
config.substitutions.insert(
0, (r"%clang\b", """*** Do not use the driver in Sema tests. ***""")
)
+
+if lit_config.update_tests:
+import sys
+import os
+
+curdir = os.path.dirname(os.path.realpath(__file__))
+testdir = os.path.dirname(curdir)
+clangdir = os.path.dirname(testdir)
+utilspath = os.path.join(clangdir, "utils")
+sys.path.append(utilspath)
+from UpdateVerifyTests.litplugin import verify_test_updater
+lit_config.test_updaters.append(verify_test_updater)
diff --git a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
index 8c7e46c6eca9c1..d4a92eb4a7874a 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
@@ -1,3 +1,5 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: diff %s %s.expected
void foo() {
// expected-error@+1{{use of undeclared identifier 'a'}}
a = 2; a = 2;
diff --git
a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
index 6214ff382f4495..5fd72709e487f5 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
@@ -1,3 +1,5 @@
+// RUN:
[clang] fix update-verify-tests test suite for AIX (PR #108871)
https://github.com/hnrklssn created https://github.com/llvm/llvm-project/pull/108871 The diff command on AIX doesn't support the --strip-trailing-cr flag. The internal python implementation does, so execute the tests in the update-verify-tests test suite using the internal shell for compatibility. >From 9b1a291017aa599d560e4fdbd30e3d5ef8fc3937 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Mon, 16 Sep 2024 11:42:35 -0700 Subject: [PATCH] fix update-verify-tests test suite for AIX The diff command on AIX doesn't support the --strip-trailing-cr flag. The internal python implementation does, so execute the tests in the update-verify-tests test suite using the internal shell for compatibility. --- clang/test/utils/update-verify-tests/lit.local.cfg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/test/utils/update-verify-tests/lit.local.cfg b/clang/test/utils/update-verify-tests/lit.local.cfg index a0b6afccc25010..b0eebf337da5c9 100644 --- a/clang/test/utils/update-verify-tests/lit.local.cfg +++ b/clang/test/utils/update-verify-tests/lit.local.cfg @@ -23,3 +23,6 @@ else: "%s %s" % (python, shell_quote(script_path)), ) ) +# AIX 'diff' command doesn't support --strip-trailing-cr, but the internal +# python implementation does, so use that for cross platform compatibility +config.test_format = lit.formats.ShTest() ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Utils] add update-verify-tests.py" (#108630)" (PR #108658)
hnrklssn wrote: > Hi, `--strip-trailing-cr` isn't a supported option with diff on AIX. Would > you be able to adapt the tests? > > https://lab.llvm.org/buildbot/#/builders/64/builds/985 This should fix it: https://github.com/llvm/llvm-project/pull/108871 https://github.com/llvm/llvm-project/pull/108658 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/108425
>From 451a178dbb461e6b3dd264be6ede0aa26283dbbe Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Wed, 28 Aug 2024 23:30:49 -0700
Subject: [PATCH 1/3] [Utils] Add --update-tests to lit
This adds a flag to lit for detecting and updating failing tests when
possible to do so automatically. The flag uses a plugin architecture
where config files can add additional auto-updaters for the types of
tests in the test suite. When a test fails with --update-tests enabled
lit passes the test RUN invocation and output to each registered test
updater until one of them signals that it updated the test. As such it
is the responsibility of the test updater to only update tests where it
is reasonably certain that it will actually fix the test, or come close
to doing so.
Also adds one initial test updater specific to clang's Sema test suite,
which updates the expected-[diag-kind] lines in tests using the -verify
flag.
---
clang/test/Sema/lit.local.cfg | 12 ++
.../Inputs/duplicate-diag.c | 2 +
.../Inputs/duplicate-diag.c.expected | 2 +
.../Inputs/infer-indentation.c| 2 +
.../Inputs/infer-indentation.c.expected | 2 +
.../Inputs/leave-existing-diags.c | 4 +-
.../Inputs/leave-existing-diags.c.expected| 4 +-
.../Inputs/multiple-errors.c | 2 +
.../Inputs/multiple-errors.c.expected | 2 +
.../multiple-missing-errors-same-line.c | 2 +
...ltiple-missing-errors-same-line.c.expected | 2 +
.../update-verify-tests/Inputs/no-checks.c| 2 +
.../Inputs/no-checks.c.expected | 2 +
.../update-verify-tests/Inputs/no-diags.c | 2 +
.../Inputs/no-diags.c.expected| 2 +
.../Inputs/no-expected-diags.c| 2 +
.../Inputs/no-expected-diags.c.expected | 2 +
.../Inputs/non-default-prefix.c | 2 +
.../Inputs/non-default-prefix.c.expected | 2 +
.../Inputs/update-same-line.c | 2 +
.../Inputs/update-same-line.c.expected| 2 +
.../Inputs/update-single-check.c | 2 +
.../Inputs/update-single-check.c.expected | 2 +
.../update-verify-tests/LitTests/.gitignore | 4 ++
.../update-verify-tests/LitTests/lit.cfg | 31
.../utils/update-verify-tests/lit-plugin.test | 5 +++
.../utils/update-verify-tests/lit.local.cfg | 11 ++
clang/utils/UpdateVerifyTests/__init__.py | 1 +
clang/utils/UpdateVerifyTests/litplugin.py| 37 +++
clang/utils/update-verify-tests.py| 3 ++
llvm/utils/lit/lit/LitConfig.py | 3 ++
llvm/utils/lit/lit/TestRunner.py | 12 ++
llvm/utils/lit/lit/cl_arguments.py| 6 +++
llvm/utils/lit/lit/llvm/config.py | 5 +++
llvm/utils/lit/lit/main.py| 1 +
35 files changed, 177 insertions(+), 2 deletions(-)
create mode 100644 clang/test/utils/update-verify-tests/LitTests/.gitignore
create mode 100644 clang/test/utils/update-verify-tests/LitTests/lit.cfg
create mode 100644 clang/test/utils/update-verify-tests/lit-plugin.test
create mode 100644 clang/utils/UpdateVerifyTests/__init__.py
create mode 100644 clang/utils/UpdateVerifyTests/litplugin.py
diff --git a/clang/test/Sema/lit.local.cfg b/clang/test/Sema/lit.local.cfg
index baf1b39ef238cd..b385fd92098a8e 100644
--- a/clang/test/Sema/lit.local.cfg
+++ b/clang/test/Sema/lit.local.cfg
@@ -2,3 +2,15 @@ config.substitutions = list(config.substitutions)
config.substitutions.insert(
0, (r"%clang\b", """*** Do not use the driver in Sema tests. ***""")
)
+
+if lit_config.update_tests:
+import sys
+import os
+
+curdir = os.path.dirname(os.path.realpath(__file__))
+testdir = os.path.dirname(curdir)
+clangdir = os.path.dirname(testdir)
+utilspath = os.path.join(clangdir, "utils")
+sys.path.append(utilspath)
+from UpdateVerifyTests.litplugin import verify_test_updater
+lit_config.test_updaters.append(verify_test_updater)
diff --git a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
index 8c7e46c6eca9c1..d4a92eb4a7874a 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
@@ -1,3 +1,5 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: diff %s %s.expected
void foo() {
// expected-error@+1{{use of undeclared identifier 'a'}}
a = 2; a = 2;
diff --git
a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
index 6214ff382f4495..5fd72709e487f5 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
@@ -1,3 +1,5 @@
+// RUN:
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
@@ -2,3 +2,15 @@ config.substitutions = list(config.substitutions) config.substitutions.insert( 0, (r"%clang\b", """*** Do not use the driver in Sema tests. ***""") ) + +if lit_config.update_tests: +import sys +import os + +curdir = os.path.dirname(os.path.realpath(__file__)) +testdir = os.path.dirname(curdir) +clangdir = os.path.dirname(testdir) +utilspath = os.path.join(clangdir, "utils") hnrklssn wrote: Ah neat, wasn't aware that was a thing https://github.com/llvm/llvm-project/pull/108425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/108425
>From 451a178dbb461e6b3dd264be6ede0aa26283dbbe Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Wed, 28 Aug 2024 23:30:49 -0700
Subject: [PATCH 1/4] [Utils] Add --update-tests to lit
This adds a flag to lit for detecting and updating failing tests when
possible to do so automatically. The flag uses a plugin architecture
where config files can add additional auto-updaters for the types of
tests in the test suite. When a test fails with --update-tests enabled
lit passes the test RUN invocation and output to each registered test
updater until one of them signals that it updated the test. As such it
is the responsibility of the test updater to only update tests where it
is reasonably certain that it will actually fix the test, or come close
to doing so.
Also adds one initial test updater specific to clang's Sema test suite,
which updates the expected-[diag-kind] lines in tests using the -verify
flag.
---
clang/test/Sema/lit.local.cfg | 12 ++
.../Inputs/duplicate-diag.c | 2 +
.../Inputs/duplicate-diag.c.expected | 2 +
.../Inputs/infer-indentation.c| 2 +
.../Inputs/infer-indentation.c.expected | 2 +
.../Inputs/leave-existing-diags.c | 4 +-
.../Inputs/leave-existing-diags.c.expected| 4 +-
.../Inputs/multiple-errors.c | 2 +
.../Inputs/multiple-errors.c.expected | 2 +
.../multiple-missing-errors-same-line.c | 2 +
...ltiple-missing-errors-same-line.c.expected | 2 +
.../update-verify-tests/Inputs/no-checks.c| 2 +
.../Inputs/no-checks.c.expected | 2 +
.../update-verify-tests/Inputs/no-diags.c | 2 +
.../Inputs/no-diags.c.expected| 2 +
.../Inputs/no-expected-diags.c| 2 +
.../Inputs/no-expected-diags.c.expected | 2 +
.../Inputs/non-default-prefix.c | 2 +
.../Inputs/non-default-prefix.c.expected | 2 +
.../Inputs/update-same-line.c | 2 +
.../Inputs/update-same-line.c.expected| 2 +
.../Inputs/update-single-check.c | 2 +
.../Inputs/update-single-check.c.expected | 2 +
.../update-verify-tests/LitTests/.gitignore | 4 ++
.../update-verify-tests/LitTests/lit.cfg | 31
.../utils/update-verify-tests/lit-plugin.test | 5 +++
.../utils/update-verify-tests/lit.local.cfg | 11 ++
clang/utils/UpdateVerifyTests/__init__.py | 1 +
clang/utils/UpdateVerifyTests/litplugin.py| 37 +++
clang/utils/update-verify-tests.py| 3 ++
llvm/utils/lit/lit/LitConfig.py | 3 ++
llvm/utils/lit/lit/TestRunner.py | 12 ++
llvm/utils/lit/lit/cl_arguments.py| 6 +++
llvm/utils/lit/lit/llvm/config.py | 5 +++
llvm/utils/lit/lit/main.py| 1 +
35 files changed, 177 insertions(+), 2 deletions(-)
create mode 100644 clang/test/utils/update-verify-tests/LitTests/.gitignore
create mode 100644 clang/test/utils/update-verify-tests/LitTests/lit.cfg
create mode 100644 clang/test/utils/update-verify-tests/lit-plugin.test
create mode 100644 clang/utils/UpdateVerifyTests/__init__.py
create mode 100644 clang/utils/UpdateVerifyTests/litplugin.py
diff --git a/clang/test/Sema/lit.local.cfg b/clang/test/Sema/lit.local.cfg
index baf1b39ef238cd..b385fd92098a8e 100644
--- a/clang/test/Sema/lit.local.cfg
+++ b/clang/test/Sema/lit.local.cfg
@@ -2,3 +2,15 @@ config.substitutions = list(config.substitutions)
config.substitutions.insert(
0, (r"%clang\b", """*** Do not use the driver in Sema tests. ***""")
)
+
+if lit_config.update_tests:
+import sys
+import os
+
+curdir = os.path.dirname(os.path.realpath(__file__))
+testdir = os.path.dirname(curdir)
+clangdir = os.path.dirname(testdir)
+utilspath = os.path.join(clangdir, "utils")
+sys.path.append(utilspath)
+from UpdateVerifyTests.litplugin import verify_test_updater
+lit_config.test_updaters.append(verify_test_updater)
diff --git a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
index 8c7e46c6eca9c1..d4a92eb4a7874a 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
@@ -1,3 +1,5 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: diff %s %s.expected
void foo() {
// expected-error@+1{{use of undeclared identifier 'a'}}
a = 2; a = 2;
diff --git
a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
index 6214ff382f4495..5fd72709e487f5 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
@@ -1,3 +1,5 @@
+// RUN:
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/108425
>From 451a178dbb461e6b3dd264be6ede0aa26283dbbe Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Wed, 28 Aug 2024 23:30:49 -0700
Subject: [PATCH 1/5] [Utils] Add --update-tests to lit
This adds a flag to lit for detecting and updating failing tests when
possible to do so automatically. The flag uses a plugin architecture
where config files can add additional auto-updaters for the types of
tests in the test suite. When a test fails with --update-tests enabled
lit passes the test RUN invocation and output to each registered test
updater until one of them signals that it updated the test. As such it
is the responsibility of the test updater to only update tests where it
is reasonably certain that it will actually fix the test, or come close
to doing so.
Also adds one initial test updater specific to clang's Sema test suite,
which updates the expected-[diag-kind] lines in tests using the -verify
flag.
---
clang/test/Sema/lit.local.cfg | 12 ++
.../Inputs/duplicate-diag.c | 2 +
.../Inputs/duplicate-diag.c.expected | 2 +
.../Inputs/infer-indentation.c| 2 +
.../Inputs/infer-indentation.c.expected | 2 +
.../Inputs/leave-existing-diags.c | 4 +-
.../Inputs/leave-existing-diags.c.expected| 4 +-
.../Inputs/multiple-errors.c | 2 +
.../Inputs/multiple-errors.c.expected | 2 +
.../multiple-missing-errors-same-line.c | 2 +
...ltiple-missing-errors-same-line.c.expected | 2 +
.../update-verify-tests/Inputs/no-checks.c| 2 +
.../Inputs/no-checks.c.expected | 2 +
.../update-verify-tests/Inputs/no-diags.c | 2 +
.../Inputs/no-diags.c.expected| 2 +
.../Inputs/no-expected-diags.c| 2 +
.../Inputs/no-expected-diags.c.expected | 2 +
.../Inputs/non-default-prefix.c | 2 +
.../Inputs/non-default-prefix.c.expected | 2 +
.../Inputs/update-same-line.c | 2 +
.../Inputs/update-same-line.c.expected| 2 +
.../Inputs/update-single-check.c | 2 +
.../Inputs/update-single-check.c.expected | 2 +
.../update-verify-tests/LitTests/.gitignore | 4 ++
.../update-verify-tests/LitTests/lit.cfg | 31
.../utils/update-verify-tests/lit-plugin.test | 5 +++
.../utils/update-verify-tests/lit.local.cfg | 11 ++
clang/utils/UpdateVerifyTests/__init__.py | 1 +
clang/utils/UpdateVerifyTests/litplugin.py| 37 +++
clang/utils/update-verify-tests.py| 3 ++
llvm/utils/lit/lit/LitConfig.py | 3 ++
llvm/utils/lit/lit/TestRunner.py | 12 ++
llvm/utils/lit/lit/cl_arguments.py| 6 +++
llvm/utils/lit/lit/llvm/config.py | 5 +++
llvm/utils/lit/lit/main.py| 1 +
35 files changed, 177 insertions(+), 2 deletions(-)
create mode 100644 clang/test/utils/update-verify-tests/LitTests/.gitignore
create mode 100644 clang/test/utils/update-verify-tests/LitTests/lit.cfg
create mode 100644 clang/test/utils/update-verify-tests/lit-plugin.test
create mode 100644 clang/utils/UpdateVerifyTests/__init__.py
create mode 100644 clang/utils/UpdateVerifyTests/litplugin.py
diff --git a/clang/test/Sema/lit.local.cfg b/clang/test/Sema/lit.local.cfg
index baf1b39ef238cd..b385fd92098a8e 100644
--- a/clang/test/Sema/lit.local.cfg
+++ b/clang/test/Sema/lit.local.cfg
@@ -2,3 +2,15 @@ config.substitutions = list(config.substitutions)
config.substitutions.insert(
0, (r"%clang\b", """*** Do not use the driver in Sema tests. ***""")
)
+
+if lit_config.update_tests:
+import sys
+import os
+
+curdir = os.path.dirname(os.path.realpath(__file__))
+testdir = os.path.dirname(curdir)
+clangdir = os.path.dirname(testdir)
+utilspath = os.path.join(clangdir, "utils")
+sys.path.append(utilspath)
+from UpdateVerifyTests.litplugin import verify_test_updater
+lit_config.test_updaters.append(verify_test_updater)
diff --git a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
index 8c7e46c6eca9c1..d4a92eb4a7874a 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
@@ -1,3 +1,5 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: diff %s %s.expected
void foo() {
// expected-error@+1{{use of undeclared identifier 'a'}}
a = 2; a = 2;
diff --git
a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
index 6214ff382f4495..5fd72709e487f5 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
@@ -1,3 +1,5 @@
+// RUN:
[clang] fix update-verify-tests test suite for AIX (PR #108871)
https://github.com/hnrklssn closed https://github.com/llvm/llvm-project/pull/108871 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/108425
>From 451a178dbb461e6b3dd264be6ede0aa26283dbbe Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Wed, 28 Aug 2024 23:30:49 -0700
Subject: [PATCH 1/5] [Utils] Add --update-tests to lit
This adds a flag to lit for detecting and updating failing tests when
possible to do so automatically. The flag uses a plugin architecture
where config files can add additional auto-updaters for the types of
tests in the test suite. When a test fails with --update-tests enabled
lit passes the test RUN invocation and output to each registered test
updater until one of them signals that it updated the test. As such it
is the responsibility of the test updater to only update tests where it
is reasonably certain that it will actually fix the test, or come close
to doing so.
Also adds one initial test updater specific to clang's Sema test suite,
which updates the expected-[diag-kind] lines in tests using the -verify
flag.
---
clang/test/Sema/lit.local.cfg | 12 ++
.../Inputs/duplicate-diag.c | 2 +
.../Inputs/duplicate-diag.c.expected | 2 +
.../Inputs/infer-indentation.c| 2 +
.../Inputs/infer-indentation.c.expected | 2 +
.../Inputs/leave-existing-diags.c | 4 +-
.../Inputs/leave-existing-diags.c.expected| 4 +-
.../Inputs/multiple-errors.c | 2 +
.../Inputs/multiple-errors.c.expected | 2 +
.../multiple-missing-errors-same-line.c | 2 +
...ltiple-missing-errors-same-line.c.expected | 2 +
.../update-verify-tests/Inputs/no-checks.c| 2 +
.../Inputs/no-checks.c.expected | 2 +
.../update-verify-tests/Inputs/no-diags.c | 2 +
.../Inputs/no-diags.c.expected| 2 +
.../Inputs/no-expected-diags.c| 2 +
.../Inputs/no-expected-diags.c.expected | 2 +
.../Inputs/non-default-prefix.c | 2 +
.../Inputs/non-default-prefix.c.expected | 2 +
.../Inputs/update-same-line.c | 2 +
.../Inputs/update-same-line.c.expected| 2 +
.../Inputs/update-single-check.c | 2 +
.../Inputs/update-single-check.c.expected | 2 +
.../update-verify-tests/LitTests/.gitignore | 4 ++
.../update-verify-tests/LitTests/lit.cfg | 31
.../utils/update-verify-tests/lit-plugin.test | 5 +++
.../utils/update-verify-tests/lit.local.cfg | 11 ++
clang/utils/UpdateVerifyTests/__init__.py | 1 +
clang/utils/UpdateVerifyTests/litplugin.py| 37 +++
clang/utils/update-verify-tests.py| 3 ++
llvm/utils/lit/lit/LitConfig.py | 3 ++
llvm/utils/lit/lit/TestRunner.py | 12 ++
llvm/utils/lit/lit/cl_arguments.py| 6 +++
llvm/utils/lit/lit/llvm/config.py | 5 +++
llvm/utils/lit/lit/main.py| 1 +
35 files changed, 177 insertions(+), 2 deletions(-)
create mode 100644 clang/test/utils/update-verify-tests/LitTests/.gitignore
create mode 100644 clang/test/utils/update-verify-tests/LitTests/lit.cfg
create mode 100644 clang/test/utils/update-verify-tests/lit-plugin.test
create mode 100644 clang/utils/UpdateVerifyTests/__init__.py
create mode 100644 clang/utils/UpdateVerifyTests/litplugin.py
diff --git a/clang/test/Sema/lit.local.cfg b/clang/test/Sema/lit.local.cfg
index baf1b39ef238cd..b385fd92098a8e 100644
--- a/clang/test/Sema/lit.local.cfg
+++ b/clang/test/Sema/lit.local.cfg
@@ -2,3 +2,15 @@ config.substitutions = list(config.substitutions)
config.substitutions.insert(
0, (r"%clang\b", """*** Do not use the driver in Sema tests. ***""")
)
+
+if lit_config.update_tests:
+import sys
+import os
+
+curdir = os.path.dirname(os.path.realpath(__file__))
+testdir = os.path.dirname(curdir)
+clangdir = os.path.dirname(testdir)
+utilspath = os.path.join(clangdir, "utils")
+sys.path.append(utilspath)
+from UpdateVerifyTests.litplugin import verify_test_updater
+lit_config.test_updaters.append(verify_test_updater)
diff --git a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
index 8c7e46c6eca9c1..d4a92eb4a7874a 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c
@@ -1,3 +1,5 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: diff %s %s.expected
void foo() {
// expected-error@+1{{use of undeclared identifier 'a'}}
a = 2; a = 2;
diff --git
a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
index 6214ff382f4495..5fd72709e487f5 100644
--- a/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
+++ b/clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected
@@ -1,3 +1,5 @@
+// RUN:
[clang] [llvm] Revert "[Utils] Add new --update-tests flag to llvm-lit" (PR #110772)
https://github.com/hnrklssn closed https://github.com/llvm/llvm-project/pull/110772 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Revert "[Utils] Add new --update-tests flag to llvm-lit" (PR #110772)
https://github.com/hnrklssn created
https://github.com/llvm/llvm-project/pull/110772
Reverts llvm/llvm-project#108425
>From 133d368b16d11e44bc3fca92a042cf5ad0ce8dd1 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Tue, 1 Oct 2024 17:14:31 -0700
Subject: [PATCH] Revert "[Utils] Add new --update-tests flag to llvm-lit
(#108425)"
This reverts commit bb8b9ac0ba5382bcf02f614b5b3a84c3699cf52c.
---
clang/test/lit.cfg.py| 10 --
llvm/docs/CommandGuide/lit.rst | 5 ---
llvm/test/lit.cfg.py | 10 --
llvm/utils/lit/lit/LitConfig.py | 3 --
llvm/utils/lit/lit/TestRunner.py | 12 ---
llvm/utils/lit/lit/cl_arguments.py | 6
llvm/utils/lit/lit/llvm/config.py| 5 ---
llvm/utils/lit/lit/main.py | 1 -
llvm/utils/update_any_test_checks.py | 54 ++--
9 files changed, 3 insertions(+), 103 deletions(-)
diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index 32ed5239b90795..92a3361ce672e2 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -362,13 +362,3 @@ def calculate_arch_features(arch_string):
# possibly be present in system and user configuration files, so disable
# default configs for the test runs.
config.environment["CLANG_NO_DEFAULT_CONFIG"] = "1"
-
-if lit_config.update_tests:
-import sys
-import os
-
-utilspath = os.path.join(config.llvm_src_root, "utils")
-sys.path.append(utilspath)
-from update_any_test_checks import utc_lit_plugin
-
-lit_config.test_updaters.append(utc_lit_plugin)
diff --git a/llvm/docs/CommandGuide/lit.rst b/llvm/docs/CommandGuide/lit.rst
index dadecef567b7c9..c9d5baba3e2f49 100644
--- a/llvm/docs/CommandGuide/lit.rst
+++ b/llvm/docs/CommandGuide/lit.rst
@@ -313,11 +313,6 @@ ADDITIONAL OPTIONS
List all of the discovered tests and exit.
-.. option:: --update-tests
-
- Pass failing tests to functions in the ``lit_config.update_tests`` list to
- check whether any of them know how to update the test to make it pass.
-
EXIT STATUS
---
diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py
index 1d5b2bcae1b766..5a03a85386e0aa 100644
--- a/llvm/test/lit.cfg.py
+++ b/llvm/test/lit.cfg.py
@@ -630,13 +630,3 @@ def have_ld64_plugin_support():
if config.has_logf128:
config.available_features.add("has_logf128")
-
-if lit_config.update_tests:
-import sys
-import os
-
-utilspath = os.path.join(config.llvm_src_root, "utils")
-sys.path.append(utilspath)
-from update_any_test_checks import utc_lit_plugin
-
-lit_config.test_updaters.append(utc_lit_plugin)
diff --git a/llvm/utils/lit/lit/LitConfig.py b/llvm/utils/lit/lit/LitConfig.py
index 198a2bf3172330..5dc712ae28370c 100644
--- a/llvm/utils/lit/lit/LitConfig.py
+++ b/llvm/utils/lit/lit/LitConfig.py
@@ -38,7 +38,6 @@ def __init__(
parallelism_groups={},
per_test_coverage=False,
gtest_sharding=True,
-update_tests=False,
):
# The name of the test runner.
self.progname = progname
@@ -90,8 +89,6 @@ def __init__(
self.parallelism_groups = parallelism_groups
self.per_test_coverage = per_test_coverage
self.gtest_sharding = bool(gtest_sharding)
-self.update_tests = update_tests
-self.test_updaters = []
@property
def maxIndividualTestTime(self):
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 3a2cdc5026b0c2..a1785073547ad0 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1190,18 +1190,6 @@ def executeScriptInternal(
str(result.timeoutReached),
)
-if litConfig.update_tests:
-for test_updater in litConfig.test_updaters:
-try:
-update_output = test_updater(result, test)
-except Exception as e:
-out += f"Exception occurred in test updater: {e}"
-continue
-if update_output:
-for line in update_output.splitlines():
-out += f"# {line}\n"
-break
-
return out, err, exitCode, timeoutInfo
diff --git a/llvm/utils/lit/lit/cl_arguments.py
b/llvm/utils/lit/lit/cl_arguments.py
index dcbe553c6d4827..ed78256ee414b4 100644
--- a/llvm/utils/lit/lit/cl_arguments.py
+++ b/llvm/utils/lit/lit/cl_arguments.py
@@ -204,12 +204,6 @@ def parse_args():
action="store_true",
help="Exit with status zero even if some tests fail",
)
-execution_group.add_argument(
-"--update-tests",
-dest="update_tests",
-action="store_true",
-help="Try to update regression tests to reflect current behavior, if
possible",
-)
execution_test_time_group = execution_group.add_mutually_exclusive_group()
execution_test_time_group.add_argument(
"--skip-test-time-recording",
diff --git a/llvm/utils/lit/lit/llv
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/hnrklssn closed https://github.com/llvm/llvm-project/pull/108425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert update-verify-tests.py (PR #109171)
https://github.com/hnrklssn created https://github.com/llvm/llvm-project/pull/109171 This reverts commits c96ee0ffaf5ee7afa1f4b0be0662852f57b47244 and 9ceb9676678ad979a0b767450855d7852ce6a553. Discussion in github PR #108658. >From 0c8a40ed24b0b5f319fd2fb49a27703dc7f1 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Wed, 18 Sep 2024 10:23:53 -0700 Subject: [PATCH] Revert update-verify-tests.py This reverts commits c96ee0ffaf5ee7afa1f4b0be0662852f57b47244 and 9ceb9676678ad979a0b767450855d7852ce6a553. Discussion in github PR #108658. --- .../Inputs/duplicate-diag.c | 8 - .../Inputs/duplicate-diag.c.expected | 8 - .../Inputs/infer-indentation.c| 8 - .../Inputs/infer-indentation.c.expected | 11 - .../Inputs/leave-existing-diags.c | 11 - .../Inputs/leave-existing-diags.c.expected| 12 - .../Inputs/multiple-errors.c | 6 - .../Inputs/multiple-errors.c.expected | 9 - .../multiple-missing-errors-same-line.c | 8 - ...ltiple-missing-errors-same-line.c.expected | 13 - .../update-verify-tests/Inputs/no-checks.c| 3 - .../Inputs/no-checks.c.expected | 4 - .../update-verify-tests/Inputs/no-diags.c | 5 - .../Inputs/no-diags.c.expected| 5 - .../Inputs/no-expected-diags.c| 4 - .../Inputs/no-expected-diags.c.expected | 4 - .../Inputs/non-default-prefix.c | 5 - .../Inputs/non-default-prefix.c.expected | 5 - .../Inputs/update-same-line.c | 4 - .../Inputs/update-same-line.c.expected| 4 - .../Inputs/update-single-check.c | 4 - .../Inputs/update-single-check.c.expected | 4 - .../update-verify-tests/duplicate-diag.test | 4 - .../infer-indentation.test| 3 - .../leave-existing-diags.test | 4 - .../utils/update-verify-tests/lit.local.cfg | 28 -- .../update-verify-tests/multiple-errors.test | 3 - .../multiple-missing-errors-same-line.test| 3 - .../utils/update-verify-tests/no-checks.test | 3 - .../utils/update-verify-tests/no-diags.test | 4 - .../no-expected-diags.test| 4 - .../non-default-prefix.test | 4 - .../update-verify-tests/update-same-line.test | 4 - .../update-single-check.test | 3 - clang/utils/UpdateVerifyTests/core.py | 452 -- clang/utils/update-verify-tests.py| 38 -- 36 files changed, 702 deletions(-) delete mode 100644 clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/duplicate-diag.c.expected delete mode 100644 clang/test/utils/update-verify-tests/Inputs/infer-indentation.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/infer-indentation.c.expected delete mode 100644 clang/test/utils/update-verify-tests/Inputs/leave-existing-diags.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/leave-existing-diags.c.expected delete mode 100644 clang/test/utils/update-verify-tests/Inputs/multiple-errors.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/multiple-errors.c.expected delete mode 100644 clang/test/utils/update-verify-tests/Inputs/multiple-missing-errors-same-line.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/multiple-missing-errors-same-line.c.expected delete mode 100644 clang/test/utils/update-verify-tests/Inputs/no-checks.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/no-checks.c.expected delete mode 100644 clang/test/utils/update-verify-tests/Inputs/no-diags.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/no-diags.c.expected delete mode 100644 clang/test/utils/update-verify-tests/Inputs/no-expected-diags.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/no-expected-diags.c.expected delete mode 100644 clang/test/utils/update-verify-tests/Inputs/non-default-prefix.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/non-default-prefix.c.expected delete mode 100644 clang/test/utils/update-verify-tests/Inputs/update-same-line.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/update-same-line.c.expected delete mode 100644 clang/test/utils/update-verify-tests/Inputs/update-single-check.c delete mode 100644 clang/test/utils/update-verify-tests/Inputs/update-single-check.c.expected delete mode 100644 clang/test/utils/update-verify-tests/duplicate-diag.test delete mode 100644 clang/test/utils/update-verify-tests/infer-indentation.test delete mode 100644 clang/test/utils/update-verify-tests/leave-existing-diags.test delete mode 100644 clang/test/utils/update-verify-tests/lit.local.cfg delete mode 100644 clang/test/utils/update-verify-tests/multiple-errors.test delete mode 100644 clang/test/utils/update-ver
[clang] Revert update-verify-tests.py (PR #109171)
https://github.com/hnrklssn closed https://github.com/llvm/llvm-project/pull/109171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [BoundsSafety][NFC] Specify taking address of a variable referred to by '__counted_by' is forbidden (PR #106147)
@@ -759,7 +759,24 @@ relationship must hold even after any of these related
variables are updated. To
this end, the model requires that assignments to ``buf`` and ``count`` must be
side by side, with no side effects between them. This prevents ``buf`` and
``count`` from temporarily falling out of sync due to updates happening at a
-distance.
+distance. In addition, taking address of ``count`` is not allowed in order to
+prevent the programmers from updating the ``count`` through the pointer, which
+will evade the necessary checks to make ``count`` and ``buf`` in sync.
+
+.. code-block:: c
+
+ struct counted_buf {
+ int *__counted_by(count) buf;
+ size_t count;
+ };
+
+ void foo(struct counted_buf *p) {
+ int *pointer_to_count = &p->count; // error: variable referred to by
+ // '__counted_by' cannot be pointed to by any other variable; exception
is
+ // when the pointer is passed as a compatible argument to a function.
hnrklssn wrote:
I think we should define what a compatible argument is
https://github.com/llvm/llvm-project/pull/106147
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [BoundsSafety][NFC] Specify taking address of a variable referred to by '__counted_by' is forbidden (PR #106147)
https://github.com/hnrklssn edited https://github.com/llvm/llvm-project/pull/106147 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [BoundsSafety][NFC] Specify taking address of a variable referred to by '__counted_by' is forbidden (PR #106147)
https://github.com/hnrklssn commented: Overall looks good, just a small clarification https://github.com/llvm/llvm-project/pull/106147 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/108425
>From 6cdb6bec945725d69d1073deeb53b7485c7e40cd Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Wed, 28 Aug 2024 23:30:49 -0700
Subject: [PATCH 1/3] [Utils] Add --update-tests to lit
This adds a flag to lit for detecting and updating failing tests when
possible to do so automatically. The flag uses a plugin architecture
where config files can add additional auto-updaters for the types of
tests in the test suite. When a test fails with --update-tests enabled
lit passes the test RUN invocation and output to each registered test
updater until one of them signals that it updated the test. As such it
is the responsibility of the test updater to only update tests where it
is reasonably certain that it will actually fix the test, or come close
to doing so.
---
llvm/utils/lit/lit/LitConfig.py| 3 +++
llvm/utils/lit/lit/TestRunner.py | 12
llvm/utils/lit/lit/cl_arguments.py | 6 ++
llvm/utils/lit/lit/llvm/config.py | 5 +
llvm/utils/lit/lit/main.py | 1 +
5 files changed, 27 insertions(+)
diff --git a/llvm/utils/lit/lit/LitConfig.py b/llvm/utils/lit/lit/LitConfig.py
index 5dc712ae28370c..198a2bf3172330 100644
--- a/llvm/utils/lit/lit/LitConfig.py
+++ b/llvm/utils/lit/lit/LitConfig.py
@@ -38,6 +38,7 @@ def __init__(
parallelism_groups={},
per_test_coverage=False,
gtest_sharding=True,
+update_tests=False,
):
# The name of the test runner.
self.progname = progname
@@ -89,6 +90,8 @@ def __init__(
self.parallelism_groups = parallelism_groups
self.per_test_coverage = per_test_coverage
self.gtest_sharding = bool(gtest_sharding)
+self.update_tests = update_tests
+self.test_updaters = []
@property
def maxIndividualTestTime(self):
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index a1785073547ad0..7556433b83452c 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1190,6 +1190,18 @@ def executeScriptInternal(
str(result.timeoutReached),
)
+if litConfig.update_tests:
+for test_updater in litConfig.test_updaters:
+try:
+update_output = test_updater(result)
+except Exception as e:
+out += f"Exception occurred in test updater: {e}"
+continue
+if update_output:
+for line in update_output.splitlines():
+out += f"# {line}\n"
+break
+
return out, err, exitCode, timeoutInfo
diff --git a/llvm/utils/lit/lit/cl_arguments.py
b/llvm/utils/lit/lit/cl_arguments.py
index ed78256ee414b4..dcbe553c6d4827 100644
--- a/llvm/utils/lit/lit/cl_arguments.py
+++ b/llvm/utils/lit/lit/cl_arguments.py
@@ -204,6 +204,12 @@ def parse_args():
action="store_true",
help="Exit with status zero even if some tests fail",
)
+execution_group.add_argument(
+"--update-tests",
+dest="update_tests",
+action="store_true",
+help="Try to update regression tests to reflect current behavior, if
possible",
+)
execution_test_time_group = execution_group.add_mutually_exclusive_group()
execution_test_time_group.add_argument(
"--skip-test-time-recording",
diff --git a/llvm/utils/lit/lit/llvm/config.py
b/llvm/utils/lit/lit/llvm/config.py
index 5f762ec7f3514a..c05ec1664d4c45 100644
--- a/llvm/utils/lit/lit/llvm/config.py
+++ b/llvm/utils/lit/lit/llvm/config.py
@@ -64,12 +64,17 @@ def __init__(self, lit_config, config):
self.with_environment("_TAG_REDIR_ERR", "TXT")
self.with_environment("_CEE_RUNOPTS", "FILETAG(AUTOCVT,AUTOTAG)
POSIX(ON)")
+if lit_config.update_tests:
+self.use_lit_shell = True
+
# Choose between lit's internal shell pipeline runner and a real shell.
# If LIT_USE_INTERNAL_SHELL is in the environment, we use that as an
# override.
lit_shell_env = os.environ.get("LIT_USE_INTERNAL_SHELL")
if lit_shell_env:
self.use_lit_shell = lit.util.pythonize_bool(lit_shell_env)
+if not self.use_lit_shell and lit_config.update_tests:
+print("note: --update-tests is not supported when using
external shell")
if not self.use_lit_shell:
features.add("shell")
diff --git a/llvm/utils/lit/lit/main.py b/llvm/utils/lit/lit/main.py
index 24ba804f0c363f..745e376de7d529 100755
--- a/llvm/utils/lit/lit/main.py
+++ b/llvm/utils/lit/lit/main.py
@@ -42,6 +42,7 @@ def main(builtin_params={}):
config_prefix=opts.configPrefix,
per_test_coverage=opts.per_test_coverage,
gtest_sharding=opts.gtest_sharding,
+update_tests=opts.update_tests,
)
discovered_tests = lit.discov
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
hnrklssn wrote: > Thank you. This pretty much LGTM, but it just occurred to me that there's a > docs/CommandGuide/lit.rst which should be updated to document the new option. Done > As update-verify-tests was reverted, can you rebase this to just the > update_test_checks support? On the LLVM side, we definitely want to have this > feature. and done https://github.com/llvm/llvm-project/pull/108425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/108425
>From 6cdb6bec945725d69d1073deeb53b7485c7e40cd Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson"
Date: Wed, 28 Aug 2024 23:30:49 -0700
Subject: [PATCH 1/2] [Utils] Add --update-tests to lit
This adds a flag to lit for detecting and updating failing tests when
possible to do so automatically. The flag uses a plugin architecture
where config files can add additional auto-updaters for the types of
tests in the test suite. When a test fails with --update-tests enabled
lit passes the test RUN invocation and output to each registered test
updater until one of them signals that it updated the test. As such it
is the responsibility of the test updater to only update tests where it
is reasonably certain that it will actually fix the test, or come close
to doing so.
---
llvm/utils/lit/lit/LitConfig.py| 3 +++
llvm/utils/lit/lit/TestRunner.py | 12
llvm/utils/lit/lit/cl_arguments.py | 6 ++
llvm/utils/lit/lit/llvm/config.py | 5 +
llvm/utils/lit/lit/main.py | 1 +
5 files changed, 27 insertions(+)
diff --git a/llvm/utils/lit/lit/LitConfig.py b/llvm/utils/lit/lit/LitConfig.py
index 5dc712ae28370c..198a2bf3172330 100644
--- a/llvm/utils/lit/lit/LitConfig.py
+++ b/llvm/utils/lit/lit/LitConfig.py
@@ -38,6 +38,7 @@ def __init__(
parallelism_groups={},
per_test_coverage=False,
gtest_sharding=True,
+update_tests=False,
):
# The name of the test runner.
self.progname = progname
@@ -89,6 +90,8 @@ def __init__(
self.parallelism_groups = parallelism_groups
self.per_test_coverage = per_test_coverage
self.gtest_sharding = bool(gtest_sharding)
+self.update_tests = update_tests
+self.test_updaters = []
@property
def maxIndividualTestTime(self):
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index a1785073547ad0..7556433b83452c 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1190,6 +1190,18 @@ def executeScriptInternal(
str(result.timeoutReached),
)
+if litConfig.update_tests:
+for test_updater in litConfig.test_updaters:
+try:
+update_output = test_updater(result)
+except Exception as e:
+out += f"Exception occurred in test updater: {e}"
+continue
+if update_output:
+for line in update_output.splitlines():
+out += f"# {line}\n"
+break
+
return out, err, exitCode, timeoutInfo
diff --git a/llvm/utils/lit/lit/cl_arguments.py
b/llvm/utils/lit/lit/cl_arguments.py
index ed78256ee414b4..dcbe553c6d4827 100644
--- a/llvm/utils/lit/lit/cl_arguments.py
+++ b/llvm/utils/lit/lit/cl_arguments.py
@@ -204,6 +204,12 @@ def parse_args():
action="store_true",
help="Exit with status zero even if some tests fail",
)
+execution_group.add_argument(
+"--update-tests",
+dest="update_tests",
+action="store_true",
+help="Try to update regression tests to reflect current behavior, if
possible",
+)
execution_test_time_group = execution_group.add_mutually_exclusive_group()
execution_test_time_group.add_argument(
"--skip-test-time-recording",
diff --git a/llvm/utils/lit/lit/llvm/config.py
b/llvm/utils/lit/lit/llvm/config.py
index 5f762ec7f3514a..c05ec1664d4c45 100644
--- a/llvm/utils/lit/lit/llvm/config.py
+++ b/llvm/utils/lit/lit/llvm/config.py
@@ -64,12 +64,17 @@ def __init__(self, lit_config, config):
self.with_environment("_TAG_REDIR_ERR", "TXT")
self.with_environment("_CEE_RUNOPTS", "FILETAG(AUTOCVT,AUTOTAG)
POSIX(ON)")
+if lit_config.update_tests:
+self.use_lit_shell = True
+
# Choose between lit's internal shell pipeline runner and a real shell.
# If LIT_USE_INTERNAL_SHELL is in the environment, we use that as an
# override.
lit_shell_env = os.environ.get("LIT_USE_INTERNAL_SHELL")
if lit_shell_env:
self.use_lit_shell = lit.util.pythonize_bool(lit_shell_env)
+if not self.use_lit_shell and lit_config.update_tests:
+print("note: --update-tests is not supported when using
external shell")
if not self.use_lit_shell:
features.add("shell")
diff --git a/llvm/utils/lit/lit/main.py b/llvm/utils/lit/lit/main.py
index 24ba804f0c363f..745e376de7d529 100755
--- a/llvm/utils/lit/lit/main.py
+++ b/llvm/utils/lit/lit/main.py
@@ -42,6 +42,7 @@ def main(builtin_params={}):
config_prefix=opts.configPrefix,
per_test_coverage=opts.per_test_coverage,
gtest_sharding=opts.gtest_sharding,
+update_tests=opts.update_tests,
)
discovered_tests = lit.discov
[clang] [-Wunsafe-buffer-usage] Warning Libc functions (PR #101583)
@@ -443,6 +443,426 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
return false;
}
+AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
+ return Node.getNumArgs() == Num;
+}
+
+namespace libc_func_matchers {
+// Under `libc_func_matchers`, define a set of matchers that match unsafe
+// functions in libc and unsafe calls to them.
+
+// A tiny parser to strip off common prefix and suffix of libc function names
+// in real code.
+//
+// Given a function name, `matchName` returns `CoreName` according to the
+// following grammar:
+//
+// LibcName := CoreName | CoreName + "_s"
+// MatchingName := "__builtin_" + LibcName |
+// "__builtin___" + LibcName + "_chk" |
+// "__asan_" + LibcName
+//
+struct LibcFunNamePrefixSuffixParser {
+ StringRef matchName(StringRef FunName, bool isBuiltin) {
+// Try to match __builtin_:
+if (isBuiltin && FunName.starts_with("__builtin_"))
+ // Then either it is __builtin_LibcName or __builtin___LibcName_chk or
+ // no match:
+ return matchLibcNameOrBuiltinChk(
+ FunName.drop_front(10 /* truncate "__builtin_" */));
+// Try to match __asan_:
+if (FunName.starts_with("__asan_"))
+ return matchLibcName(FunName.drop_front(7 /* truncate of "__asan_" */));
+return matchLibcName(FunName);
+ }
+
+ // Parameter `Name` is the substring after stripping off the prefix
+ // "__builtin_".
+ StringRef matchLibcNameOrBuiltinChk(StringRef Name) {
+if (Name.starts_with("__") && Name.ends_with("_chk"))
+ return matchLibcName(
+ Name.drop_front(2).drop_back(4) /* truncate "__" and "_chk" */);
+return matchLibcName(Name);
+ }
+
+ StringRef matchLibcName(StringRef Name) {
+if (Name.ends_with("_s"))
+ return Name.drop_back(2 /* truncate "_s" */);
+return Name;
+ }
+};
+
+// A pointer type expression is known to be null-terminated, if it has the
+// form: E.c_str(), for any expression E of `std::string` type.
+static bool isNullTermPointer(const Expr *Ptr) {
+ if (isa(Ptr->IgnoreParenImpCasts()))
+return true;
+ if (isa(Ptr->IgnoreParenImpCasts()))
+return true;
+ if (auto *MCE = dyn_cast(Ptr->IgnoreParenImpCasts())) {
+const CXXMethodDecl *MD = MCE->getMethodDecl();
+const CXXRecordDecl *RD = MCE->getRecordDecl()->getCanonicalDecl();
+
+if (MD && RD && RD->isInStdNamespace())
+ if (MD->getName() == "c_str" && RD->getName() == "basic_string")
+return true;
+ }
+ return false;
+}
+
+// Return true iff at least one of following cases holds:
+// 1. Format string is a literal and there is an unsafe pointer argument
+// corresponding to an `s` specifier;
+// 2. Format string is not a literal and there is least an unsafe pointer
+// argument (including the formatter argument).
+//
+// `UnsafeArg` is the output argument that will be set only if this function
+// returns true.
+static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg,
+ const unsigned FmtArgIdx, ASTContext &Ctx,
+ bool isKprintf = false) {
+ class StringFormatStringHandler
+ : public analyze_format_string::FormatStringHandler {
+const CallExpr *Call;
+unsigned FmtArgIdx;
+const Expr *&UnsafeArg;
+
+ public:
+StringFormatStringHandler(const CallExpr *Call, unsigned FmtArgIdx,
+ const Expr *&UnsafeArg)
+: Call(Call), FmtArgIdx(FmtArgIdx), UnsafeArg(UnsafeArg) {}
+
+bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
+ const char *startSpecifier,
+ unsigned specifierLen,
+ const TargetInfo &Target) override {
+ if (FS.getConversionSpecifier().getKind() ==
+ analyze_printf::PrintfConversionSpecifier::sArg) {
+unsigned ArgIdx = FS.getPositionalArgIndex() + FmtArgIdx;
+
+if (0 < ArgIdx && ArgIdx < Call->getNumArgs())
+ if (!isNullTermPointer(Call->getArg(ArgIdx))) {
+UnsafeArg = Call->getArg(ArgIdx); // output
+// returning false stops parsing immediately
+return false;
+ }
+ }
+ return true; // continue parsing
+}
+ };
+
+ const Expr *Fmt = Call->getArg(FmtArgIdx);
+
+ if (auto *SL = dyn_cast(Fmt->IgnoreParenImpCasts())) {
+StringRef FmtStr = SL->getString();
+StringFormatStringHandler Handler(Call, FmtArgIdx, UnsafeArg);
+
+return analyze_format_string::ParsePrintfString(
+Handler, FmtStr.begin(), FmtStr.end(), Ctx.getLangOpts(),
+Ctx.getTargetInfo(), isKprintf);
+ }
+ // If format is not a string literal, we cannot analyze the format string.
+ // In this case, this call is considered unsafe if at least one argument
+ // (including the format argument) is unsafe pointer.
+ return llvm::any_of(
+ llvm::make_range(Call->arg_begin(
[clang] Reland "[Utils] add update-verify-tests.py" (#108630)" (PR #108658)
hnrklssn wrote:
> In fact, we used to have guidance that only the very first instance of the
> diagnostic should be spelled out fully, and all subsequent ones should be
> limited to just the bare minimum needed to identify what the diagnostic is.
> That's why there are hundreds of instances of things like // expected-note
> {{here}}.
Side note: In general I think the fact that these tests pass if the check
implicitly match on a subset of the diagnostic does more harm than good. When
test authors actively want to do that the can always use the regex feature, but
I think the default should be to match the exact string. When there are similar
diagnostics or shorter/longer version it's too much of a footgun to
accidentally have shorter diagnostic match the longer version as well. When I
review, i prefer the test to give me a picture as close to the exact output as
possible, so I can evaluate the user friendliness. I think it's a massive
shortcoming that we don't check the order in which notes are emitted, or which
range is highlighted, because reviewers don't get the full picture without
checking out the branch and manually compiling test cases.
> Finally, as @Endilll pointed out, there are tests which have multiple
> (sometime numerous) RUN lines where the output is expected to differ between
> RUN lines; we typically ask authors to use custom verify prefixes for those
> tests, but crafting those can sometimes be an art form (and other times is
> very straightforward).
Again, this script doesn't run on tests with multiple prefixes. It's meant for
test cases which are trivial to update, of which there are many.
> Writing tests is not a mechanical operation, it requires you to think about
> what you need to test as well as think about how you test it. I think the
> same is less true for things like testing LLVM IR because that tends to be
> more routine pattern matching, which typically requires less stylistic
> choices.
Sometimes it is, sometimes it isn't. Even when it isn't just having the checks
added to the test file in approximately the final location for me to move
around is a massive boon compared to manually adding them all.
> Given the amount of push-back, I think we should probably revert the changes
> here so we can discuss with a bit wider of an audience than weighed in on the
> original PR.
Yup, will revert.
> So another huge side note: I've been pushing REALLY hard for us to use the
> bookmark feature more aggressively for notes. So I've been asking authors to
> do THAT as well.
I appreciate that, it can be really tricky to get a feel for the flow of notes,
especially when a diagnostic has multiple notes attached. The test sadly
doesn't check that the order of the notes emitted is actually sensical, for
example. I don't know the solution, but I think there is some inherent flaw
when it comes to how `-verify` tests operate currently, in that you need to
trust that the author formatted the test to accurately reflect reality.
https://github.com/llvm/llvm-project/pull/108658
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Utils] add update-verify-tests.py" (#108630)" (PR #108658)
hnrklssn wrote:
> > >
> >
> >
> > +1, it's why I stopped recommending this approach in my reviews.
>
> While I agree for the most part, there are quite a few diagnostics that are
> absurdly verbose (particularly ones with "did you mean to FLOOP it?"). I
> think partial matches are good/fine, but they need enough of the diag to not
> be ambiguous.
I don't necessarily oppose the use of truncated checks where it makes sense, I
oppose the implicit default on the matcher's part. An
`expected-note-re{{.*FLOOP.*}}` would be a reasonable explicit opt-in. A new
mode like `expected-note-part{{FLOOP}}` would be even better, making the old
mode behave like `expected-note-re{{^FLOOP$}}`. As an (admittedly extreme)
example of footgunnery with this implicit behaviour, we had bug downstream
where there were two versions of an error, something like this: `cannot use FOO
in context asdf` vs `cannot use FOO in context asdf; did you mean to use BAR?`.
The `; did you mean to use BAR?` version was accidentally emitted in some cases
where it didn't make sense to do so, but the tests still passed because the
diagnostic *did* contain `cannot use FOO in context asdf`.
https://github.com/llvm/llvm-project/pull/108658
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix a bug and suppress libc warnings for C files (PR #109496)
https://github.com/hnrklssn edited https://github.com/llvm/llvm-project/pull/109496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix a bug and suppress libc warnings for C files (PR #109496)
@@ -789,7 +791,7 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg, if (!FristParmTy->isPointerType()) return false; // possibly some user-defined printf function - QualType FirstPteTy = (cast(FristParmTy))->getPointeeType(); + QualType FirstPteTy = FristParmTy->getAs()->getPointeeType(); hnrklssn wrote: Nit: FristParmTy -> FirstParmTy https://github.com/llvm/llvm-project/pull/109496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix a bug and suppress libc warnings for C files (PR #109496)
https://github.com/hnrklssn commented: You may want to add some tests with annotated pointers to verify that you treat them correctly, e.g. `_Nullable` https://github.com/llvm/llvm-project/pull/109496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CGRecordLayout] Remove dependency on isZeroSize (PR #96422)
hnrklssn wrote:
This change leads to a crash in the following case:
```
struct S {
};
union U {
struct S s;
int x;
};
void foo() {
union U bar = {};
}
```
`isEmptyRecordForLayout` returns false for `union U` because the recursive call
for `U::x` returns false. This means we call `Init->HasSideEffects()` despite
`Init` being null (because `ILE->getNumInits()` is `0`).
https://github.com/llvm/llvm-project/pull/96422
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Utils] add update-verify-tests.py" (#108630)" (PR #108658)
hnrklssn wrote: > > The ; did you mean to use BAR? version was accidentally emitted in some > > cases where it didn't make sense to do so, but the tests still passed > > because the diagnostic did contain cannot use FOO in context asdf. > > > > There's a `FileCheck` flag to enforce this: `--match-full-lines` Clang diagnostic tests don't use FileCheck, instead clang has a -verify flag. https://github.com/llvm/llvm-project/pull/108658 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Utils] add update-verify-tests.py" (#108630)" (PR #108658)
hnrklssn wrote: > As a CFE reviewer, I very much hate the existence of this script (and the > CodeGen equivalents, but that is a separate discussion). > > This existing means I can no longer trust that a test is a reflection of what > the author intended to write, as they might have just run this and "YOLO'ed" > it away. So now I have to be 100x more careful reviewing tests, which means > significantly worse review bandwidth. This is the same problem I have with AI > generated code/tests, as this is basically only a 1/2 step above, "Hey > Google, write me a lit test for this". > > @cor3ntin and @AaronBallman and @Endilll . That was always true, in the sense that there's nothing preventing the author from doing the exact same thing: looking at the output from the `clang -verify` failure and adding/removing expected diagnostics until it matches. In fact, even without this script that is the most mindless way of doing it, and I'm sure it already happens a lot. In either way, authors should review their own changes first. I would argue that this script is significantly less harmful than any FileCheck generator script, because given an input program there is much less flexibility in how to structure the checks. With FileCheck you may not want to check every line, since some of them aren't directly relevant to the thing you're testing, instead making the test more fragile and harder to review through bloat. So creating FileCheck check lines is a bit of an art. But for `clang -verify` you have to add checkers for exactly the emitted diagnostics. No more or less. So the only real question is on which line to place them. That said my main goal is not to make creating tests easier (although it of course also does that, and I don't know that I agree that it's harmful to lower the threshold to create tests), but to make it easier to batch update many/large breaking tests. We have many tests downstream that while testing features that don't yet exist upstream, are affected by new diagnostics added upstream. Handling many tests breaking all at once because of upstream changes is easier if it doesn't require a ton of `sed`-fu. Similar interactions happen when cherry-picking a patch from a dev branch to one or multiple release branches, that may not contain the exact same set of diagnostics. I could of course keep this script downstream, but we are not the only ones with downstream changes and I think others will find it helpful. And that's for a completely new script: keeping something like #108425 downstream only would be result in merge conflicts. Furthermore, I have observed how existing regression tests create friction to improving phrasing of existing diagnostics. Old diagnostics often appear in many many tests, so while the code change to improving the diagnostic may be quick, you know it'll take significant time to update all of the regression tests using the old phrasing. So you don't do that drive-by improvement, because that wasn't your goal anyways. I think there are a ton of diagnostics in clang that could be improved to be clearer, but when the improvement is minor and friction is high we get stuck with the "good enough". https://github.com/llvm/llvm-project/pull/108658 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
@@ -5125,24 +5127,54 @@ QualType ASTContext::getUnresolvedUsingType(
QualType ASTContext::getAttributedType(attr::Kind attrKind,
QualType modifiedType,
- QualType equivalentType) const {
+ QualType equivalentType,
+ const Attr *attr) const {
llvm::FoldingSetNodeID id;
AttributedType::Profile(id, attrKind, modifiedType, equivalentType);
hnrklssn wrote:
`AttributedType` is uniqued based on `attrKind`, but not `attr`. So calling
this function may return a type with a pointer to a different `Attr` (although
of the same kind). Is this a problem? Do we want to fully unique
`AttributedType` based on it's `Attr*`?
The other option I believe is to fetch the `Attr*` from `AttributedTypeLoc`
(which you can get through the `TypeSourceInfo` from `Decl` or
`ExplicitCastExpr`) rather than the `AttributedType`, but doing that for
generic expressions gets quite hairy.
https://github.com/llvm/llvm-project/pull/108631
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [BoundsSafety][Sema] Allow counted_by and counted_by_or_null on pointers where the pointee type is incomplete but potentially completable (PR #106321)
@@ -186,4 +218,370 @@ bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr
*E, bool CountInBytes,
return false;
}
+SourceRange Sema::BoundsSafetySourceRangeFor(const CountAttributedType *CATy) {
hnrklssn wrote:
[#108631](https://github.com/llvm/llvm-project/pull/108631) seems to have
similar issues with wanting to access `AttributedTypeLoc` when all they have is
`AttributedType`. We might need to improve the infrastructure for accessing
`TypeLoc`s in a more ergonomic fashion.
https://github.com/llvm/llvm-project/pull/106321
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[Utils] add update-verify-tests.py" (#108630)" (PR #108658)
hnrklssn wrote: > > But for clang -verify you have to add checkers for exactly the emitted > > diagnostics. No more or less. So the only real question is on which line to > > place them. > > I don't think it's that simple at least for some tests. We have tests that > run in `-verify` mode multiple times with different combinations of compiler > options. Such tests might rely on custom prefixes > (`-verify=prefix1,prefix2`), and updating them automatically can easily yield > incorrect result. As an example how more complicated tests might look like, > consider C++ DR test suite. Here's one example: > > https://github.com/llvm/llvm-project/blob/ca0613e0fcef2a9972f2802318201f8272e74693/clang/test/CXX/drs/cwg3xx.cpp#L779-L782 This script doesn't support test failures in multiple prefixes or regex matchers for that reason, because I could not find a reliable way to update those failures without manual intervention. > > That said my main goal is not to make creating tests easier (although it of > > course also does that, and I don't know that I agree that it's harmful to > > lower the threshold to create tests), but to make it easier to batch update > > many/large breaking tests. We have many tests downstream that while testing > > features that don't yet exist upstream, are affected by new diagnostics > > added upstream. Handling many tests breaking all at once because of > > upstream changes is easier if it doesn't require a ton of sed-fu. Similar > > interactions happen when cherry-picking a patch from a dev branch to one or > > multiple release branches, that may not contain the exact same set of > > diagnostics. I could of course keep this script downstream, but we are not > > the only ones with downstream changes and I think others will find it > > helpful. And that's for a completely new script: keeping something like > > #108425 downstream only would be result in merge conflicts. > > I don't intend to dismiss the pain downstream could have over this, but in my > opinion it doesn't justify opening floodgates of automated test updates > upstream. At the moment I don't have a strong opinion on infrastructure that > supports such automated updates, like #108425 you mentioned. > > > Furthermore, I have observed how existing regression tests create friction > > to improving phrasing of existing diagnostics. Old diagnostics often appear > > in many many tests, so while the code change to improving the diagnostic > > may be quick, you know it'll take significant time to update all of the > > regression tests using the old phrasing. > > I agree that the pain is there. But again, I don't think the end justifies > the mean. That's fair, we disagree on this point. I think responsible use of automatic test updaters is reasonable, and we shouldn't prevent people from using it just because it _could_ be abused. I'm sympathetic that there may be many low quality PRs to wade through, but I'd rather let the number of open PRs continue to grow and let more people step up to the task of reviewing as it becomes more apparent that we need more reviewers. Artificially hampering developer productivity just because it's not the bottleneck is hiding the symptom rather than fixing the problem (too few reviewers in comparison to the number of PRs). I do want to stress though, that this script handles pretty **trivial** updates. Yes, it doesn't force people to look at the changes they're submitting, but compared to existing scripts this is significantly less risky. I don't want to resort to whataboutism, but I do think we need to be consistent if we're going to ban test update scripts. > > So you don't do that drive-by improvement, because that wasn't your goal > > anyways. > > Drive-by fix that makes you update a lot of test would introduce a lot of > noise into your PR anyway, making it noticeably harder to review. Of course the drive-by fix would have to be submitted as a separate PR in that case, but that's quite straightforward even after the fact when you have a script like this: extract the implementation into a separate commit, and rerun the test updater on both branches to bring over the test updates to the new PR and undo the noise on the original PR. > I believe the script addresses the wrong problem here. There's no getting around that the tests need to be updated if diagnostics are updated. Lack of reviewers doesn't mean developer issues can't also be addressed. https://github.com/llvm/llvm-project/pull/108658 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement __attribute__((format_matches)) (PR #116708)
=?utf-8?q?Félix?= Cloutier , =?utf-8?q?Félix?= Cloutier , =?utf-8?q?Félix?= Cloutier , =?utf-8?q?Félix?= Cloutier , =?utf-8?q?Félix?= Cloutier , =?utf-8?q?Félix?= Cloutier , =?utf-8?q?Félix?= Cloutier , =?utf-8?q?Félix?= Cloutier , =?utf-8?q?Félix?= Cloutier , =?utf-8?q?Félix?= Cloutier , =?utf-8?q?Félix?= Cloutier Message-ID: In-Reply-To: https://github.com/hnrklssn closed https://github.com/llvm/llvm-project/pull/116708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)
@@ -8663,31 +8663,95 @@ static const RecordDecl
*GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD) {
return RD;
}
-static bool
-CheckCountExpr(Sema &S, FieldDecl *FD, Expr *E,
- llvm::SmallVectorImpl &Decls) {
+enum class CountedByInvalidPointeeTypeKind {
+ INCOMPLETE,
+ SIZELESS,
+ FUNCTION,
+ FLEXIBLE_ARRAY_MEMBER,
+ VALID,
+};
+
+static bool CheckCountedByAttrOnField(
+Sema &S, FieldDecl *FD, Expr *E,
+llvm::SmallVectorImpl &Decls) {
+ // Check the context the attribute is used in
+
if (FD->getParent()->isUnion()) {
S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_in_union)
<< FD->getSourceRange();
return true;
}
- if (!E->getType()->isIntegerType() || E->getType()->isBooleanType()) {
-S.Diag(E->getBeginLoc(), diag::err_counted_by_attr_argument_not_integer)
-<< E->getSourceRange();
+ const auto FieldTy = FD->getType();
+ if (!FieldTy->isArrayType() && !FieldTy->isPointerType()) {
+S.Diag(FD->getBeginLoc(),
+ diag::err_counted_by_attr_not_on_ptr_or_flexible_array_member)
+<< FD->getLocation();
return true;
}
LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
LangOptions::StrictFlexArraysLevelKind::IncompleteOnly;
-
- if (!Decl::isFlexibleArrayMemberLike(S.getASTContext(), FD, FD->getType(),
+ if (FieldTy->isArrayType() &&
+ !Decl::isFlexibleArrayMemberLike(S.getASTContext(), FD, FieldTy,
StrictFlexArraysLevel, true)) {
-// The "counted_by" attribute must be on a flexible array member.
-SourceRange SR = FD->getLocation();
-S.Diag(SR.getBegin(),
- diag::err_counted_by_attr_not_on_flexible_array_member)
-<< SR;
+S.Diag(FD->getBeginLoc(),
+ diag::err_counted_by_attr_on_array_not_flexible_array_member)
hnrklssn wrote:
I believe the question is not why only flexible array members are allowed the
attribute, but why only incomplete arrays are. Some old projects use this as
syntax as incomplete array:
```
struct S {
int len;
char fam[0];
};
```
Would converting the above to an incomplete array like this be an issue in
those projects? It seems to me that if you're introducing `__counted_by`,
removing any old integer constant isn't much more extra work.
```
struct S {
int len;
char fam[__counted_by(len)];
};
```
The reason is that a complete array already has a bound, which we use for
bounds checking with `-fbounds-safety` enabled. In fact, an array parameter
like this will decay into a `int * __counted_by(len)` pointer:
```
void foo(int len, int arr[len]);
```
If you allow annotating these arrays with `__counted_by` also, you effectively
end up with two separate sources of bounds info.
https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)
https://github.com/hnrklssn edited https://github.com/llvm/llvm-project/pull/93121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix FP -Wformat in functions with 2+ attribute((format)) (PR #129954)
https://github.com/hnrklssn closed https://github.com/llvm/llvm-project/pull/129954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] update_test_checks: add new --filter-out-after option (PR #129739)
https://github.com/hnrklssn approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/129739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
