https://github.com/carlosgalvezp approved this pull request.
LGTM! Worth keeping in mind that this new .clang-format file makes clang-tidy
potentially disconnected from the rest of the repo, in case the top-level file
were to introduce new options. But hopefully everything is baked into the
"L
carlosgalvezp wrote:
The check name is a bit too vague/general, I would call it
`bugprone-derived-method-shadowing-base-method` or similar. You can easily
apply the renaming with the `rename_check.py`. But let's wait until there's
some consensus on this.
https://github.com/llvm/llvm-project/p
https://github.com/carlosgalvezp closed
https://github.com/llvm/llvm-project/pull/132016
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
Yes, the patch can be merged and gets rid of about half the remaining warnings
in system headers! I've removed the condition about checking for the
`SystemHeaders` flag, for consistency with similar code for ignoring the
`Decl`s, which further simplifies the logic.
https:
https://github.com/carlosgalvezp edited
https://github.com/llvm/llvm-project/pull/132016
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp updated
https://github.com/llvm/llvm-project/pull/132016
>From f57920884bdc225e453fd0d5508494c35d87b3c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20G=C3=A1lvez?=
Date: Wed, 19 Mar 2025 12:28:49 +
Subject: [PATCH] [clang-tidy] Skip system macros in
rea
carlosgalvezp wrote:
Let me refresh my memory about this patch and come back :)
https://github.com/llvm/llvm-project/pull/132016
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
Yes, thank you, closing!
https://github.com/llvm/llvm-project/pull/132725
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp closed
https://github.com/llvm/llvm-project/pull/132725
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/155083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp closed
https://github.com/llvm/llvm-project/pull/155015
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp created
https://github.com/llvm/llvm-project/pull/155015
…uidelines-pro-bounds-pointer-arithmetic
Fixes #154907
>From c4f59473a6bac427e393cbc19aee9d485348acee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20G=C3=A1lvez?=
Date: Fri, 22 Aug 2025 19:13:18 +
https://github.com/carlosgalvezp created
https://github.com/llvm/llvm-project/pull/154806
Currently it's hard to find the "general" options, since they are listed in the
middle of the "specific" options. Split them into two categories so they are
easier to find.
This can help in adding a pote
@@ -4,6 +4,9 @@
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
+// SPDX-FileCopyrightText: Portions Copyright 2025 Siemens and/or its
affiliates
+// May 2025 modified by Siemens and/or its affiliates b
https://github.com/carlosgalvezp closed
https://github.com/llvm/llvm-project/pull/154545
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
Thank you for the quick response! Will try that out.
https://github.com/llvm/llvm-project/pull/152911
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
Hi!
It appears this commit breaks our code, with the error:
```
error: use of undeclared identifier '__builtin_ia32_vcvtph2ps256';
```
I could not find any mention of this change in the Release Notes, nor
documentation on how to migrate from it. Could that be added?
http
carlosgalvezp wrote:
Thanks for the review! @zwuis Let me know if I've addressed your comments :)
https://github.com/llvm/llvm-project/pull/154545
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/
https://github.com/carlosgalvezp updated
https://github.com/llvm/llvm-project/pull/154545
>From 0e006a3e28d4d3a6a0faeb88caab344e08fcde38 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20G=C3=A1lvez?=
Date: Wed, 20 Aug 2025 13:46:58 +
Subject: [PATCH] Do not trigger -Wmissing-noreturn on la
https://github.com/carlosgalvezp created
https://github.com/llvm/llvm-project/pull/154545
Fixes #154493
>From ad5814e0d775db9c455809bec7c2e80ab5c5cfc3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20G=C3=A1lvez?=
Date: Wed, 20 Aug 2025 13:46:58 +
Subject: [PATCH] Do not trigger -Wmissing
carlosgalvezp wrote:
FYI bisecting https://github.com/llvm/llvm-project/issues/153770 leads to this
patch
https://github.com/llvm/llvm-project/pull/147835
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/l
https://github.com/carlosgalvezp approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/154017
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
LGTM, thank you!
https://github.com/llvm/llvm-project/pull/153870
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
LGTM, thanks for fixing!
It might be worth double-checking the comment I wrote about this carets option
(which I guess is equivalent to passing the command line flag):
https://github.com/llvm/llvm-project/issues/47042#issuecomment-13
https://github.com/carlosgalvezp edited
https://github.com/llvm/llvm-project/pull/154012
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
This will be a breaking change for people who post-process the clang-tidy logs
in some way and search for `clang-diagnostic-error`:
https://sourcegraph.com/search?q=context:global+clang-diagnostic-error&patternType=keyword&sm=0
I see hits in scripts from "major" organisati
https://github.com/carlosgalvezp approved this pull request.
https://github.com/llvm/llvm-project/pull/154005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp closed
https://github.com/llvm/llvm-project/pull/153941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp closed
https://github.com/llvm/llvm-project/pull/151035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -650,3 +650,20 @@ struct InitFromBindingDecl {
}
};
} // namespace GH82970
+
+namespace inherited_members {
carlosgalvezp wrote:
But like I said I don't have a strong opinion and it's definitely not a blocker
:)
https://github.com/llvm/llvm-project/pull
@@ -650,3 +650,20 @@ struct InitFromBindingDecl {
}
};
} // namespace GH82970
+
+namespace inherited_members {
carlosgalvezp wrote:
Personally I would just remove the namespace altogether, most checks have tests
for templates without handling them any diffe
https://github.com/carlosgalvezp edited
https://github.com/llvm/llvm-project/pull/153941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
LGTM, please fix the CI errors :)
https://github.com/llvm/llvm-project/pull/153941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe
@@ -650,3 +650,20 @@ struct InitFromBindingDecl {
}
};
} // namespace GH82970
+
+namespace inherited_members {
carlosgalvezp wrote:
Nit: the issue is not just about inherited members, since that's working well
before this patch. The key is the child class b
carlosgalvezp wrote:
This should fix #104400
https://github.com/llvm/llvm-project/pull/153941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
LGTM! I agree cleaning existing code shall be done in another PR, via some
automated script.
https://github.com/llvm/llvm-project/pull/153942
___
cfe-commits mailing list
cfe-commits@lists.l
carlosgalvezp wrote:
> the planned follow-up is that the unfinished low-quality
> alpha.core.CastToStruct check will be removed from the Clang Static Analyzer.
Thanks for the clarification, it was indeed written `move` in the commit
message. Sounds good to me!
https://github.com/llvm/llvm-pro
carlosgalvezp wrote:
> You need to have `Sphinx` and build `docs-clang-tools-html` target in CMake
> (possibly using `ninja docs-clang-tools-html`).
> https://clang.llvm.org/extra/clang-tidy/Contributing.html#documenting-your-check
I believe you also need to enable these CMake flags:
```
-DLL
https://github.com/carlosgalvezp edited
https://github.com/llvm/llvm-project/pull/153428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,54 @@
+.. title:: clang-tidy - bugprone-cast-to-struct
+
+bugprone-cast-to-struct
+===
+
+Finds casts from pointers to struct or scalar type to pointers to struct type.
+
+Casts between pointers to different structs can be unsafe because it is
possi
https://github.com/carlosgalvezp closed
https://github.com/llvm/llvm-project/pull/153372
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
I will leave the patch open one more week before landing in case there's more
feedback, thank you!
https://github.com/llvm/llvm-project/pull/151035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi
https://github.com/carlosgalvezp deleted
https://github.com/llvm/llvm-project/pull/147553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -31,8 +31,6 @@ class NoLintDirectiveHandler {
public:
NoLintDirectiveHandler();
~NoLintDirectiveHandler();
carlosgalvezp wrote:
> It's = default; anyway.
Then it should just be removed? Rule of 0.
https://github.com/llvm/llvm-project/pull/147553
_
carlosgalvezp wrote:
Friendly ping @steakhal @Xazax-hun , let me know if I've answered your
questions/comments.
https://github.com/llvm/llvm-project/pull/151035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/m
https://github.com/carlosgalvezp updated
https://github.com/llvm/llvm-project/pull/151035
>From 511308493d8eb6612f6e6413919c7eaa19596820 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20G=C3=A1lvez?=
Date: Thu, 24 Jul 2025 21:10:43 +
Subject: [PATCH] [clang-tidy] Avoid matching nodes in sy
https://github.com/carlosgalvezp closed
https://github.com/llvm/llvm-project/pull/152101
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp created
https://github.com/llvm/llvm-project/pull/152101
Useful when the check warns on template functions to know which type it's
complaining about. Otherwise, since the instantiation stack is not printed,
it's very hard to tell.
>From fc1f9ac108af0721f5fb3f2
https://github.com/carlosgalvezp updated
https://github.com/llvm/llvm-project/pull/151035
>From ba0f486d6509c4227dfd3fdd66f0514534230186 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20G=C3=A1lvez?=
Date: Thu, 24 Jul 2025 21:10:43 +
Subject: [PATCH] [clang-tidy] Avoid matching nodes in sy
carlosgalvezp wrote:
> In one of the files we check. Would we still get warnings for UserCode?
Yes, we would. For example, a simple:
```
int x = 0;
assert(reinterpret_cast(x) == nullptr);
```
Triggers `cppcoreguidelines-pro-type-reinterpret-cast` for the use of
`reinterpret_cast`, even if it'
https://github.com/carlosgalvezp updated
https://github.com/llvm/llvm-project/pull/151035
>From 638b35e0f66f216cf29b67834f212f56f09acd7a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20G=C3=A1lvez?=
Date: Thu, 24 Jul 2025 21:10:43 +
Subject: [PATCH] [clang-tidy] Avoid matching nodes in sy
@@ -1336,6 +1336,44 @@ class MatchASTVisitor : public
RecursiveASTVisitor,
return false;
}
+ template static SourceLocation getNodeLocation(const T &Node) {
+return Node.getBeginLoc();
+ }
+
+ static SourceLocation getNodeLocation(const QualType &Node) { return
https://github.com/carlosgalvezp updated
https://github.com/llvm/llvm-project/pull/151035
>From 89a2228e6599b5d1dbd0b734403b317076a93669 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20G=C3=A1lvez?=
Date: Thu, 24 Jul 2025 21:10:43 +
Subject: [PATCH] [clang-tidy] Avoid matching nodes in sy
@@ -1336,6 +1336,44 @@ class MatchASTVisitor : public
RecursiveASTVisitor,
return false;
}
+ template static SourceLocation getNodeLocation(const T &Node) {
+return Node.getBeginLoc();
+ }
+
+ static SourceLocation getNodeLocation(const QualType &Node) { return
@@ -1336,6 +1336,44 @@ class MatchASTVisitor : public
RecursiveASTVisitor,
return false;
}
+ template static SourceLocation getNodeLocation(const T &Node) {
+return Node.getBeginLoc();
+ }
+
+ static SourceLocation getNodeLocation(const QualType &Node) { return
@@ -1507,11 +1544,17 @@ bool MatchASTVisitor::TraverseStmt(Stmt *StmtNode,
DataRecursionQueue *Queue) {
}
bool MatchASTVisitor::TraverseType(QualType TypeNode) {
+ if (shouldSkipNode(TypeNode))
carlosgalvezp wrote:
My goal was to establish a somewhat consis
@@ -105,6 +105,10 @@ Improvements to clang-tidy
now run checks in parallel by default using all available hardware threads.
Both scripts display the number of threads being used in their output.
+- :program:`clang-tidy` no longer attemps to analyze code from system headers
carlosgalvezp wrote:
> what is the expected behavior for code expanded from macros defined in system
> header?
As it is right now, we do not perform any expansion of the `SourceLocation`
before checking if it is in a system header, so the behavior is that macros
coming from system headers won
carlosgalvezp wrote:
@Xazax-hun @steakhal @AaronBallman Do you have any feedback on the changes to
`ASTMatchers`?
https://github.com/llvm/llvm-project/pull/151035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/m
carlosgalvezp wrote:
> Something else to benchmark:
> https://github.com/llvm/llvm-project/issues/109450.
@firewave Here's the results:
On trunk:
```
6874 warnings generated.
Suppressed 6874 warnings (6874 in non-user code).
real0m5.298s
user0m5.209s
sys 0m0.089s
```
With this p
https://github.com/carlosgalvezp edited
https://github.com/llvm/llvm-project/pull/151035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
> Could you please save this file (or make a gist) for later.
https://godbolt.org/z/s7jPbGdWz
https://github.com/llvm/llvm-project/pull/151035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin
https://github.com/carlosgalvezp approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/151801
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp closed
https://github.com/llvm/llvm-project/pull/151772
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp edited
https://github.com/llvm/llvm-project/pull/151772
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp created
https://github.com/llvm/llvm-project/pull/151772
…heck
One typically only wants to perform renaming operations in user code, not in
system headers (which are out of the user's control). Let's skip those
altogether.
This leads to performance improvemen
https://github.com/carlosgalvezp edited
https://github.com/llvm/llvm-project/pull/151035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp edited
https://github.com/llvm/llvm-project/pull/118120
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp requested changes to this pull request.
Couple of minor changes.
I'm not convinced about the name of the check, `make-function-to-direct` does
not really say much. What is a "make function" and what is "direct"?
Would it make sense to call it `modernize-use-cta
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - modernize-make-direct
+
+modernize-make-direct
+=
+
+Replaces ``std::make_*`` function calls with direct constructor calls using
class template
+argument deduction (CTAD).
+
+==
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - modernize-make-direct
+
+modernize-make-direct
+=
+
+Replaces ``std::make_*`` function calls with direct constructor calls using
class template
+argument deduction (CTAD).
+
+==
carlosgalvezp wrote:
I'm happy to split the little refactor of `shouldSkipNode` into a separate
patch to make this one smaller / not have to revert the whole thing if it
doesn't work out, please let me know :)
https://github.com/llvm/llvm-project/pull/151035
__
@@ -1336,6 +1336,45 @@ class MatchASTVisitor : public
RecursiveASTVisitor,
return false;
}
+ bool isInSystemHeader(const SourceLocation &Loc) {
+const SourceManager &SM = getASTContext().getSourceManager();
+return SM.isInSystemHeader(Loc);
+ }
+
+ template
https://github.com/carlosgalvezp updated
https://github.com/llvm/llvm-project/pull/151035
>From d52db8ca5d53d021852f85600a63ba235e73fb0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20G=C3=A1lvez?=
Date: Thu, 24 Jul 2025 21:10:43 +
Subject: [PATCH] [clang-tidy] Avoid matching nodes in sy
https://github.com/carlosgalvezp edited
https://github.com/llvm/llvm-project/pull/151035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
LGTM!
Not sure if possible but a less aggressive option would be to have a "-l" flag
like "make" has, which uses as many threads as requested while keeping track on
the load of the system and adjusting accordingly.
https://github.co
@@ -0,0 +1,248 @@
+// TODO: When Clang adds support for C++23 floating-point types, enable these
tests by:
+//1. Removing all the #if 0 + #endif guards.
+//2. Removing all occurrences of the string "DISABLED-" in this file.
+//3. Deleting this message.
+// These suffi
@@ -225,6 +226,89 @@ void integer_complex_suffix() {
static_assert(v28 == 1J, "");
}
+// This is a C++23 feature, but Clang supports it in earlier language modes
+// as an extension, so we test it unconditionally.
carlosgalvezp wrote:
I agree!
https://gith
https://github.com/carlosgalvezp approved this pull request.
LGTM, but please rebase onto latest trunk and address/resolve pending comments.
https://github.com/llvm/llvm-project/pull/144213
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https:
carlosgalvezp wrote:
I agree that a new module seems unnecessary, do we know that it will contain
more checks in the future? If not moving to llvm or plugins would perhaps be
more suitable.
https://github.com/llvm/llvm-project/pull/149148
___
cfe-com
https://github.com/carlosgalvezp approved this pull request.
https://github.com/llvm/llvm-project/pull/148622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
Maybe this has been already discussed, but it feels strange to me to capitalize
the names of the tools, like Clang-Tidy. Why is that? I don't know what is
"correct" in this case, do we have such a pattern in other documents?
https://github.com/llvm/llvm-project/pull/148622
https://github.com/carlosgalvezp approved this pull request.
LGTM!
https://github.com/llvm/llvm-project/pull/148549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
LGTM, thank you!
https://github.com/llvm/llvm-project/pull/148547
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
https://github.com/llvm/llvm-project/pull/148399
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
LGTM!
https://github.com/llvm/llvm-project/pull/147142
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
https://github.com/llvm/llvm-project/pull/148384
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
https://github.com/llvm/llvm-project/pull/148357
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
https://github.com/llvm/llvm-project/pull/148352
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
https://github.com/llvm/llvm-project/pull/147793
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
> I will add it in a separate PR. Don't feel right to make kind-of unrelated
> changes after most of the people gave their consent.
Ok!
https://github.com/llvm/llvm-project/pull/147793
___
cfe-commits mailing list
cfe-commits@li
carlosgalvezp wrote:
Thank you for driving this, LGTM!
One last thing I'm missing: can you update the clang-tidy contributors guide,
to explain that we expect people to run clang-tidy as well (and what command to
run)? Since it's not currently enforced in CI.
https://github.com/llvm/llvm-pro
carlosgalvezp wrote:
> bellow specific size
Like I mentioned in the other patch I think this is brittle and leads to poor
UX, because you add 1 field to a struct in a header file and suddenly you have
to change all clients.
I'd be preferred to have the user specify which types to warn on. Ma
carlosgalvezp wrote:
@PiotrZSL Here's some examples discussing the performance implications
https://youtu.be/IDQ0ng8RIqs?si=fDUuTFK9GiGCB-Po
https://github.com/llvm/llvm-project/pull/147809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https
https://github.com/carlosgalvezp approved this pull request.
https://github.com/llvm/llvm-project/pull/142839
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
I agree with the sentiment that it would be good to have one single check
enforcing either one style or the other, possibly a "readability" type of
check. Then llvm could be an alias with the proper config.
I suppose this patch does not impede further refactoring if we wa
https://github.com/carlosgalvezp approved this pull request.
LGTM!
https://github.com/llvm/llvm-project/pull/146830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/carlosgalvezp approved this pull request.
LGTM!
https://github.com/llvm/llvm-project/pull/147793
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
+1 to moving to `readability`.
What's the behavior when you have:
```cpp
#if defined(foo) && defined(bar)
```
? I don't believe I saw a test for this use case.
As a user I would probably prefer to keep it as is instead of having one
`ifdef` and one `if defined`. Perhaps
carlosgalvezp wrote:
Check makes sense to me! As a first iteration it's fine without autofix but I
can imagine it will be hard for a codebase to enable this check given the large
amount of things to fix. Maybe users can specify a mapping of wanted types via
an option?
https://github.com/llvm/
1 - 100 of 685 matches
Mail list logo
