[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti commented:

Thanks for the cleanup, it looks overall good (w.r.t `asyncio`: I only know 
about `asyncio` what I read in this pr).

> Only print the filename after completion, not the entire Clang-Tidy 
> invocation command. I find this neater but the behavior can easily be 
> restored.

> By default print like in original script (command line), but add --progress 
> switch that will change it to current behavior.

I'd prefer the original behavior as well, but printing only the filename can be 
added as a flag. Although, I'm not sure `--progress` is a good name for 
something that only changes if the command or file is printed, because the 
progress indicator should be emitted either way.

> Added an initial message with the number of files to process over the number 
> of files in compilation database.

Maybe that was removed? Either way, I think it's +- not necessary, but I'm not 
against having it.

> More graceful shutdown would be welcome.

+1
IMO, that needs to be done before merging.


https://github.com/llvm/llvm-project/pull/89490
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)

[Julian Schmidt via cfe-commits]

@@ -501,70 +506,72 @@ def main():
 
 # Build up a big regexy filter from all command line arguments.
 file_name_re = re.compile("|".join(args.files))
+files = {f for f in files if file_name_re.search(f)}
 
-return_code = 0
+returncode = 0
 try:
-# Spin up a bunch of tidy-launching threads.
-task_queue = queue.Queue(max_task)
-# List of files with a non-zero return code.
-failed_files = []
-lock = threading.Lock()
-for _ in range(max_task):
-t = threading.Thread(
-target=run_tidy,
-args=(
-args,
-clang_tidy_binary,
-export_fixes_dir,
-build_path,
-task_queue,
-lock,
-failed_files,
-),
+semaphore = asyncio.Semaphore(max_task)
+tasks = [
+run_with_semaphore(
+semaphore,
+run_tidy,
+args,
+f,
+clang_tidy_binary,
+export_fixes_dir,
+build_path,
 )
-t.daemon = True
-t.start()
-
-# Fill the queue with files.
-for name in files:
-if file_name_re.search(name):
-task_queue.put(name)
-
-# Wait for all threads to be done.
-task_queue.join()
-if len(failed_files):
-return_code = 1
-
+for f in files
+]
+
+for i, coro in enumerate(asyncio.as_completed(tasks)):
+name, process_returncode, stdout, stderr = await coro
+if process_returncode != 0:
+returncode = 1
+if process_returncode < 0:
+stderr += f"{name}: terminated by signal 
{-process_returncode}\n"
+print(f"[{i + 1}/{len(files)}] {name}")

5chmidti wrote:

It would look even nicer if the first number was printed with the width of the 
second number, i.e.: 

```
[ 3/15]
...
[15/15]
```

That way, the position of the second number and the command(/file) does not 
shift when the next power of ten is reached.

```python
i = 4
num_files = 15

print("[{0: >{1}}/{2}]".format(i+1,len(f"{num_files}"), num_files))
print(f'[{i+1: >{len(f"{num_files}")}}/{num_files}]')
```


https://github.com/llvm/llvm-project/pull/89490
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)

[Julian Schmidt via cfe-commits]

@@ -501,70 +506,72 @@ def main():
 
 # Build up a big regexy filter from all command line arguments.
 file_name_re = re.compile("|".join(args.files))
+files = {f for f in files if file_name_re.search(f)}
 
-return_code = 0
+returncode = 0
 try:
