@@ -74,16 +75,31 @@ class formatv_object_base {
static std::pair
splitLiteralAndReplacement(StringRef Fmt);
- formatv_object_base(StringRef Fmt,
+ formatv_object_base(StringRef Fmt, bool ValidateNumArgs,
ArrayRef Adapters)
- : Fmt(Fmt), Adapte
https://github.com/jurahul edited
https://github.com/llvm/llvm-project/pull/105745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/jurahul edited
https://github.com/llvm/llvm-project/pull/105745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/105745
>From 9ae8a0361b0d26cf29cc4547658baec0c2654c89 Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Thu, 22 Aug 2024 08:47:02 -0700
Subject: [PATCH] [Support] Detect invalid formatv() calls
- Detect formatv() calls
https://github.com/jurahul ready_for_review
https://github.com/llvm/llvm-project/pull/105745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
jurahul wrote:
Hi all, this is related to
https://discourse.llvm.org/t/adding-argument-count-validation-for-formatv/80876/1
I still have formatv() and formatvv() functions in the code, but only a handful
instances. So, if this looks ok overall, I will go ahead and rename formatvv()
to formatv
jurahul wrote:
> CI failed FYI.
Yeah, it looked like an unrelated failure. Windows CI passed.
https://github.com/llvm/llvm-project/pull/105745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/ll
@@ -67,92 +68,123 @@ formatv_object_base::parseReplacementItem(StringRef Spec) {
StringRef Options;
size_t Index = 0;
RepString = RepString.trim();
- if (RepString.consumeInteger(0, Index)) {
-assert(false && "Invalid replacement sequence index!");
-return Replac
jurahul wrote:
> compile-time tests with clang for example
I can check. How do I run them?
https://github.com/llvm/llvm-project/pull/105745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-
https://github.com/jurahul edited
https://github.com/llvm/llvm-project/pull/105745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/jurahul edited
https://github.com/llvm/llvm-project/pull/105745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
jurahul wrote:
> > compile-time tests with clang for example
>
> I can check. How do I run them?
I am wondering if running mlir-tblgen is a better benchmark, given that
formatv() is widely used there (as opposed to clang), in case this measurement
has to be done manually.
https://github.com/
jurahul wrote:
I did 2 sets of experiments, but data wise I am inconclusive if this causes a
real compile time regression.
1. Build MLIR verbose and capture all mlir-gen command lines to a file:
ninja -C build check-mlir --verbose | tee build_log.txt
grep "NATIVE/bin/mlir-tblgen " build_l
jurahul wrote:
Yeah. For the unbalanced {{ I actually committed a change to fail in
release builds as well by printing an error message as formatv() output.
The only issue with debug only is that many folks build tablegen in release
mode, so those bad uses may not get flagged. But as long as the
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/105745
>From d805dcdc44d22bb21d086b186cc7f644323f68ef Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Thu, 22 Aug 2024 08:47:02 -0700
Subject: [PATCH] [Support] Detect invalid formatv() calls
- Detect formatv() calls
jurahul wrote:
I have uploaded a new version where the validation is enabled only in debug
builds, and also added a benchmark for formatv(). On my machine, the benchmark
results are as follows:
```
Old:
BM_FormatVariadic_mean 3427456 ns 3427426 ns 10
New
BM_FormatVariadic_
jurahul wrote:
Looks like the CI builds are release but with LLVM_ENABLE_ASSERTION=ON, so this
validation code will be enabled and potential bad usage will be flagged as well
going forward as long as it's exercised during the build or test.
https://github.com/llvm/llvm-project/pull/105745
@@ -655,7 +655,7 @@ DWARFUnit::GetDIE(dw_offset_t die_offset) {
if (!ContainsDIEOffset(die_offset)) {
GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
-"GetDIE for DIE {0:x16} is outside of its CU {0:x16}", die_offset,
+"GetDIE for DIE {0:x
jurahul wrote:
> LGTM
Thanks. I'll split out an additional PR for the independent fixes, and then in
this one I'll change ENABLE_VALIDATION back to 0 and also remove the
`formatvv()` and use `formatv(false` instead so we just have one function name.
https://github.com/llvm/llvm-project/pull/1
https://github.com/jurahul created
https://github.com/llvm/llvm-project/pull/106454
Fix several uses of formatv() that would be flagged as invalid by an upcoming
change that will add additional validation to formatv().
>From b97456af86580c7f2d4473fbc13d3e345f071486 Mon Sep 17 00:00:00 2001
Fro
jurahul wrote:
Started https://github.com/llvm/llvm-project/pull/106454 for the independent
fixes.
https://github.com/llvm/llvm-project/pull/105745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listin
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/106454
>From 581c47377f77a9d2819a5997b1c0a3c23b2c3346 Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Wed, 28 Aug 2024 13:55:58 -0700
Subject: [PATCH] [NFC] Fix formatv() usage in preparation of validation
Fix severa
https://github.com/jurahul closed
https://github.com/llvm/llvm-project/pull/106454
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/jurahul ready_for_review
https://github.com/llvm/llvm-project/pull/107536
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
jurahul wrote:
Yes, the motivation is const correctness. Please see the discourse thread here:
https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089
https://github.com/llvm/llvm-project/pull/107536
https://github.com/jurahul edited
https://github.com/llvm/llvm-project/pull/107536
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
jurahul wrote:
Done.
https://github.com/llvm/llvm-project/pull/107536
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
jurahul wrote:
@JDevlieghere or @dwblaikie one last ping to comment if consistency of file
names is a strong enough motivation. If not, I will drop this.
https://github.com/llvm/llvm-project/pull/110692
___
lldb-commits mailing list
lldb-commits@lists
jurahul wrote:
Sounds good. thanks.
https://github.com/llvm/llvm-project/pull/110692
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/jurahul closed
https://github.com/llvm/llvm-project/pull/110692
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
jurahul wrote:
@JDevlieghere @cyndyishida @topperc gentle ping. It's mostly mechanical.
https://github.com/llvm/llvm-project/pull/110692
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-comm
jurahul wrote:
more of a hypothetical concern at this point. It's also making file names under
Option consistent, in the sense that we have `Option.h` and `Option.cpp` but
`OptTable.h` `OptSpecifier.h` and `OptTable.cpp`
https://github.com/llvm/llvm-project/pull/110692
___
https://github.com/jurahul created
https://github.com/llvm/llvm-project/pull/110692
Rename option parsing related files from OptXYZ -> OptionXYZ, since Opt could
be confused with optimization and Opt as a short hand for Option is not really
that smaller to warrant its use.
>From 7ee776230f113
https://github.com/jurahul ready_for_review
https://github.com/llvm/llvm-project/pull/110692
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
jurahul wrote:
Even if a variable is defined in the main file, it could conflict with
another one in a library being linked.
In any case, my understanding is that this is a common C++ practice,
thought I do not see it spelled out explicitly in the coding standards doc:
file scoped global variabl
@@ -20,7 +20,7 @@ using namespace mlir::lsp;
LogicalResult mlir::MlirLspServerMain(int argc, char **argv,
DialectRegistry ®istry) {
- llvm::cl::opt inputStyle{
+ static llvm::cl::opt inputStyle{
jurahul wrote:
These are
@@ -119,24 +119,24 @@ int main(int argc, char **argv) {
// options as static variables.. some of which overlap with our options.
llvm::cl::ResetCommandLineParser();
- llvm::cl::opt inputFilename(
+ static llvm::cl::opt inputFilename(
jurahul wrote:
Thes
jurahul wrote:
> > I don't quite follow the motivation for this, can you expand on this in the
> > description please? Right now this seems unnecessary for the changes I can
> > see in MLIR for example.
>
> The linked bug seems to explain it, I think? It seems to be the usual "if
> something
jurahul wrote:
Also, the commit description should be something like: "[NFC] Make file-local
cl::opt global variables static" or something like that
https://github.com/llvm/llvm-project/pull/126243
___
lldb-commits mailing list
lldb-commits@lists.llvm
jurahul wrote:
I understand this is mostly mechanical changes, but wondering if review wise it
will help if its split up into 4-5 PRs. For example, bolt, clang, flag, llvm,
mlir etc.
https://github.com/llvm/llvm-project/pull/126243
___
lldb-commits m
jurahul wrote:
Yeah, that’s why the description said most but not all. We should get link
failures if one of these is made static.
On Fri, Feb 7, 2025 at 7:17 AM Joseph Huber ***@***.***>
wrote:
> ***@***. commented on this pull request.
>
> This should definitely be split up. Also some opt
jurahul wrote:
@arsenm did you intent to approve it without splitting? Is this trivial enough
to not need splitting as long as CI checks pass?
https://github.com/llvm/llvm-project/pull/126243
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
h
jurahul wrote:
You can check the CI logs for the exact command line it uses and replicate
it locally.
On Fri, Feb 7, 2025 at 7:46 AM chrisPyr ***@***.***> wrote:
> OK, I'll do it.
> Except for checking it on CI, is there any other method I can test
> locally? E.g. what options should I add
> I'
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/138174
Rate limit · GitHub
body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe
UI,Helvetica,Arial,sans-s
https://github.com/jurahul edited
https://github.com/llvm/llvm-project/pull/138174
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/138174
>From 278179a35bacc7c70dd1724e7ef7a41e9cdd999a Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Wed, 30 Apr 2025 23:47:37 -0700
Subject: [PATCH] [NFC][Support] Add llvm::uninitialized_copy
Add `llvm::uninitiali
https://github.com/jurahul edited
https://github.com/llvm/llvm-project/pull/138174
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -2981,7 +2981,7 @@ ScalarEvolution::getOrCreateAddExpr(ArrayRef Ops,
static_cast(UniqueSCEVs.FindNodeOrInsertPos(ID, IP));
if (!S) {
const SCEV **O = SCEVAllocator.Allocate(Ops.size());
-std::uninitialized_copy(Ops.begin(), Ops.end(), O);
+llvm::uninitial
@@ -22,8 +22,8 @@ Checksum &Checksum::operator=(const Checksum &checksum) {
}
void Checksum::SetMD5(llvm::MD5::MD5Result md5) {
- const constexpr size_t md5_length = 16;
- std::uninitialized_copy_n(md5.begin(), md5_length, m_checksum.begin());
+ static_assert(sizeof(md5) ==
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/138174
>From 7c67ef6748f8813b7aaa91f8e6b463d9fde57d94 Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Wed, 30 Apr 2025 23:47:37 -0700
Subject: [PATCH] [NFC][Support] Add llvm::uninitialized_copy
Add `llvm::uninitiali
@@ -2038,6 +2038,11 @@ template auto mismatch(R1
&&Range1, R2 &&Range2) {
adl_end(Range2));
}
+template
+auto uninitialized_copy(R1 &&Src, IterTy Dst) {
jurahul wrote:
Thanks, fixed.
https://github.com/llvm/llvm-project/pull/138174
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/138174
>From 46542ce5b946735c2c0a8f65e185761ebbf77073 Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Wed, 30 Apr 2025 23:47:37 -0700
Subject: [PATCH] [NFC][Support] Add llvm::uninitialized_copy
Add `llvm::uninitiali
@@ -22,8 +22,8 @@ Checksum &Checksum::operator=(const Checksum &checksum) {
}
void Checksum::SetMD5(llvm::MD5::MD5Result md5) {
- const constexpr size_t md5_length = 16;
- std::uninitialized_copy_n(md5.begin(), md5_length, m_checksum.begin());
+ static_assert(sizeof(md5) ==
@@ -2981,7 +2981,7 @@ ScalarEvolution::getOrCreateAddExpr(ArrayRef Ops,
static_cast(UniqueSCEVs.FindNodeOrInsertPos(ID, IP));
if (!S) {
const SCEV **O = SCEVAllocator.Allocate(Ops.size());
-std::uninitialized_copy(Ops.begin(), Ops.end(), O);
+llvm::uninitial
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/138174
>From de1b49fc6b8819b591e48b81634567ceeffe5089 Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Wed, 30 Apr 2025 23:47:37 -0700
Subject: [PATCH] [NFC][Support] Add llvm::uninitialized_copy
Add `llvm::uninitiali
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/138174
>From fbe3ca0c2e4d195149f5e3e6b8d32797cf47b9df Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Wed, 30 Apr 2025 23:47:37 -0700
Subject: [PATCH] [NFC][Support] Add llvm::uninitialized_copy
Add `llvm::uninitiali
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/138174
Rate limit · GitHub
body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe
UI,Helvetica,Arial,sans-s
https://github.com/jurahul edited
https://github.com/llvm/llvm-project/pull/138174
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/jurahul ready_for_review
https://github.com/llvm/llvm-project/pull/138174
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/jurahul created
https://github.com/llvm/llvm-project/pull/138174
None
>From d6f69414e3ac5c1a22f6509149609258ef980c13 Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Wed, 30 Apr 2025 23:47:37 -0700
Subject: [PATCH] [NFC][Support] Add llvm::uninitialized_copy
---
clang/incl
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/138174
>From b34e9b6c708dfbe097504804a0a85e1169518911 Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Wed, 30 Apr 2025 23:47:37 -0700
Subject: [PATCH] [NFC][Support] Add llvm::uninitialized_copy
---
clang/include/cl
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/138174
>From 705699c4b1772915f6d8773ce786d5601de0a926 Mon Sep 17 00:00:00 2001
From: Rahul Joshi
Date: Wed, 30 Apr 2025 23:47:37 -0700
Subject: [PATCH] [NFC][Support] Add llvm::uninitialized_copy
Add `llvm::uninitiali
@@ -2981,7 +2981,7 @@ ScalarEvolution::getOrCreateAddExpr(ArrayRef Ops,
static_cast(UniqueSCEVs.FindNodeOrInsertPos(ID, IP));
if (!S) {
const SCEV **O = SCEVAllocator.Allocate(Ops.size());
-std::uninitialized_copy(Ops.begin(), Ops.end(), O);
+llvm::uninitial
@@ -2981,7 +2981,7 @@ ScalarEvolution::getOrCreateAddExpr(ArrayRef Ops,
static_cast(UniqueSCEVs.FindNodeOrInsertPos(ID, IP));
if (!S) {
const SCEV **O = SCEVAllocator.Allocate(Ops.size());
-std::uninitialized_copy(Ops.begin(), Ops.end(), O);
+llvm::uninitial
https://github.com/jurahul edited
https://github.com/llvm/llvm-project/pull/138174
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
jurahul wrote:
> This looks right, but I'd like someone else to go through to make sure it is
> right everywhere.
>
> Also, the md5 change looks... odd and counts a lot on the internal
> representation of md5, so I'm not sure about that one, but at least it is
> right NOW (Since MD5 inherits
https://github.com/jurahul updated
https://github.com/llvm/llvm-project/pull/138174
Rate limit · GitHub
body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe
UI,Helvetica,Arial,sans-s
jurahul wrote:
Thanks @jpienaar. Given I have 2 approvals, will commit it today.
https://github.com/llvm/llvm-project/pull/138174
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
68 matches
Mail list logo