-# Spin up a bunch of tidy-launching threads.
-task_queue = queue.Queue(max_task)
-# List of files with a non-zero return code.
-failed_files = []
-lock = threading.Lock()
-for _ in range(max_task):
-t = threading.Thread(
-target=run_tidy,
-args=(
-args,
-clang_tidy_binary,
-export_fixes_dir,
-build_path,
-task_queue,
-lock,
-failed_files,
-),
+semaphore = asyncio.Semaphore(max_task)
+tasks = [
+run_with_semaphore(
+semaphore,
+run_tidy,
+args,
+f,
+clang_tidy_binary,
+export_fixes_dir,
+build_path,
 )
-t.daemon = True
-t.start()
-
-# Fill the queue with files.
-for name in files:
-if file_name_re.search(name):
-task_queue.put(name)
-
-# Wait for all threads to be done.
-task_queue.join()
-if len(failed_files):
-return_code = 1
-
+for f in files
+]
+
+for i, coro in enumerate(asyncio.as_completed(tasks)):
+name, process_returncode, stdout, stderr = await coro
+if process_returncode != 0:
+returncode = 1
+if process_returncode < 0:
+stderr += f"{name}: terminated by signal 
{-process_returncode}\n"
+print(f"[{i + 1}/{len(files)}] {name}")
+if stdout:
+print(stdout)
+if stderr:
+print(stderr, file=sys.stderr)
 except KeyboardInterrupt:
 # This is a sad hack. Unfortunately subprocess goes
 # bonkers with ctrl-c and we start forking merrily.
 print("\nCtrl-C detected, goodbye.")
 if delete_fixes_dir:
+assert export_fixes_dir
 shutil.rmtree(export_fixes_dir)
 os.kill(0, 9)
 
 if combine_fixes:
-print("Writing fixes to " + args.export_fixes + " ...")
+print(f"Writing fixes to {args.export_fixes} ...")
 try:
+assert export_fixes_dir
 merge_replacement_files(export_fixes_dir, args.export_fixes)
 except:
 print("Error exporting fixes.\n", file=sys.stderr)
 traceback.print_exc()
-return_code = 1
+returncode = 1
 
 if args.fix:
 print("Applying fixes ...")
 try:
+assert export_fixes_dir
 apply_fixes(args, clang_apply_replacements_binary, 
export_fixes_dir)
 except:
 print("Error applying fixes.\n", file=sys.stderr)
 traceback.print_exc()
-return_code = 1
+returncode = 1
 
 if delete_fixes_dir:
+assert export_fixes_dir
 shutil.rmtree(export_fixes_dir)
-sys.exit(return_code)
+sys.exit(returncode)
 
 
 if __name__ == "__main__":
-main()
+# FIXME Python 3.7: This can be simplified by asyncio.run(main()).
+loop = asyncio.new_event_loop()
+loop.run_until_complete(main())
+loop.close()

5chmidti wrote:

Just asking: Being unfamiliar with `asyncio`, why do we need this part and an 
`async main`, instead of calling plain `main`?

https://github.com/llvm/llvm-project/pull/89490
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/89490
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Fix handling of members in readability-redundant-member-init (PR #93217)

[Julian Schmidt via cfe-commits]

@@ -387,6 +387,11 @@ Changes in existing checks
   ` check to properly
   emit warnings for static data member with an in-class initializer.
 
+- Improved :doc:`readability-redundant-member-init
+  ` check to avoid
+  false-positives when type of the member does not match type of the
+  initializer.
+

5chmidti wrote:

I think this note should mention that base-class initializers are now checked 
as well. It's a minor user-facing change.

(While you're at it: the last to `type` need a `the` beforehand)

https://github.com/llvm/llvm-project/pull/93217
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Fix handling of members in readability-redundant-member-init (PR #93217)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.

LGTM, with a release note nit.

https://github.com/llvm/llvm-project/pull/93217
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Fix handling of members in readability-redundant-member-init (PR #93217)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/93217
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Optimize realpath in readability-identifier-naming (PR #92659)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.

This halves the time clang-tidy takes to run this check on 
`IdentifierNamingCheck.cpp` for me :)

Maybe add a release note?


https://github.com/llvm/llvm-project/pull/92659
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Correcting issues in `readability-implicit-bool-conversion` on C23 (PR #92241)

[Julian Schmidt via cfe-commits]

=?utf-8?q?Björn?= Svensson ,
=?utf-8?q?Björn?= Svensson 
Message-ID:
In-Reply-To: 



@@ -96,8 +96,8 @@ The rules for generating fix-it hints are:
   - ``if (!pointer)`` is changed to ``if (pointer == nullptr)``,
 
 - in case of conversions from bool to other built-in types, an explicit
-  ``static_cast`` is proposed to make it clear that a conversion is taking
-  place:
+  ``static_cast`` (or a C-style cast for C23) is proposed to make it clear that

5chmidti wrote:

Please change this to `for` -> `since`

https://github.com/llvm/llvm-project/pull/92241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Correcting issues in `readability-implicit-bool-conversion` on C23 (PR #92241)

[Julian Schmidt via cfe-commits]

=?utf-8?q?Björn?= Svensson ,
=?utf-8?q?Björn?= Svensson 
Message-ID:
In-Reply-To: 



@@ -368,7 +368,8 @@ Changes in existing checks
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
-  fixed problem with duplicate parentheses in double implicit casts.
+  fixed problem with duplicate parentheses in double implicit casts. Corrected
+  the fix suggestions for C23 by using C-style casts instead of 
``static_cast``.

5chmidti wrote:

`for C23 and later`

https://github.com/llvm/llvm-project/pull/92241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Correcting issues in `readability-implicit-bool-conversion` on C23 (PR #92241)

[Julian Schmidt via cfe-commits]

=?utf-8?q?Bj=C3=B6rn?= Svensson ,
=?utf-8?q?Bj=C3=B6rn?= Svensson 
Message-ID:
In-Reply-To: 


https://github.com/5chmidti approved this pull request.

LGTM, just two nits w.r.t documentation wording

https://github.com/llvm/llvm-project/pull/92241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Correcting issues in `readability-implicit-bool-conversion` on C23 (PR #92241)

[Julian Schmidt via cfe-commits]

=?utf-8?q?Björn?= Svensson ,
=?utf-8?q?Björn?= Svensson 
Message-ID:
In-Reply-To: 


https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/92241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Rename out-of-line function definitions (PR #91954)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.


https://github.com/llvm/llvm-project/pull/91954
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

5chmidti wrote:

Somehow I couldn't link that issue the normal way through the `Development` 
entry on the side, so I eddited in a `Fixed` message into the initial comment.

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix crash due to assumed callee in min-max-use-initializer-list (PR #91992)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti closed 
https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix crash due to assumed callee in min-max-use-initializer-list (PR #91992)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy][NFC] replace comparison of begin and end iterators with range empty (PR #91994)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti closed 
https://github.com/llvm/llvm-project/pull/91994
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Fix crash in modernize-use-constraints (PR #92019)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.


https://github.com/llvm/llvm-project/pull/92019
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy][NFC] replace comparison of begin and end iterators with range empty (PR #91994)

[Julian Schmidt via cfe-commits]

5chmidti wrote:

(The commit was a bit older, where I still used the private email, fixed by 
recommitting with the public one)

https://github.com/llvm/llvm-project/pull/91994
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy][NFC] replace comparison of begin and end iterators with range empty (PR #91994)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti updated 
https://github.com/llvm/llvm-project/pull/91994

>From a73f5a9cb3127893dd65ba2040634a5f9f6924b0 Mon Sep 17 00:00:00 2001
From: Julian Schmidt 
Date: Mon, 13 May 2024 18:53:25 +0200
Subject: [PATCH] [clang-tidy][NFC] replace comparison of begin and end
 iterators with range empty

Improves readability by changing comparisons of `*_begin` and `*_end`
iterators into `.empty()` on their range.
---
 .../clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp  | 3 +--
 .../MisleadingCaptureDefaultByValueCheck.cpp  | 3 +--
 clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp   | 4 +---
 .../clang-tidy/modernize/UseConstraintsCheck.cpp  | 2 +-
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
index ca1ae551cc632..2fca7ae2e7eee 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
@@ -171,8 +171,7 @@ void SuspiciousEnumUsageCheck::check(const 
MatchFinder::MatchResult ) {
 // Skip when one of the parameters is an empty enum. The
 // hasDisjointValueRange function could not decide the values properly in
 // case of an empty enum.
-if (EnumDec->enumerator_begin() == EnumDec->enumerator_end() ||
-OtherEnumDec->enumerator_begin() == OtherEnumDec->enumerator_end())
+if (EnumDec->enumerators().empty() || OtherEnumDec->enumerators().empty())
   return;
 
 if (!hasDisjointValueRange(EnumDec, OtherEnumDec))
diff --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/MisleadingCaptureDefaultByValueCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/MisleadingCaptureDefaultByValueCheck.cpp
index 00dfa17a1ccf6..5dee7f91a9341 100644
--- 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/MisleadingCaptureDefaultByValueCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/MisleadingCaptureDefaultByValueCheck.cpp
@@ -67,8 +67,7 @@ static std::string createReplacementText(const LambdaExpr 
*Lambda) {
   AppendName("this");
 }
   }
-  if (!Replacement.empty() &&
-  Lambda->explicit_capture_begin() != Lambda->explicit_capture_end()) {
+  if (!Replacement.empty() && !Lambda->explicit_captures().empty()) {
 // Add back separator if we are adding explicit capture variables.
 Stream << ", ";
   }
diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
index 3f1d2f9f58099..c2d9286312dc4 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -192,9 +192,7 @@ void UnusedParametersCheck::check(const 
MatchFinder::MatchResult ) {
 
 // In non-strict mode ignore function definitions with empty bodies
 // (constructor initializer counts for non-empty body).
-if (StrictMode ||
-(Function->getBody()->child_begin() !=
- Function->getBody()->child_end()) ||
+if (StrictMode || !Function->getBody()->children().empty() ||
 (isa(Function) &&
  cast(Function)->getNumCtorInitializers() > 0))
   warnOnUnusedParameter(Result, Function, I);
diff --git a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
index 6d7d1d6b87c60..1585925ee9967 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
@@ -254,7 +254,7 @@ findInsertionForConstraint(const FunctionDecl *Function, 
ASTContext ) {
 return utils::lexer::findPreviousTokenKind(Init->getSourceLocation(),
SM, LangOpts, tok::colon);
 }
-if (Constructor->init_begin() != Constructor->init_end())
+if (!Constructor->inits().empty())
   return std::nullopt;
   }
   if (Function->isDeleted()) {

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy][NFC] replace comparison of begin and end iterators with range empty (PR #91994)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91994
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy][NFC] replace comparison of begin and end iterators with range empty (PR #91994)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti created 
https://github.com/llvm/llvm-project/pull/91994

Improves readability by changing comparisons of `*begin` and `*end`
iterators into `.empty()` on their range.


>From a2cf452b69c9a4ad173db34ed9098a76d65187da Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmi...@users.noreply.github.com>
Date: Tue, 16 Jan 2024 18:18:57 +0100
Subject: [PATCH] [clang-tidy][NFC] replace comparison of begin and end
 iterators with range empty

Improves readability by changing comparisons of `*begin` and `*end`
iterators into `.empty()` on their range.
---
 .../clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp  | 3 +--
 .../MisleadingCaptureDefaultByValueCheck.cpp  | 3 +--
 clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp   | 4 +---
 .../clang-tidy/modernize/UseConstraintsCheck.cpp  | 2 +-
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
index ca1ae551cc632..2fca7ae2e7eee 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
@@ -171,8 +171,7 @@ void SuspiciousEnumUsageCheck::check(const 
MatchFinder::MatchResult ) {
 // Skip when one of the parameters is an empty enum. The
 // hasDisjointValueRange function could not decide the values properly in
 // case of an empty enum.
-if (EnumDec->enumerator_begin() == EnumDec->enumerator_end() ||
-OtherEnumDec->enumerator_begin() == OtherEnumDec->enumerator_end())
+if (EnumDec->enumerators().empty() || OtherEnumDec->enumerators().empty())
   return;
 
 if (!hasDisjointValueRange(EnumDec, OtherEnumDec))
diff --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/MisleadingCaptureDefaultByValueCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/MisleadingCaptureDefaultByValueCheck.cpp
index 00dfa17a1ccf6..5dee7f91a9341 100644
--- 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/MisleadingCaptureDefaultByValueCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/MisleadingCaptureDefaultByValueCheck.cpp
@@ -67,8 +67,7 @@ static std::string createReplacementText(const LambdaExpr 
*Lambda) {
   AppendName("this");
 }
   }
-  if (!Replacement.empty() &&
-  Lambda->explicit_capture_begin() != Lambda->explicit_capture_end()) {
+  if (!Replacement.empty() && !Lambda->explicit_captures().empty()) {
 // Add back separator if we are adding explicit capture variables.
 Stream << ", ";
   }
diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
index 3f1d2f9f58099..c2d9286312dc4 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -192,9 +192,7 @@ void UnusedParametersCheck::check(const 
MatchFinder::MatchResult ) {
 
 // In non-strict mode ignore function definitions with empty bodies
 // (constructor initializer counts for non-empty body).
-if (StrictMode ||
-(Function->getBody()->child_begin() !=
- Function->getBody()->child_end()) ||
+if (StrictMode || !Function->getBody()->children().empty() ||
 (isa(Function) &&
  cast(Function)->getNumCtorInitializers() > 0))
   warnOnUnusedParameter(Result, Function, I);
diff --git a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
index 6d7d1d6b87c60..1585925ee9967 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
@@ -254,7 +254,7 @@ findInsertionForConstraint(const FunctionDecl *Function, 
ASTContext ) {
 return utils::lexer::findPreviousTokenKind(Init->getSourceLocation(),
SM, LangOpts, tok::colon);
 }
-if (Constructor->init_begin() != Constructor->init_end())
+if (!Constructor->inits().empty())
   return std::nullopt;
   }
   if (Function->isDeleted()) {

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix crash due to assumed callee in min-max-use-initializer-list (PR #91992)

[Julian Schmidt via cfe-commits]

5chmidti wrote:

I have a few small changes to `findArgs` that I have not committed, which would 
change the `if`'s to check the exact number of args that are expected, instead 
of `< 3` and `else` (+change from iterators to `getArg(n)`). Should I add them 
as well?

https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix crash due to assumed callee in min-max-use-initializer-list (PR #91992)

[Julian Schmidt via cfe-commits]

5chmidti wrote:

CC @sopyb I was already looking into it ^^

https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix crash due to assumed callee in min-max-use-initializer-list (PR #91992)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix crash due to assumed callee in min-max-use-initializer-list (PR #91992)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti created 
https://github.com/llvm/llvm-project/pull/91992

Previously, the call to `findArgs` for a `CallExpr` inside of a `min` or
`max` call would call `findArgs` before checking if the argument is a
call to `min` or `max`, which is what `findArgs` is expecting.
The fix moves the name checking before the call to `findArgs`, such that
only a `min` or `max` function call is used as an argument.


>From 4819130e4b5aa02edc452eaccea9b451dfff9de1 Mon Sep 17 00:00:00 2001
From: Julian Schmidt 
Date: Mon, 13 May 2024 18:35:51 +0200
Subject: [PATCH] [clang-tidy] fix crash due to assumed callee in
 min-max-use-initializer-list

Previously, the call to `findArgs` for a `CallExpr` inside of a `min` or
`max` call would call `findArgs` before checking if the argument is a
call to `min` or `max`, which is what `findArgs` is expecting.
The fix moves the name checking before the call to `findArgs`, such that
only a `min` or `max` function call is used as an argument.
---
 .../MinMaxUseInitializerListCheck.cpp | 10 -
 .../min-max-use-initializer-list.cpp  | 21 +++
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git 
a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 45f7700463d57..418699ffbc4d1 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -129,17 +129,17 @@ generateReplacements(const MatchFinder::MatchResult 
,
   continue;
 }
 
+// if the nested call is not the same as the top call
+if (InnerCall->getDirectCallee()->getQualifiedNameAsString() !=
+TopCall->getDirectCallee()->getQualifiedNameAsString())
+  continue;
+
 const FindArgsResult InnerResult = findArgs(InnerCall);
 
 // if the nested call doesn't have arguments skip it
 if (!InnerResult.First || !InnerResult.Last)
   continue;
 
-// if the nested call is not the same as the top call
-if (InnerCall->getDirectCallee()->getQualifiedNameAsString() !=
-TopCall->getDirectCallee()->getQualifiedNameAsString())
-  continue;
-
 // if the nested call doesn't have the same compare function
 if ((Result.Compare || InnerResult.Compare) &&
 !utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
diff --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp
index 51ab9bda975f1..1f2dad2b933ca 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp
@@ -300,6 +300,27 @@ B maxTT2 = std::max(B(), std::max(B(), B()));
 B maxTT3 = std::max(B(), std::max(B(), B()), [](const B , const B ) { 
return lhs.a[0] < rhs.a[0]; });
 // CHECK-FIXES: B maxTT3 = std::max(B(), std::max(B(), B()), [](const B , 
const B ) { return lhs.a[0] < rhs.a[0]; });
 
+struct GH91982 {
+  int fun0Args();
+  int fun1Arg(int a);
+  int fun2Args(int a, int b);
+  int fun3Args(int a, int b, int c);
+  int fun4Args(int a, int b, int c, int d);
+
+  int foo() {
+return std::max(
+fun0Args(),
+std::max(fun1Arg(0),
+ std::max(fun2Args(0, 1),
+  std::max(fun3Args(0, 1, 2), fun4Args(0, 1, 2, 3);
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: do not use nested 'std::max' 
calls, use an initializer list instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: return std::max(
+// CHECK-FIXES-NEXT: {fun0Args(),
+// CHECK-FIXES-NEXT: fun1Arg(0),
+// CHECK-FIXES-NEXT: fun2Args(0, 1),
+// CHECK-FIXES-NEXT: fun3Args(0, 1, 2), fun4Args(0, 1, 2, 3)});
+  }
+};
 
 } // namespace
 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Ignore `if consteval` in else-after-return (PR #91588)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.


https://github.com/llvm/llvm-project/pull/91588
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,30 @@
+//===--- VirtualArithmeticCheck.h - clang-tidy---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_VIRTUAL_ARITHMETIC_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_VIRTUAL_ARITHMETIC_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Finds pointer arithmetic on classes that declare a virtual function.

5chmidti wrote:

This sentence, the first sentence of `virtual-arithmetic.rst` and the sentence 
in the release notes (missing) should all be the same.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,59 @@
+// RUN: %check_clang_tidy %s bugprone-virtual-arithmetic %t
+
+class Base {
+public:  
+  virtual ~Base() {}
+};
+
+class Derived : public Base {};
+
+void operators() {
+  Base *b = new Derived[10];
+
+  b += 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 
'Base' that declares a virtual function, undefined behavior if the pointee is a 
different class [bugprone-virtual-arithmetic]
+
+  b = b + 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on class 
'Base' that declares a virtual function, undefined behavior if the pointee is a 
different class [bugprone-virtual-arithmetic]
+
+  b++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 
'Base' that declares a virtual function, undefined behavior if the pointee is a 
different class [bugprone-virtual-arithmetic]
+
+  b[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 
'Base' that declares a virtual function, undefined behavior if the pointee is a 
different class [bugprone-virtual-arithmetic]
+
+  delete[] static_cast(b);
+}
+
+void subclassWarnings() {
+  Base *b = new Base[10];
+
+  // False positive that's impossible to distinguish without
+  // path-sensitive analysis, but the code is bug-prone regardless.
+  b += 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 
'Base' that declares a virtual function
+
+  delete[] b;
+
+  // Common false positive is a class that overrides all parent functions.
+  Derived *d = new Derived[10];
+
+  d += 1;
+  // no-warning
+
+  delete[] d;
+}
+
+template 
+void templateWarning(T *t) {
+  // FIXME: Show the location of the template instantiation in diagnostic.
+  t += 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 
'Base' that declares a virtual function
+}
+
+void functionArgument(Base *b) {
+  b += 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 
'Base' that declares a virtual function
+
+  templateWarning(b);
+}

5chmidti wrote:

Please add a newline to the end of the file.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,30 @@
+//===--- VirtualArithmeticCheck.h - clang-tidy---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_VIRTUAL_ARITHMETIC_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_VIRTUAL_ARITHMETIC_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Finds pointer arithmetic on classes that declare a virtual function.
+///
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/virtual-arithmetic.html
+class VirtualArithmeticCheck : public ClangTidyCheck {
+public:
+  VirtualArithmeticCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;

5chmidti wrote:

Please add
```c++
  bool isLanguageVersionSupported(const LangOptions ) const override {
return LangOpts.CPlusPlus;
  }
```
because this check is only for C++.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,49 @@
+//===--- VirtualArithmeticCheck.cpp - 
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "VirtualArithmeticCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void VirtualArithmeticCheck::registerMatchers(MatchFinder *Finder) {
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(pointerType(pointee(hasDeclaration(
+   cxxRecordDecl(hasMethod(isVirtualAsWritten(
+  .bind("pointer");
+
+  const auto ArraySubscript =
+  arraySubscriptExpr(hasBase(PointerExprWithVirtualMethod));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+ hasEitherOperand(PointerExprWithVirtualMethod));
+
+  const auto UnaryOperators =
+  unaryOperator(hasAnyOperatorName("++", "--"),
+hasUnaryOperand(PointerExprWithVirtualMethod));
+
+  Finder->addMatcher(
+  expr(anyOf(ArraySubscript, BinaryOperators, UnaryOperators)), this);

5chmidti wrote:

The check does not diagnose a few cases that IMO should be:

### Inheritance
Diagnose expressions with a pointer type, where the class either directly 
declares a virtual method (current matcher), or it inherits a pure virtual 
function that was not yet overridden in the inheritance chain.

This can be fixed by using
```c++
  const auto VirtualRecord =
  cxxRecordDecl(anyOf(isAbstract(), hasMethod(isVirtualAsWritten(;

  const auto PointerExprWithVirtualMethod =
  expr(hasType(pointerType(pointee(hasDeclaration(VirtualRecord)
  .bind("pointer");
```

### Type Aliases
Secondly, the matcher does not check the underlying type of type aliases.
The 
```c++
expr(hasType(pointerType(pointee(hasDeclaration(
   cxxRecordDecl(hasMethod(isVirtualAsWritten(
  .bind("pointer")
```
matcher should have a `hasCanonicalType` after the `hasType` and after the 
`pointee` to fix this.

E.g.:

```c++
using BaseAlias = Base;
using DerivedAlias = Derived;

using BasePtr = BaseAlias*;
using DerivedPtr = DerivedAlias*;

void foo(Derived* p1, BaseAlias* p2, DerivedAlias* p3, BasePtr p4, DerivedPtr 
p5,
 BasePtr* p6, DerivedPtr* p7) {
++p1;
++p2;
++p3;
++p4;
++p5;
++p6;
++p7;
}

class PureBase {
  virtual void foo()=0;
};
class PureDerived : public PureBase {};

using PureBaseAlias = PureBase;
using PureDerivedAlias = PureDerived;

using PureBasePtr = PureBaseAlias*;
using PureDerivedPtr = PureDerivedAlias*;

void pure_foo(PureDerived* p1, PureBaseAlias* p2, PureDerivedAlias* p3, 
PureBasePtr p4, PureDerivedPtr p5,
 PureBasePtr* p6, PureDerivedPtr* p7) {
++p1;
++p2;
++p3;
++p4;
++p5;
++p6;
++p7;
}
```

pure1-pure5 should be diagnosed, but they currently are not.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,49 @@
+//===--- VirtualArithmeticCheck.cpp - 
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "VirtualArithmeticCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void VirtualArithmeticCheck::registerMatchers(MatchFinder *Finder) {
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(pointerType(pointee(hasDeclaration(
+   cxxRecordDecl(hasMethod(isVirtualAsWritten(
+  .bind("pointer");
+
+  const auto ArraySubscript =
+  arraySubscriptExpr(hasBase(PointerExprWithVirtualMethod));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+ hasEitherOperand(PointerExprWithVirtualMethod));
+
+  const auto UnaryOperators =
+  unaryOperator(hasAnyOperatorName("++", "--"),
+hasUnaryOperand(PointerExprWithVirtualMethod));
+
+  Finder->addMatcher(
+  expr(anyOf(ArraySubscript, BinaryOperators, UnaryOperators)), this);
+}
+
+void VirtualArithmeticCheck::check(const MatchFinder::MatchResult ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("pointer");
+  const CXXRecordDecl *PointeeType =
+  PointerExpr->getType()->getPointeeType()->getAsCXXRecordDecl();
+
+  diag(PointerExpr->getBeginLoc(),
+   "pointer arithmetic on class '%0' that declares a virtual function, "
+   "undefined behavior if the pointee is a different class")
+  << PointeeType->getName();

5chmidti wrote:

- Please stream the `SourceRange` of `PointerExpr` into the diagnostic, so that 
it is fully underlined in the diagnostic/editor
- I think the comma should be replaced with `(can) result in`, wdyt?
- You don't have to escape the `%0` with `'` when you pass `PointeeType` 
without the `getName` instead.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,50 @@
+.. title:: clang-tidy - bugprone-virtual-arithmetic
+
+bugprone-virtual-arithmetic
+===
+
+Warn if pointer arithmetic is performed on a class that declares a
+virtual function.
+
+Pointer arithmetic on polymorphic objects where the pointer's static type is 
+different from its dynamic type is undefined behavior.
+Finding pointers where the static type contains a virtual member function is a
+good heuristic, as the pointer is likely to point to a different, derived 
class.
+
+This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer 
arithmetic on polymorphic objects 
`_.
+
+Example:
+
+.. code-block:: c++
+
+  struct Base {
+virtual void ~Base();
+  };
+
+  struct Derived : public Base {};
+
+  void foo() {
+Base *b = new Derived[10];
+
+b += 1;
+// warning: pointer arithmetic on class that declares a virtual function, 
undefined behavior if the pointee is a different class

5chmidti wrote:

Please reflow this comment to fit into 80 columns.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti requested changes to this pull request.

Needs some small changes before landing. It's a good check to have, thanks.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [WIP][clang-tidy] Ignore `if consteval` in else-after-return (PR #91588)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy -std=c++20 %s readability-else-after-return %t 

5chmidti wrote:

While `consteval` was added in C++20, `if consteval` is a C++23 feature. Please 
use `-std=c++23`
https://en.cppreference.com/w/cpp/language/if#Consteval_if

https://github.com/llvm/llvm-project/pull/91588
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [WIP][clang-tidy] Ignore `if consteval` in else-after-return (PR #91588)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti commented:

Please add an entry to the release notes here: 
https://github.com/llvm/llvm-project/blob/502e77df1fc4aa859db6709e14e93af6207e4dc4/clang-tools-extra/docs/ReleaseNotes.rst?plain=1#L176
do note that the entries are sorted by their check names.

Otherwise looks good.

https://github.com/llvm/llvm-project/pull/91588
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [WIP][clang-tidy] Ignore `if consteval` in else-after-return (PR #91588)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91588
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Add option to exclude headers from clang-tidy analysis (PR #91400)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.


https://github.com/llvm/llvm-project/pull/91400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] support expect no diagnosis test (PR #91293)

[Julian Schmidt via cfe-commits]

@@ -226,6 +236,10 @@ def run_clang_tidy(self):
 
print("--")
 return clang_tidy_output
 
+def check_no_diagnosis(self, clang_tidy_output):
+if clang_tidy_output != "":
+sys.exit("No diagnostics were expected, but found the ones above")

5chmidti wrote:

Ah, ok. So the original printing that I pointed out was not intended to be 
there anyway.

https://github.com/llvm/llvm-project/pull/91293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] use llvm::any_of refactor getAnalyzerCheckersAndPackages [NFC] (PR #91713)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.


https://github.com/llvm/llvm-project/pull/91713
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] support expect no diagnosis test (PR #91293)

[Julian Schmidt via cfe-commits]

@@ -226,6 +236,10 @@ def run_clang_tidy(self):
 
print("--")
 return clang_tidy_output
 
+def check_no_diagnosis(self, clang_tidy_output):
+if clang_tidy_output != "":
+sys.exit("No diagnostics were expected, but found the ones above")

5chmidti wrote:

You may have accidentally not staged the hunk with the `print`. Either way, 
what I meant was:

```python 
if clang_tidy_output != "":
print(clang_tidy_output)
sys.exit("No diagnostics were expected, but found the ones above")
```
Because we already check if `clang_tidy_output` is empty to potentially 
terminate because errors were found when they shouldn't have, we can call print 
inside the if, so we don't print an empty string. (-> nit).

https://github.com/llvm/llvm-project/pull/91293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Ignore unevaluated context in bugprone-optional-value-conversion (PR #90410)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.


https://github.com/llvm/llvm-project/pull/90410
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Ignore unevaluated context in bugprone-optional-value-conversion (PR #90410)

[Julian Schmidt via cfe-commits]

5chmidti wrote:

> And in example that you provided there is no issue.

Alright, at least not a bugrprone issue. And the argument for a readability 
issue (which is the direction I initially was going) would probably end in 
someone saying `we use this to diagnose compile errors earlier, not deeper in 
any instantiations`

https://github.com/llvm/llvm-project/pull/90410
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Add option to exclude headers from clang-tidy analysis (PR #91400)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Add option to exclude headers from clang-tidy analysis (PR #91400)

[Julian Schmidt via cfe-commits]

@@ -311,7 +311,12 @@ ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
 : Context(Ctx), ExternalDiagEngine(ExternalDiagEngine),
   RemoveIncompatibleErrors(RemoveIncompatibleErrors),
   GetFixesFromNotes(GetFixesFromNotes),
-  EnableNolintBlocks(EnableNolintBlocks) {}
+  EnableNolintBlocks(EnableNolintBlocks) {
+
+  if (Context.getOptions().ExcludeHeaderFilterRegex)
+ExcludeHeaderFilter = std::make_unique(
+*Context.getOptions().ExcludeHeaderFilterRegex);
+}

5chmidti wrote:

> which I think is handled in the latest revision

I agree

> I could migrate HeaderFilter to mirror the new and improved 
> ExcludeHeaderFilter...though I worry a bit about this change growing larger 
> and larger

Sure, a small follow-up pr would probably be better.

https://github.com/llvm/llvm-project/pull/91400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] support expect no diagnosis test (PR #91293)

[Julian Schmidt via cfe-commits]

@@ -226,6 +226,11 @@ def run_clang_tidy(self):
 
print("--")
 return clang_tidy_output
 
+def check_no_diagnosis(self, clang_tidy_output):
+print(clang_tidy_output)
+if clang_tidy_output != "":
+sys.exit("No diagnostics were expected, but found the ones above")

5chmidti wrote:

You can move the print inside the if-branch

https://github.com/llvm/llvm-project/pull/91293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] support expect no diagnosis test (PR #91293)

[Julian Schmidt via cfe-commits]

@@ -172,12 +173,11 @@ def get_prefixes(self):
 )
 
 if not has_check_fix and not has_check_message and not 
has_check_note:
-sys.exit(
-"%s, %s or %s not found in the input"
-% (self.fixes.prefix, self.messages.prefix, 
self.notes.prefix)
-)
+self.expect_no_diagnosis = True
 
-assert self.has_check_fixes or self.has_check_messages or 
self.has_check_notes
+assert self.expect_no_diagnosis != (
+self.has_check_fixes or self.has_check_messages or 
self.has_check_notes
+)

5chmidti wrote:

What do you think about emitting a message instead of asserting? I.e., 
something using `sys.ext` like in the other cases that the script exits?

https://github.com/llvm/llvm-project/pull/91293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] support expect no diagnosis test (PR #91293)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.

Overall LGTM, just two small nits. 

W.r.t flag or no flag: This seems to be so rarely used (judging from the single 
test file that needed modification), that not having the flag is fine IMO. A 
check writer doesn't have to know about this small test infra detail, and it's 
also not *that* surprising, that if there are no `CHECK` messages, none are 
expected. Similar to the `implicit check-not`.

However, I think I'd be best to get @PiotrZSL's thoughts as well

https://github.com/llvm/llvm-project/pull/91293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] support expect no diagnosis test (PR #91293)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Add option to exclude headers from clang-tidy analysis (PR #91400)

[Julian Schmidt via cfe-commits]

@@ -311,7 +311,12 @@ ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
 : Context(Ctx), ExternalDiagEngine(ExternalDiagEngine),
   RemoveIncompatibleErrors(RemoveIncompatibleErrors),
   GetFixesFromNotes(GetFixesFromNotes),
-  EnableNolintBlocks(EnableNolintBlocks) {}
+  EnableNolintBlocks(EnableNolintBlocks) {
+
+  if (Context.getOptions().ExcludeHeaderFilterRegex)
+ExcludeHeaderFilter = std::make_unique(
+*Context.getOptions().ExcludeHeaderFilterRegex);
+}

5chmidti wrote:

It's not a good idea to have basically the same thing implemented in two 
different ways (`HeaderFilter` vs `ExcludeHeaderFilter`). I think the 
`ExcludeHeaderFilter` should be implemented like the `HeaderFilter`.
https://github.com/llvm/llvm-project/blob/dcf92a249233cab103f848dd12e96e0d642a8899/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp#L574-L579
This would also solve @PiotrZSL's comment.

We can probably save the constructions of `llvm::Regex` and the call to `match` 
on the empty regex. wdyt? This could also be done in another pr.

https://github.com/llvm/llvm-project/pull/91400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [NFC][clang-tidy]increase stability for bugprone-return-const-ref-from-parameter (PR #91160)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.

Is there something specific you hit (test-case)? Either way, it's good to check

https://github.com/llvm/llvm-project/pull/91160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] new check misc-use-internal-linkage (PR #90830)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/90830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] new check misc-use-internal-linkage (PR #90830)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/90830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] new check misc-use-internal-linkage (PR #90830)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,79 @@
+//===--- UseInternalLinkageCheck.cpp - 
clang-tidy--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UseInternalLinkageCheck.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/STLExtras.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+namespace {
+
+AST_POLYMORPHIC_MATCHER(isFirstDecl,
+AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+VarDecl)) {
+  return Node.isFirstDecl();
+}
+
+AST_MATCHER(Decl, isInMainFile) {
+  return llvm::all_of(Node.redecls(), [&](const Decl *D) {
+return Finder->getASTContext().getSourceManager().isInMainFile(
+D->getLocation());
+  });
+}
+
+AST_POLYMORPHIC_MATCHER(isExternStorageClass,
+AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+VarDecl)) {
+  return Node.getStorageClass() == SC_Extern;
+}
+
+} // namespace
+
+void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
+  auto Common = allOf(isFirstDecl(), isInMainFile(),
+  unless(anyOf(
+  // 1. internal linkage
+  isStaticStorageClass(), isInAnonymousNamespace(),
+  // 2. explicit external linkage
+  isExternStorageClass(), isExternC(),
+  // 3. template
+  isExplicitTemplateSpecialization(),
+  clang::ast_matchers::isTemplateInstantiation(),

5chmidti wrote:

Template instantiations will already be ignored because of the traversal kind

https://github.com/llvm/llvm-project/pull/90830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] new check misc-use-internal-linkage (PR #90830)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti commented:

Regarding unit build:
`google-global-names-in-headers` is one of the checks that check file 
extensions: 
https://github.com/llvm/llvm-project/blob/d33937b6236767137a1ec3393d0933f10eed4ffe/clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp#L42-L44

Nice that there is a check for this now, it looks like there are a few 
instances of this issue in LLVM as well.

https://github.com/llvm/llvm-project/pull/90830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] new check misc-use-internal-linkage (PR #90830)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/90830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] new check misc-use-internal-linkage (PR #90830)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,79 @@
+//===--- UseInternalLinkageCheck.cpp - 
clang-tidy--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UseInternalLinkageCheck.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/STLExtras.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+namespace {
+
+AST_POLYMORPHIC_MATCHER(isFirstDecl,
+AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+VarDecl)) {
+  return Node.isFirstDecl();
+}

5chmidti wrote:

You don't need to make this a polymorphic matcher, `isFirstDecl` is part of 
`clang::Decl`.

https://github.com/llvm/llvm-project/pull/90830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] new check misc-use-internal-linkage (PR #90830)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,79 @@
+//===--- UseInternalLinkageCheck.cpp - 
clang-tidy--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UseInternalLinkageCheck.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/STLExtras.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+namespace {
+
+AST_POLYMORPHIC_MATCHER(isFirstDecl,
+AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+VarDecl)) {
+  return Node.isFirstDecl();
+}
+
+AST_MATCHER(Decl, isInMainFile) {
+  return llvm::all_of(Node.redecls(), [&](const Decl *D) {
+return Finder->getASTContext().getSourceManager().isInMainFile(
+D->getLocation());
+  });
+}
+
+AST_POLYMORPHIC_MATCHER(isExternStorageClass,
+AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+VarDecl)) {
+  return Node.getStorageClass() == SC_Extern;
+}
+
+} // namespace
+
+void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
+  auto Common = allOf(isFirstDecl(), isInMainFile(),
+  unless(anyOf(
+  // 1. internal linkage
+  isStaticStorageClass(), isInAnonymousNamespace(),
+  // 2. explicit external linkage
+  isExternStorageClass(), isExternC(),
+  // 3. template
+  isExplicitTemplateSpecialization(),
+  clang::ast_matchers::isTemplateInstantiation(),
+  // 4. friend
+  hasAncestor(friendDecl();
+  Finder->addMatcher(
+  functionDecl(Common, unless(cxxMethodDecl()), unless(isMain()))
+  .bind("fn"),
+  this);
+  Finder->addMatcher(varDecl(Common, hasGlobalStorage()).bind("var"), this);
+}
+
+static constexpr StringRef Message =
+"%0 %1 can be made static or moved into an anonymous namespace "
+"to enforce internal linkage";
+
+void UseInternalLinkageCheck::check(const MatchFinder::MatchResult ) {
+  if (const auto *FD = Result.Nodes.getNodeAs("fn")) {
+diag(FD->getLocation(), Message) << "function" << FD;
+return;
+  }
+  if (const auto *VD = Result.Nodes.getNodeAs("var")) {
+diag(VD->getLocation(), Message) << "variable" << VD;
+return;
+  }

5chmidti wrote:

Please stream `SourceRange`s into the diagnostics. For the `FunctionDecl` you 
could use `SourceRange(FD->getBeginLoc(), FD->getTypeSpecEndLoc())`, which 
would not highlight the body (that would be quite noisy).

https://github.com/llvm/llvm-project/pull/90830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy][docs] Fix modernize-use-std-print docs (PR #91069)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti closed 
https://github.com/llvm/llvm-project/pull/91069
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix false-positives for templates in `bugprone-return-const-ref-from-parameter` (PR #90273)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti closed 
https://github.com/llvm/llvm-project/pull/90273
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Improve modernize-use-std-print diagnostic (PR #91071)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.

LGTM, thanks

https://github.com/llvm/llvm-project/pull/91071
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy][docs] Fix modernize-use-std-print docs (PR #91069)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.

LGTM, thanks

https://github.com/llvm/llvm-project/pull/91069
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add modernize-use-std-format check (PR #90397)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/90397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add modernize-use-std-format check (PR #90397)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.

Just nits, otherwise LGTM!

https://github.com/llvm/llvm-project/pull/90397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add modernize-use-std-format check (PR #90397)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,107 @@
+//===--- UseStdFormatCheck.cpp - clang-tidy 
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UseStdFormatCheck.h"
+#include "../utils/FormatStringConverter.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+namespace {
+AST_MATCHER(StringLiteral, isOrdinary) { return Node.isOrdinary(); }
+} // namespace
+
+UseStdFormatCheck::UseStdFormatCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  StrictMode(Options.getLocalOrGlobal("StrictMode", false)),
+  StrFormatLikeFunctions(utils::options::parseStringList(
+  Options.get("StrFormatLikeFunctions", ""))),
+  ReplacementFormatFunction(
+  Options.get("ReplacementFormatFunction", "std::format")),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),
+  areDiagsSelfContained()),
+  MaybeHeaderToInclude(Options.get("FormatHeader")) {
+  if (StrFormatLikeFunctions.empty())
+StrFormatLikeFunctions.push_back("absl::StrFormat");
+
+  if (!MaybeHeaderToInclude && ReplacementFormatFunction == "std::format")
+MaybeHeaderToInclude = "";
+}
+
+void UseStdFormatCheck::registerPPCallbacks(const SourceManager ,
+Preprocessor *PP,
+Preprocessor *ModuleExpanderPP) {
+  IncludeInserter.registerPreprocessor(PP);
+}
+
+void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  callExpr(argumentCountAtLeast(1),
+   hasArgument(0, stringLiteral(isOrdinary())),
+   callee(functionDecl(unless(cxxMethodDecl()),
+   matchers::matchesAnyListedName(
+   StrFormatLikeFunctions))
+  .bind("func_decl")))
+  .bind("strformat"),
+  this);
+}
+
+void UseStdFormatCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  using utils::options::serializeStringList;
+  Options.store(Opts, "StrictMode", StrictMode);
+  Options.store(Opts, "StrFormatLikeFunctions",
+serializeStringList(StrFormatLikeFunctions));
+  Options.store(Opts, "ReplacementFormatFunction", ReplacementFormatFunction);
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+  if (MaybeHeaderToInclude)
+Options.store(Opts, "FormatHeader", *MaybeHeaderToInclude);
+}
+
+void UseStdFormatCheck::check(const MatchFinder::MatchResult ) {
+  const unsigned FormatArgOffset = 0;
+  const auto *OldFunction = Result.Nodes.getNodeAs("func_decl");
+  const auto *StrFormat = Result.Nodes.getNodeAs("strformat");
+
+  utils::FormatStringConverter::Configuration ConverterConfig;
+  ConverterConfig.StrictMode = StrictMode;
+  utils::FormatStringConverter Converter(Result.Context, StrFormat,
+ FormatArgOffset, ConverterConfig,
+ getLangOpts());
+  const Expr *StrFormatCall = StrFormat->getCallee();
+  if (!Converter.canApply()) {
+DiagnosticBuilder Diag = diag(StrFormat->getBeginLoc(),
+  "unable to use '%0' instead of %1 because 
%2")
+ << ReplacementFormatFunction
+ << OldFunction->getIdentifier()
+ << Converter.conversionNotPossibleReason();

5chmidti wrote:

Please stream `StrFormatCall->getSourceRange()` into the diagnostic so that the 
function name is underlined as well in this case. In the other cases, the 
FixIt's will take care of that.

https://github.com/llvm/llvm-project/pull/90397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add modernize-use-std-format check (PR #90397)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,84 @@
+.. title:: clang-tidy - modernize-use-std-format
+
+modernize-use-std-format
+
+
+Converts calls to ``absl::StrFormat``, or other functions via
+configuration options, to C++20's ``std::format``, or another function
+via a configuration option, modifying the format string appropriately and
+removing now-unnecessary calls to ``std::string::c_str()`` and
+``std::string::data()``.
+
+For example, it turns lines like
+
+.. code-block:: c++
+
+  return absl::StrFormat("The %s is %3d", description.c_str(), value);
+
+into:
+
+.. code-block:: c++
+
+  return std::format("The {} is {:3}", description, value);
+
+The check uses the same format-string-conversion algorithm as
+`modernize-use-std-print <../modernize/use-std-print.html>`_ and its
+shortcomings are described in the documentation for that check.
+
+Options
+---
+
+.. option:: StrictMode
+
+   When `true`, the check will add casts when converting from variadic
+   functions and printing signed or unsigned integer types (including
+   fixed-width integer types from , ``ptrdiff_t``, ``size_t``
+   and ``ssize_t``) as the opposite signedness to ensure that the output
+   would matches that of a simple wrapper for ``std::sprintf`` that
+   accepted a C-style variable argument list. For example, with
+   `StrictMode` enabled,
+
+  .. code-block:: c++
+
+extern std::string strprintf(const char *format, ...);
+int i = -42;
+unsigned int u = 0x;
+return strprintf("%d %u\n", i, u);
+
+  would be converted to
+
+  .. code-block:: c++
+
+return std::format("{} {}\n", static_cast(i), 
static_cast(u));
+
+  to ensure that the output will continue to be the unsigned representation
+  of -42 and the signed representation of 0x (often 4294967254
+  and -1 respectively). When `false` (which is the default), these casts
+  will not be added which may cause a change in the output. Note that this
+  option makes no difference for the default value of
+  `StrFormatLikeFunctions` since ``absl::StrFormat`` takes a function
+  parameter pack and is not a variadic function.
+
+.. option:: StrFormatLikeFunctions
+
+   A semicolon-separated list of (fully qualified) extra function names to

5chmidti wrote:

The `extra` is wrong here. If you don't pass an option the default is described 
later, but when you do pass something, then default `absl::StrFormat` is not 
part of the checked function names.

https://github.com/llvm/llvm-project/pull/90397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add modernize-use-std-format check (PR #90397)

[Julian Schmidt via cfe-commits]

5chmidti wrote:

Should the check options use the simpler format? E.g. 
```
CheckOptions: 
  modernize-use-std-format.StrFormatLikeFunctions: 
'unqualified_strprintf;::strprintf; mynamespace::strprintf2'
```

https://github.com/llvm/llvm-project/pull/90397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add modernize-use-std-format check (PR #90397)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/90397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add modernize-use-std-format check (PR #90397)

[Julian Schmidt via cfe-commits]

@@ -0,0 +1,107 @@
+//===--- UseStdFormatCheck.cpp - clang-tidy 
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UseStdFormatCheck.h"
+#include "../utils/FormatStringConverter.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+namespace {
+AST_MATCHER(StringLiteral, isOrdinary) { return Node.isOrdinary(); }
+} // namespace
+
+UseStdFormatCheck::UseStdFormatCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  StrictMode(Options.getLocalOrGlobal("StrictMode", false)),
+  StrFormatLikeFunctions(utils::options::parseStringList(
+  Options.get("StrFormatLikeFunctions", ""))),
+  ReplacementFormatFunction(
+  Options.get("ReplacementFormatFunction", "std::format")),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),
+  areDiagsSelfContained()),
+  MaybeHeaderToInclude(Options.get("FormatHeader")) {
+  if (StrFormatLikeFunctions.empty())
+StrFormatLikeFunctions.push_back("absl::StrFormat");
+
+  if (!MaybeHeaderToInclude && ReplacementFormatFunction == "std::format")
+MaybeHeaderToInclude = "";
+}
+
+void UseStdFormatCheck::registerPPCallbacks(const SourceManager ,
+Preprocessor *PP,
+Preprocessor *ModuleExpanderPP) {
+  IncludeInserter.registerPreprocessor(PP);
+}
+
+void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  callExpr(argumentCountAtLeast(1),
+   hasArgument(0, stringLiteral(isOrdinary())),
+   callee(functionDecl(unless(cxxMethodDecl()),
+   matchers::matchesAnyListedName(
+   StrFormatLikeFunctions))
+  .bind("func_decl")))
+  .bind("strformat"),
+  this);
+}
+
+void UseStdFormatCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  using utils::options::serializeStringList;
+  Options.store(Opts, "StrictMode", StrictMode);
+  Options.store(Opts, "StrFormatLikeFunctions",
+serializeStringList(StrFormatLikeFunctions));
+  Options.store(Opts, "ReplacementFormatFunction", ReplacementFormatFunction);
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+  if (MaybeHeaderToInclude)
+Options.store(Opts, "FormatHeader", *MaybeHeaderToInclude);
+}
+
+void UseStdFormatCheck::check(const MatchFinder::MatchResult ) {
+  const unsigned FormatArgOffset = 0;
+  const auto *OldFunction = Result.Nodes.getNodeAs("func_decl");
+  const auto *StrFormat = Result.Nodes.getNodeAs("strformat");
+
+  utils::FormatStringConverter::Configuration ConverterConfig;
+  ConverterConfig.StrictMode = StrictMode;
+  utils::FormatStringConverter Converter(Result.Context, StrFormat,
+ FormatArgOffset, ConverterConfig,
+ getLangOpts());
+  const Expr *StrFormatCall = StrFormat->getCallee();
+  if (!Converter.canApply()) {
+DiagnosticBuilder Diag = diag(StrFormat->getBeginLoc(),

5chmidti wrote:

This `Diag` variable is not needed, you can call `diag` by itself without 
creating the variable

https://github.com/llvm/llvm-project/pull/90397
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] use existing functions for code locations in the scopify enum tweak (PR #88737)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti closed 
https://github.com/llvm/llvm-project/pull/88737
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] use existing functions for code locations in the scopify enum tweak (PR #88737)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/88737
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix false-positives for templates in `bugprone-return-const-ref-from-parameter` (PR #90273)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti updated 
https://github.com/llvm/llvm-project/pull/90273

>From 9b5bf4e1d53b3a55f9290199aeccb02c20a1e2cc Mon Sep 17 00:00:00 2001
From: Julian Schmidt 
Date: Fri, 26 Apr 2024 22:49:38 +0200
Subject: [PATCH 1/2] [clang-tidy] fix false-positives for templates in
 `bugprone-return-const-ref-from-parameter`

In the AST for function templates, the return will be a DeclRefExpr,
even if the return type differs from that of the returned variable.
Protect against false-positives by constraining the canonical return
type to be that of the parameter.
Also streams the source range of the returned expression into the
diagnostic.
---
 .../ReturnConstRefFromParameterCheck.cpp  |  10 +-
 .../return-const-ref-from-parameter.cpp   | 114 ++
 2 files changed, 121 insertions(+), 3 deletions(-)

diff --git 
a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
index 8ae37d4f774d23..b3f7dd6d1c86f8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
@@ -17,8 +17,11 @@ namespace clang::tidy::bugprone {
 
 void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  returnStmt(hasReturnValue(declRefExpr(to(parmVarDecl(hasType(
- hasCanonicalType(matchers::isReferenceToConst(
+  returnStmt(
+  hasReturnValue(declRefExpr(to(parmVarDecl(hasType(hasCanonicalType(
+  qualType(matchers::isReferenceToConst()).bind("type"))),
+  hasAncestor(functionDecl(hasReturnTypeLoc(
+  loc(qualType(hasCanonicalType(equalsBoundNode("type"
   .bind("ret"),
   this);
 }
@@ -28,7 +31,8 @@ void ReturnConstRefFromParameterCheck::check(
   const auto *R = Result.Nodes.getNodeAs("ret");
   diag(R->getRetValue()->getBeginLoc(),
"returning a constant reference parameter may cause a use-after-free "
-   "when the parameter is constructed from a temporary");
+   "when the parameter is constructed from a temporary")
+  << R->getRetValue()->getSourceRange();
 }
 
 } // namespace clang::tidy::bugprone
diff --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
index a83a019ec7437d..205d93606993e1 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
@@ -4,6 +4,15 @@ using T = int;
 using TConst = int const;
 using TConstRef = int const&;
 
+template 
+struct Wrapper { Wrapper(T); };
+
+template 
+struct Identity { using type = T; };
+
+template 
+struct ConstRef { using type = const T&; };
+
 namespace invalid {
 
 int const (int const ) { return a; }
@@ -18,8 +27,59 @@ int const (TConstRef a) { return a; }
 int const (TConst ) { return a; }
 // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: returning a constant reference 
parameter
 
+template 
+const T& tf1(const T ) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: returning a constant reference 
parameter
+
+template 
+const T& itf1(const T ) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: returning a constant reference 
parameter
+
+template 
+typename ConstRef::type itf2(const T ) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: returning a constant reference 
parameter
+
+template 
+typename ConstRef::type itf3(typename ConstRef::type a) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:72: warning: returning a constant reference 
parameter
+
+template 
+const T& itf4(typename ConstRef::type a) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: returning a constant reference 
parameter
+
+void instantiate(const int , const float , int _param, float 
_paramf) {
+itf1(0);
+itf1(param);
+itf1(paramf);
+itf2(0);
+itf2(param);
+itf2(paramf);
+itf3(0);
+itf3(param);
+itf3(paramf);
+itf4(0);
+itf4(param);
+itf4(paramf);
+}
+
+struct C {
+const C& foo(const C) { return c; }
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: returning a constant reference 
parameter
+};
+
 } // namespace invalid
 
+namespace false_negative_because_dependent_and_not_instantiated {
+template 
+typename ConstRef::type tf2(const T ) { return a; }
+
+template 
+typename ConstRef::type tf3(typename ConstRef::type a) { return a; }
+
+template 
+const T& tf4(typename ConstRef::type a) { return a; }
+} // false_negative_because_dependent_and_not_instantiated
+
 namespace valid {
 
 int const (int ) { return a; }
@@ -28,4 +88,58 @@ int const (int &) { return a; }
 
 int f1(int const ) { return a; }
 
+template 
+T 

[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/83412
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti commented:

When the enumerators start with the enum name, but the names contain a `_` as a 
separator, then applying this tweak will result in `_`-prefixed enumerators. 
Can you please handle that case as well and remove the `_`? Otherwise, this 
looks good to me

https://github.com/llvm/llvm-project/pull/83412
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Handle implicit casts in hicpp-signed-bitwise for IgnorePositiveIntegerLiterals (PR #90621)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.


https://github.com/llvm/llvm-project/pull/90621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (PR #90736)

[Julian Schmidt via cfe-commits]

@@ -380,3 +387,20 @@ namespace PR51861 {
 // CHECK-FIXES: {{^}}PR51861::Foo::getBar();{{$}}
   }
 }
+
+namespace PR75163 {

5chmidti wrote:

Nit: `GH` instead of `PR`?

https://github.com/llvm/llvm-project/pull/90736
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (PR #90736)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/90736
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Handle expr with side-effects in readability-static-accessed-through-instance (PR #90736)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/90736
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Relax readability-const-return-type (PR #90560)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.


https://github.com/llvm/llvm-project/pull/90560
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Ignore casts from void to void in bugprone-casting-through-void (PR #90566)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/90566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Ignore unevaluated context in bugprone-optional-value-conversion (PR #90410)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti commented:

Do we maybe want to explicitly match `decltype`, `sizeof`, `static_cast` etc. 
instead of `typeLoc`?
Because I don't think we would want to ignore https://godbolt.org/z/r3bK961do

```c++
#include 

constexpr std::optional foo() { return 42; }

constexpr int select(std::optional val) { return val.value_or(0); }

template 
struct bar {};

void use() { using F = bar; }
```

(There are other checks with this ignore pattern, depending on the answer to my 
question we should check those as well)

https://github.com/llvm/llvm-project/pull/90410
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -164,26 +164,32 @@ TEST_P(ASTMatchersTest, AllOf) {
  "void g(int x) { struct T t; f(x, , 3, 4); }";
   EXPECT_TRUE(matches(
   Program, callExpr(allOf(callee(functionDecl(hasName("f"))),
-  hasArgument(0, declRefExpr(to(varDecl(;
+  hasArgument(0, ignoringParenImpCasts(declRefExpr(
+ to(varDecl();
   EXPECT_TRUE(matches(
   Program,
-  callExpr(
-  allOf(callee(functionDecl(hasName("f"))),
-hasArgument(0, declRefExpr(to(varDecl(,
-hasArgument(1, 
hasType(pointsTo(recordDecl(hasName("T");
+  callExpr(allOf(
+  callee(functionDecl(hasName("f"))),
+  hasArgument(0, ignoringParenImpCasts(declRefExpr(to(varDecl(),
+  hasArgument(1, ignoringParenImpCasts(
+ hasType(pointsTo(recordDecl(hasName("T"));
   EXPECT_TRUE(matches(
-  Program, callExpr(allOf(
-   callee(functionDecl(hasName("f"))),
-   hasArgument(0, declRefExpr(to(varDecl(,
-   hasArgument(1, hasType(pointsTo(recordDecl(hasName("T"),
-   hasArgument(2, integerLiteral(equals(3)));
+  Program,
+  callExpr(allOf(
+  callee(functionDecl(hasName("f"))),
+  hasArgument(0, ignoringParenImpCasts(declRefExpr(to(varDecl(),
+  hasArgument(1, ignoringParenImpCasts(
+ hasType(pointsTo(recordDecl(hasName("T")),
+  hasArgument(2, ignoringParenImpCasts(integerLiteral(equals(3;
   EXPECT_TRUE(matches(
-  Program, callExpr(allOf(
-   callee(functionDecl(hasName("f"))),
-   hasArgument(0, declRefExpr(to(varDecl(,
-   hasArgument(1, hasType(pointsTo(recordDecl(hasName("T"),
-   hasArgument(2, integerLiteral(equals(3))),
-   hasArgument(3, integerLiteral(equals(4)));
+  Program,
+  callExpr(allOf(
+  callee(functionDecl(hasName("f"))),
+  hasArgument(0, ignoringParenImpCasts(declRefExpr(to(varDecl(),
+  hasArgument(1, ignoringParenImpCasts(
+ hasType(pointsTo(recordDecl(hasName("T")),
+  hasArgument(2, ignoringParenImpCasts(integerLiteral(equals(3,
+  hasArgument(3, ignoringParenImpCasts(integerLiteral(equals(4;

5chmidti wrote:

Please remove the `ignoringParenImpCasts` from all `hasArgument(1, ` matchers. 
These only care about the type, for which implicit nodes do not play a role for 
these matchers (fwict).

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -44,9 +44,10 @@ void StringFindStartswithCheck::registerMatchers(MatchFinder 
*Finder) {
   callee(cxxMethodDecl(hasName("find")).bind("findfun")),
   on(hasType(StringType)),
   // ... with some search expression ...
-  hasArgument(0, expr().bind("needle")),
+  hasArgument(0, expr().ignoringParenImpCasts().bind("needle")),

5chmidti wrote:

There is also no need to add `ignoringParenImpCasts` to `needle`. All the 
needle node is used for is to get the source code, which does not care if we 
are at an implicit node or not (ln. 88, 103-104, 123). Leaving out the 
`ignoringParenImpCasts` will also keep a user's parentheses in-place, if they 
wrote them.

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -97,11 +97,12 @@ AST_MATCHER_FUNCTION_P(StatementMatcher, 
isConstRefReturningMethodCall,
   hasCanonicalType(recordType(hasDeclaration(namedDecl(
   unless(matchers::matchesAnyListedName(ExcludedContainerTypes));
 
-  return expr(
-  anyOf(cxxMemberCallExpr(callee(MethodDecl), on(OnExpr),
-  thisPointerType(ReceiverType)),
-cxxOperatorCallExpr(callee(MethodDecl), hasArgument(0, OnExpr),
-hasArgument(0, hasType(ReceiverType);
+  return expr(anyOf(
+  cxxMemberCallExpr(callee(MethodDecl), on(OnExpr),
+thisPointerType(ReceiverType)),
+  cxxOperatorCallExpr(
+  callee(MethodDecl), hasArgument(0, ignoringParenImpCasts(OnExpr)),
+  hasArgument(0, ignoringParenImpCasts(hasType(ReceiverType));

5chmidti wrote:

This `ignoringParenImpCasts` shouldn't be needed

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -46,20 +46,20 @@ void SlicingCheck::registerMatchers(MatchFinder *Finder) {
   isBaseInitializer(), withInitializer(equalsBoundNode("Call"));
 
   // Assignment slicing: "a = b;" and "a = std::move(b);" variants.
-  const auto SlicesObjectInAssignment =
-  callExpr(expr().bind("Call"),
-   callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
-  isMoveAssignmentOperator()),
-OfBaseClass)),
-   hasArgument(1, HasTypeDerivedFromBaseDecl));
+  const auto SlicesObjectInAssignment = callExpr(
+  expr().bind("Call"),
+  callee(cxxMethodDecl(
+  anyOf(isCopyAssignmentOperator(), isMoveAssignmentOperator()),
+  OfBaseClass)),
+  hasArgument(1, ignoringParenImpCasts(HasTypeDerivedFromBaseDecl)));

5chmidti wrote:

The `ignoringParenImpCasts` is not needed for the types we match.

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -30,9 +30,9 @@ void 
UpgradeDurationConversionsCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   cxxOperatorCallExpr(
   argumentCountIs(2),
-  hasArgument(
-  0, expr(hasType(cxxRecordDecl(hasName("::absl::Duration"),
-  hasArgument(1, expr().bind("arg")),
+  hasArgument(0, ignoringParenImpCasts(expr(hasType(
+ cxxRecordDecl(hasName("::absl::Duration")),
+  hasArgument(1, ignoringParenImpCasts(expr().bind("arg"))),

5chmidti wrote:

I don't think that we need the `ignoringParenImpCasts` for the 0-th argument, 
because we want to know if it is called on an `::absl::Duration` and not on 
something that was implicitly constructed from one. I know that the original 
`hasArgument` did include the `ignoringParenImpCasts`, but I think its better 
to leave it out. Thoughts?

The `ignoringParenImpCasts` from the 1-st argument can be removed.

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -82,8 +82,8 @@ void StringConstructorCheck::registerMatchers(MatchFinder 
*Finder) {
   Finder->addMatcher(
   cxxConstructExpr(
   hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
-  hasArgument(0, hasType(qualType(isInteger(,
-  hasArgument(1, hasType(qualType(isInteger(,
+  hasArgument(0, 
ignoringParenImpCasts(hasType(qualType(isInteger(),
+  hasArgument(1, 
ignoringParenImpCasts(hasType(qualType(isInteger(),

5chmidti wrote:

I don't *think* that these are needed/should be here, thoughts?

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -102,8 +103,8 @@ void StringConstructorCheck::registerMatchers(MatchFinder 
*Finder) {
   cxxConstructExpr(
   hasDeclaration(cxxConstructorDecl(ofClass(
   cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)),
-  hasArgument(0, hasType(CharPtrType)),
-  hasArgument(1, hasType(isInteger())),
+  hasArgument(0, ignoringParenImpCasts(hasType(CharPtrType))),
+  hasArgument(1, ignoringParenImpCasts(hasType(isInteger(,

5chmidti wrote:

Same as the other two types matchers

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -75,9 +76,10 @@ void 
UpgradeDurationConversionsCheck::registerMatchers(MatchFinder *Finder) {
hasParent(functionTemplateDecl()),
unless(hasTemplateArgument(0, refersToType(builtinType(,
hasName("::absl::operator*"))),
-   argumentCountIs(2), hasArgument(0, expr().bind("arg")),
-   hasArgument(1, expr(hasType(
-  
cxxRecordDecl(hasName("::absl::Duration"))
+   argumentCountIs(2),
+   hasArgument(0, ignoringParenImpCasts(expr().bind("arg"))),
+   hasArgument(1, ignoringParenImpCasts(expr(hasType(cxxRecordDecl(
+  hasName("::absl::Duration")))

5chmidti wrote:

Same as the first comment in this file

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

5chmidti wrote:

Please remove the formatting changes

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -62,9 +63,9 @@ void 
UpgradeDurationConversionsCheck::registerMatchers(MatchFinder *Finder) {
unless(hasTemplateArgument(0, refersToType(builtinType(,
hasAnyName("::absl::operator*", "::absl::operator/"))),
argumentCountIs(2),
-   hasArgument(0, expr(hasType(
-  
cxxRecordDecl(hasName("::absl::Duration"),
-   hasArgument(1, expr().bind("arg")))
+   hasArgument(0, ignoringParenImpCasts(expr(hasType(cxxRecordDecl(
+  hasName("::absl::Duration")),
+   hasArgument(1, ignoringParenImpCasts(expr().bind("arg"

5chmidti wrote:

Same as the first comment in this file

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -22,9 +22,10 @@ void DurationDivisionCheck::registerMatchers(MatchFinder 
*Finder) {
   traverse(TK_AsIs,
implicitCastExpr(
hasSourceExpression(ignoringParenCasts(
-   cxxOperatorCallExpr(hasOverloadedOperatorName("/"),
-   hasArgument(0, DurationExpr),
-   hasArgument(1, DurationExpr))
+   cxxOperatorCallExpr(
+   hasOverloadedOperatorName("/"),
+   hasArgument(0, ignoringParenImpCasts(DurationExpr)),
+   hasArgument(1, ignoringParenImpCasts(DurationExpr)))

5chmidti wrote:

I don't think we need to ignore implicit nodes here

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -41,14 +41,17 @@ void 
StringLiteralWithEmbeddedNulCheck::registerMatchers(MatchFinder *Finder) {
hasDeclaration(cxxMethodDecl(hasName("basic_string",
   // If present, the second argument is the alloc object which must not
   // be present explicitly.
-  cxxConstructExpr(argumentCountIs(2),
-   hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
-   hasArgument(1, cxxDefaultArgExpr();
+  cxxConstructExpr(
+  argumentCountIs(2),
+  hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
+  hasArgument(1, ignoringParenImpCasts(cxxDefaultArgExpr());
 
   // Detect passing a suspicious string literal to a string constructor.
   // example: std::string str = "abc\0def";
-  Finder->addMatcher(traverse(TK_AsIs,
-  cxxConstructExpr(StringConstructorExpr, hasArgument(0, StrLitWithNul))),
+  Finder->addMatcher(
+  traverse(TK_AsIs, cxxConstructExpr(StringConstructorExpr,
+ hasArgument(0, ignoringParenImpCasts(
+StrLitWithNul,

5chmidti wrote:

The `StrLitWithNul` already ignores these implicit nodes, so the 
`ignoringParenImpCasts` is not needed.

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -36,19 +36,21 @@ void 
DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) {
 // e.g. `absl::ToDoubleSeconds(dur)`.
 auto InverseFunctionMatcher = callExpr(
 callee(functionDecl(hasAnyName(FloatConversion, IntegerConversion))),
-hasArgument(0, expr().bind("arg")));
+hasArgument(0, ignoringParenImpCasts(expr().bind("arg";
 
 // Matcher which matches a duration divided by the factory_matcher above,
 // e.g. `dur / absl::Seconds(1)`.
 auto DivisionOperatorMatcher = cxxOperatorCallExpr(
-hasOverloadedOperatorName("/"), hasArgument(0, expr().bind("arg")),
-hasArgument(1, FactoryMatcher));
+hasOverloadedOperatorName("/"),
+hasArgument(0, ignoringParenImpCasts(expr().bind("arg"))),
+hasArgument(1, ignoringParenImpCasts(FactoryMatcher)));
 
 // Matcher which matches a duration argument to `FDivDuration`,
 // e.g. `absl::FDivDuration(dur, absl::Seconds(1))`
-auto FdivMatcher = callExpr(
-callee(functionDecl(hasName("::absl::FDivDuration"))),
-hasArgument(0, expr().bind("arg")), hasArgument(1, FactoryMatcher));
+auto FdivMatcher =
+callExpr(callee(functionDecl(hasName("::absl::FDivDuration"))),
+ hasArgument(0, ignoringParenImpCasts(expr().bind("arg"))),
+ hasArgument(1, ignoringParenImpCasts(FactoryMatcher)));
 

5chmidti wrote:

Please remove the `ignoringParentImpCasts` around the `expr().bind("arg")` 
matchers, they are not needed.

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)

[Julian Schmidt via cfe-commits]

@@ -98,16 +100,18 @@ void 
UpgradeDurationConversionsCheck::registerMatchers(MatchFinder *Finder) {
   //   `absl::Hours(x)`
   // where `x` is not of a built-in type.
   Finder->addMatcher(
-  traverse(TK_AsIs, implicitCastExpr(
-anyOf(hasCastKind(CK_UserDefinedConversion),
-  has(implicitCastExpr(
-  hasCastKind(CK_UserDefinedConversion,
-hasParent(callExpr(
-callee(functionDecl(
-DurationFactoryFunction(),
-
unless(hasParent(functionTemplateDecl(),
-hasArgument(0, expr().bind("arg")
-.bind("OuterExpr")),
+  traverse(
+  TK_AsIs,
+  implicitCastExpr(
+  anyOf(
+  hasCastKind(CK_UserDefinedConversion),
+  
has(implicitCastExpr(hasCastKind(CK_UserDefinedConversion,
+  hasParent(callExpr(
+  callee(
+  functionDecl(DurationFactoryFunction(),
+   unless(hasParent(functionTemplateDecl(),
+  hasArgument(0, ignoringParenImpCasts(expr().bind("arg"))

5chmidti wrote:

This change is not needed, the check does not if implicit nodes are ignored or 
not, so we don't need to do the extra work.

https://github.com/llvm/llvm-project/pull/89553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits