owenca wrote:
> This regressions was introduced in
> [70d7ea0](https://github.com/llvm/llvm-project/commit/70d7ea0cebcf363cd0ddcfb76375fb5fada87dd5).
> The commit moved some code and correctly picked up an explicit check for not
> running on Verilog. However, the moved code also never ran for JavaScript and
> after the commit we run it there and this causes the wrong formatting of:
>
> ```js
> export type Params = Config&{
> columns: Column[];
> };
> ```
>
> into
>
> ```js
> export type Params = Config&{
> columns:
> Column[];
> };
> ```
Thanks for fixing this! I somehow missed this
[part](https://github.com/llvm/llvm-project/commit/70d7ea0cebcf363cd0ddcfb76375fb5fada87dd5#diff-693c925375cc130d98d8edc63c7dcc509e0659c460452d361f466f367e508e1aL1931-L1934).
:(
https://github.com/llvm/llvm-project/pull/76233
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/75816
>From 511f3a4537267284554bf6b33470a01d747b8a94 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Sat, 16 Dec 2023 20:15:17 +0100
Subject: [PATCH 1/6] Remove -lFortran_main from the link line when -shared is
present
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++
1 file changed, 22 insertions(+)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 45901ee7157f77..5d525d3794ad1d 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1133,6 +1133,17 @@ static bool isWholeArchivePresent(const ArgList &Args) {
return WholeArchiveActive;
}
+static bool isSharedLinkage(const ArgList &Args) {
+ bool FoundSharedFlag = false;
+ for (auto *Arg : Args.filtered(options::OPT_shared)) {
+if (Arg) {
+ FoundSharedFlag = true;
+}
+ }
+
+ return FoundSharedFlag;
+}
+
/// Add Fortran runtime libs for MSVC
static void addFortranRuntimeLibsMSVC(const ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) {
@@ -1164,6 +1175,17 @@ static void addFortranRuntimeLibsMSVC(const ArgList
&Args,
// Add FortranMain runtime lib
static void addFortranMain(const ToolChain &TC, const ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) {
+ // 0. Shared-library linkage
+ // If we are attempting to link a shared library, we should not add
+ // -lFortran_main.a to the link line, as the `main` symbol is not
+ // required for a shared library and should also be provided by one
+ // of the translation units of the code that this shared library
+ // will be linked against eventually.
+ if (isSharedLinkage(Args)) {
+printf("MK: --> shared linkage, do not add -lFortranMain\n");
+return;
+ }
+
// 1. MSVC
if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
addFortranRuntimeLibsMSVC(Args, CmdArgs);
>From 930f2c447daa625d9e6019cd38d82b5750942f5d Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Mon, 18 Dec 2023 11:27:59 +0100
Subject: [PATCH 2/6] Update dynamic_linker.f90 test and clean up a bit
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++
flang/test/Driver/dynamic-linker.f90 | 8 ++--
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 5d525d3794ad1d..05ebd42829c95d 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1133,15 +1133,14 @@ static bool isWholeArchivePresent(const ArgList &Args) {
return WholeArchiveActive;
}
+/// Determine if driver is invoked to create a shared object library (-static)
static bool isSharedLinkage(const ArgList &Args) {
- bool FoundSharedFlag = false;
- for (auto *Arg : Args.filtered(options::OPT_shared)) {
-if (Arg) {
- FoundSharedFlag = true;
-}
- }
+ return Args.hasArg(options::OPT_shared);
+}
- return FoundSharedFlag;
+/// Determine if driver is invoked to create a static object library (-shared)
+static bool isStaticLinkage(const ArgList &Args) {
+ return Args.hasArg(options::OPT_static);
}
/// Add Fortran runtime libs for MSVC
@@ -1176,13 +1175,12 @@ static void addFortranRuntimeLibsMSVC(const ArgList
&Args,
static void addFortranMain(const ToolChain &TC, const ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) {
// 0. Shared-library linkage
- // If we are attempting to link a shared library, we should not add
+ // If we are attempting to link a library, we should not add
// -lFortran_main.a to the link line, as the `main` symbol is not
- // required for a shared library and should also be provided by one
- // of the translation units of the code that this shared library
+ // required for a library and should also be provided by one of
+ // the translation units of the code that this shared library
// will be linked against eventually.
- if (isSharedLinkage(Args)) {
-printf("MK: --> shared linkage, do not add -lFortranMain\n");
+ if (isSharedLinkage(Args) || isStaticLinkage(Args)) {
return;
}
diff --git a/flang/test/Driver/dynamic-linker.f90
b/flang/test/Driver/dynamic-linker.f90
index df119c22a2ea51..af07e2483f93fa 100644
--- a/flang/test/Driver/dynamic-linker.f90
+++ b/flang/test/Driver/dynamic-linker.f90
@@ -3,18 +3,22 @@
! RUN: %flang -### --target=x86_64-linux-gnu -rpath /path/to/dir -shared \
! RUN: -static %s 2>&1 | FileCheck \
-! RUN: --check-prefixes=GNU-LINKER-OPTIONS %s
+! RUN: --check-prefixes=GNU-LINKER-OPTIONS \
+! RUN: --implicit-check-not=GNU-LINKER-OPTIONS-NOT %s
! RUN: %flang -### --target=x86_64-windows-msvc -rpath /path/to/dir -shared \
! RUN: -static %s 2>&1 | FileCheck \
-! RUN: --check-prefixes=MSVC-LINKER-OPTIONS %s
+! RUN:
@@ -163,6 +163,62 @@ forward compiler options to the frontend driver,
`flang-new -fc1`.
You can read more on the design of `clangDriver` in Clang's [Driver Design &
Internals](https://clang.llvm.org/docs/DriverInternals.html).
+## Linker Driver
+When used as a linker, Flang's frontend driver assembles the command line for
an
+external linker command (e.g., LLVM's `lld`) and invokes it to create the final
+executable by linking static and shared libraries together with all the
+translation units supplied as object files.
+
+By default, the Flang linker driver adds several libraries to the linker
+invocation to make sure that all entrypoints for program start
+(Fortran's program unit) and runtime routines can be resolved by the linker.
+
+An abridged example (only showing the Fortran specific linker flags, omission
+indicated by `[...]`) for such a linker invocation on a Linux system would look
+like this:
+
+```
+$ flang -v -o example example.o
+"/usr/bin/ld" [...] example.o [...] "--whole-archive" "-lFortran_main"
+"--no-whole-archive" "-lFortranRuntime" "-lFortranDecimal" [...]
+```
+
+The automatically added libraries are:
+
+* `Fortran_main`: Provides the main entry point `main` that then invokes
+ `_QQmain` with the Fortran program unit. This library has a dependency to
+ the `FortranRuntime` library.
+* `FortranRuntime`: Provides most of the Flang runtime library.
+* `FortranDecimal`: Provides operations for decimal numbers.
+
+The default is that, when using Flang as the linker, one of the Fortran
+translation units provides the program unit and therefore it is assumed that
+Fortran is the main code part (calling into C/C++ routines via `BIND (C)`
+interfaces). When composing the linker commandline, Flang uses
+`--whole-archive` and `--no-whole-archive` (Windows: `/WHOLEARCHIVE:`,
+Darwin & AIX: *not implemented yet*) to make sure that all for `Fortran_main`
+is processed by the linker. This is done to issue a proper error message when
+multiple definitions of `main` occur. This happens, for instance, when linking
+a code that has a Fortran program unit with a C/C++ code that also defines a
+`main` function. A user may be required to explicitly provide the C++ runtime
+libraries at link time (e.g., via `-lstdc++` for STL)
+
+If the code is C/C++ based and invokes Fortran routines, one can either use
Clang
+for Flang as the linker driver. If Clang is used, it will automatically all
+required runtime libraries needed by C++ (e.g., for STL) to the linker
invocation.
mjklemm wrote:
I treated clang and clang++ the same here, as they seem to have reasonable
logic to figure out if C or C++ is compiled and linked,
https://github.com/llvm/llvm-project/pull/75816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -163,6 +163,62 @@ forward compiler options to the frontend driver,
`flang-new -fc1`.
You can read more on the design of `clangDriver` in Clang's [Driver Design &
Internals](https://clang.llvm.org/docs/DriverInternals.html).
+## Linker Driver
+When used as a linker, Flang's frontend driver assembles the command line for
an
+external linker command (e.g., LLVM's `lld`) and invokes it to create the final
+executable by linking static and shared libraries together with all the
+translation units supplied as object files.
+
+By default, the Flang linker driver adds several libraries to the linker
+invocation to make sure that all entrypoints for program start
+(Fortran's program unit) and runtime routines can be resolved by the linker.
+
+An abridged example (only showing the Fortran specific linker flags, omission
+indicated by `[...]`) for such a linker invocation on a Linux system would look
+like this:
+
+```
+$ flang -v -o example example.o
+"/usr/bin/ld" [...] example.o [...] "--whole-archive" "-lFortran_main"
+"--no-whole-archive" "-lFortranRuntime" "-lFortranDecimal" [...]
+```
+
+The automatically added libraries are:
+
+* `Fortran_main`: Provides the main entry point `main` that then invokes
+ `_QQmain` with the Fortran program unit. This library has a dependency to
+ the `FortranRuntime` library.
+* `FortranRuntime`: Provides most of the Flang runtime library.
+* `FortranDecimal`: Provides operations for decimal numbers.
+
+The default is that, when using Flang as the linker, one of the Fortran
+translation units provides the program unit and therefore it is assumed that
+Fortran is the main code part (calling into C/C++ routines via `BIND (C)`
+interfaces). When composing the linker commandline, Flang uses
+`--whole-archive` and `--no-whole-archive` (Windows: `/WHOLEARCHIVE:`,
+Darwin & AIX: *not implemented yet*) to make sure that all for `Fortran_main`
+is processed by the linker. This is done to issue a proper error message when
+multiple definitions of `main` occur. This happens, for instance, when linking
+a code that has a Fortran program unit with a C/C++ code that also defines a
+`main` function. A user may be required to explicitly provide the C++ runtime
+libraries at link time (e.g., via `-lstdc++` for STL)
+
+If the code is C/C++ based and invokes Fortran routines, one can either use
Clang
+for Flang as the linker driver. If Clang is used, it will automatically all
+required runtime libraries needed by C++ (e.g., for STL) to the linker
invocation.
+In this case, one has to explicitly provide the Fortran runtime libraries
+`FortranRuntime` and/or `FortranDecimal`. An alternative is to use Flang to
link
+and use the `-fno-fortran-main` flag. This flag removes
+`Fortran_main` from the linker stage and hence requires one of the C/C++
+translation units to provide a definition of the `main` function. In this case,
+it may be required to explicitly supply C++ runtime libraries as mentioned
above.
+
+When creating shared or static libraries, `Fortran_main` is automatically
mjklemm wrote:
Applied! Thanks for the rewording!
https://github.com/llvm/llvm-project/pull/75816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/75816
>From 511f3a4537267284554bf6b33470a01d747b8a94 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Sat, 16 Dec 2023 20:15:17 +0100
Subject: [PATCH 1/7] Remove -lFortran_main from the link line when -shared is
present
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++
1 file changed, 22 insertions(+)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 45901ee7157f77..5d525d3794ad1d 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1133,6 +1133,17 @@ static bool isWholeArchivePresent(const ArgList &Args) {
return WholeArchiveActive;
}
+static bool isSharedLinkage(const ArgList &Args) {
+ bool FoundSharedFlag = false;
+ for (auto *Arg : Args.filtered(options::OPT_shared)) {
+if (Arg) {
+ FoundSharedFlag = true;
+}
+ }
+
+ return FoundSharedFlag;
+}
+
/// Add Fortran runtime libs for MSVC
static void addFortranRuntimeLibsMSVC(const ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) {
@@ -1164,6 +1175,17 @@ static void addFortranRuntimeLibsMSVC(const ArgList
&Args,
// Add FortranMain runtime lib
static void addFortranMain(const ToolChain &TC, const ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) {
+ // 0. Shared-library linkage
+ // If we are attempting to link a shared library, we should not add
+ // -lFortran_main.a to the link line, as the `main` symbol is not
+ // required for a shared library and should also be provided by one
+ // of the translation units of the code that this shared library
+ // will be linked against eventually.
+ if (isSharedLinkage(Args)) {
+printf("MK: --> shared linkage, do not add -lFortranMain\n");
+return;
+ }
+
// 1. MSVC
if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
addFortranRuntimeLibsMSVC(Args, CmdArgs);
>From 930f2c447daa625d9e6019cd38d82b5750942f5d Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Mon, 18 Dec 2023 11:27:59 +0100
Subject: [PATCH 2/7] Update dynamic_linker.f90 test and clean up a bit
---
clang/lib/Driver/ToolChains/CommonArgs.cpp | 22 ++
flang/test/Driver/dynamic-linker.f90 | 8 ++--
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 5d525d3794ad1d..05ebd42829c95d 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1133,15 +1133,14 @@ static bool isWholeArchivePresent(const ArgList &Args) {
return WholeArchiveActive;
}
+/// Determine if driver is invoked to create a shared object library (-static)
static bool isSharedLinkage(const ArgList &Args) {
- bool FoundSharedFlag = false;
- for (auto *Arg : Args.filtered(options::OPT_shared)) {
-if (Arg) {
- FoundSharedFlag = true;
-}
- }
+ return Args.hasArg(options::OPT_shared);
+}
- return FoundSharedFlag;
+/// Determine if driver is invoked to create a static object library (-shared)
+static bool isStaticLinkage(const ArgList &Args) {
+ return Args.hasArg(options::OPT_static);
}
/// Add Fortran runtime libs for MSVC
@@ -1176,13 +1175,12 @@ static void addFortranRuntimeLibsMSVC(const ArgList
&Args,
static void addFortranMain(const ToolChain &TC, const ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) {
// 0. Shared-library linkage
- // If we are attempting to link a shared library, we should not add
+ // If we are attempting to link a library, we should not add
// -lFortran_main.a to the link line, as the `main` symbol is not
- // required for a shared library and should also be provided by one
- // of the translation units of the code that this shared library
+ // required for a library and should also be provided by one of
+ // the translation units of the code that this shared library
// will be linked against eventually.
- if (isSharedLinkage(Args)) {
-printf("MK: --> shared linkage, do not add -lFortranMain\n");
+ if (isSharedLinkage(Args) || isStaticLinkage(Args)) {
return;
}
diff --git a/flang/test/Driver/dynamic-linker.f90
b/flang/test/Driver/dynamic-linker.f90
index df119c22a2ea51..af07e2483f93fa 100644
--- a/flang/test/Driver/dynamic-linker.f90
+++ b/flang/test/Driver/dynamic-linker.f90
@@ -3,18 +3,22 @@
! RUN: %flang -### --target=x86_64-linux-gnu -rpath /path/to/dir -shared \
! RUN: -static %s 2>&1 | FileCheck \
-! RUN: --check-prefixes=GNU-LINKER-OPTIONS %s
+! RUN: --check-prefixes=GNU-LINKER-OPTIONS \
+! RUN: --implicit-check-not=GNU-LINKER-OPTIONS-NOT %s
! RUN: %flang -### --target=x86_64-windows-msvc -rpath /path/to/dir -shared \
! RUN: -static %s 2>&1 | FileCheck \
-! RUN: --check-prefixes=MSVC-LINKER-OPTIONS %s
+! RUN:
hstk30-hw wrote:
Use bitfield still a mess up because the different arm version have different
Instrinsics available even in the same bit group.
I will try it, hope it readable.
https://github.com/llvm/llvm-project/pull/75440
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ShamrockLee wrote:
Some commits doesn't relate to the GitHub account correctly. This may result
from incorrect `user.name` and `user.email` settings in your local Git
repository.
This error can be inspected by comparing the `From: ` field of the following
patches:
Misconfigured:
https://github.com/llvm/llvm-project/commit/0eb58740f33f2eef29c28e43e78118f9f0eea4b4.patch
Correct:
https://github.com/llvm/llvm-project/commit/5e6326fb1cf4f1591fe927c94b1d16d1a7be0e66.patch
This can be fixed by
```sh
gt config user.name "Nhat Nguyen"
git config user.email "nhat7...@gmail.com"
git fetch
git branch 71675-backup 71675
git switch -C 71675 /71675
git rebase --exec "git commit --amend --reset-author --no-edit" HEAD~4
### Double check before proceeding
git push -f
```
https://github.com/llvm/llvm-project/pull/75902
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -target-feature +sse2 < %s
| FileCheck %s --check-prefixes=CHECK
RKSimon wrote:
OK - so we have test coverage for non-SSE cases already in the backend?
https://github.com/llvm/llvm-project/pull/75156
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hnakamura5 wrote:
Thank you for the information. I added the document.
https://github.com/llvm/llvm-project/pull/76059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -4656,6 +4687,15 @@ struct FormatStyle {
/// \version 8
std::vector StatementMacros;
+ /// Tablegen
+ bool TableGenAllowBreakBeforeInheritColon;
+ bool TableGenAllowBreakAfterInheritColon;
hnakamura5 wrote:
Thank you for the information. I removed these options. Instead
BreakInheritanceList works well. There happens some changes, but seems
acceptable.
https://github.com/llvm/llvm-project/pull/76059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -40,6 +40,13 @@ class FormatTestTableGen : public ::testing::Test {
EXPECT_EQ(Code.str(), format(Code)) << "Expected code is not stable";
EXPECT_EQ(Code.str(), format(test::messUp(Code)));
}
+
+ static void verifyFormat(llvm::StringRef Code, const FormatStyle &Style) {
hnakamura5 wrote:
I have not noticed that. Thank you. I can also fix this. But now I am planing
whether (and how) to split this growing pull request. A kind of refactoring may
be better done after that.
https://github.com/llvm/llvm-project/pull/76059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hnakamura5 wrote:
@rymiel @HazardyKnusperkeks
Thank you for your review!
I have fixed the points.
But for refactoring of the test base class in
https://github.com/llvm/llvm-project/commit/f8d10d5ac9ab4b45b388c74357fc82fb96562e66
.
I'm not sure I should do here, and if I should, I should do it in splitted pull
request.
Now I really understand I should split this pull request into some parts. At
first it is large and continue growing by adding documents.
I'm wondering how and current plan is separating semantically,
- Handling multi line string (~100 lines).
- Handling numeric like identifier (~100 lines).
- Handling TableGen specific keywords (~100 lines)
- Unwrapped line parsing(~100 lines).
- Parse TableGen values (about 500+ lines including unittest).
- Basic options (but for aligning ones) (about 500+ lines including the
document).
- Aligning options (about 100 lines including document).
- Refactor unittests.
I'm not sure this is good plan. They may be complicated.
Could you help me to plan if you have some idea?
In addition, I do not know the appropriate way to split pull request after I
made one. Is it enough to refer each other, and abort this at last?
https://github.com/llvm/llvm-project/pull/76059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?=
Message-ID:
In-Reply-To:
@@ -221,9 +221,8 @@ New checks
- New :doc:`readability-avoid-return-with-void-value
` check.
- Complains about statements returning expressions of type ``void``. It can be
- confusing if a function returns an expression even though its return type is
- ``void``.
+ Finds return statements with ``void`` values used within functions with
PiotrZSL wrote:
synchronize class description, release note and first sentence in documentation.
https://github.com/llvm/llvm-project/pull/76249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vsapsai wrote:
I'll need to test this change on the real code to see the consequences. Sorry,
I'll be able to do that in January only.
https://github.com/llvm/llvm-project/pull/76119
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?= ,
Danny =?utf-8?q?Mösch?=
Message-ID:
In-Reply-To:
PiotrZSL wrote:
One more thing, there are issues like this reported in gtest, soe maybe would
be good to add IgnoreMacros options like in other checks.
https://github.com/llvm/llvm-project/pull/76249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,121 @@
+//===--- MatchFilePath.cpp - Match file path with pattern ---*- 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
+//
+//===--===//
+///
+/// \file
+/// This file implements the functionality of matching a file path name to
+/// a pattern, similar to the POSIX fnmatch() function.
+///
+//===--===//
+
+#include "MatchFilePath.h"
+
+using namespace llvm;
+
+namespace clang {
+namespace format {
+
+// Check whether `FilePath` matches `Pattern` based on POSIX Section 2.13.
owenca wrote:
```suggestion
// Check whether `FilePath` matches `Pattern` based on POSIX (1003.1-2008)
// 2.13.1, 2.13.2, and Rule 1 of 2.13.3.
```
https://github.com/llvm/llvm-project/pull/76021
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/owenca updated
https://github.com/llvm/llvm-project/pull/76021
>From 4954f52278ca41652be79318843d3538a2eb1205 Mon Sep 17 00:00:00 2001
From: Owen Pan
Date: Tue, 19 Dec 2023 23:25:57 -0800
Subject: [PATCH 1/3] [clang-format] Add an fnmatch-like function for
.clang-format-ignore
This is needed because Windows doesn't have anything equivalent to the POSIX
fnmatch() function.
---
clang/lib/Format/CMakeLists.txt | 1 +
clang/lib/Format/MatchFilePath.cpp | 121 +
clang/lib/Format/MatchFilePath.h | 22 +++
clang/unittests/Format/CMakeLists.txt| 1 +
clang/unittests/Format/MatchFilePathTest.cpp | 169 +++
5 files changed, 314 insertions(+)
create mode 100644 clang/lib/Format/MatchFilePath.cpp
create mode 100644 clang/lib/Format/MatchFilePath.h
create mode 100644 clang/unittests/Format/MatchFilePathTest.cpp
diff --git a/clang/lib/Format/CMakeLists.txt b/clang/lib/Format/CMakeLists.txt
index 015ec7c0cc84e3..84a3c136f650a8 100644
--- a/clang/lib/Format/CMakeLists.txt
+++ b/clang/lib/Format/CMakeLists.txt
@@ -11,6 +11,7 @@ add_clang_library(clangFormat
IntegerLiteralSeparatorFixer.cpp
MacroCallReconstructor.cpp
MacroExpander.cpp
+ MatchFilePath.cpp
NamespaceEndCommentsFixer.cpp
ObjCPropertyAttributeOrderFixer.cpp
QualifierAlignmentFixer.cpp
diff --git a/clang/lib/Format/MatchFilePath.cpp
b/clang/lib/Format/MatchFilePath.cpp
new file mode 100644
index 00..476af644b5f235
--- /dev/null
+++ b/clang/lib/Format/MatchFilePath.cpp
@@ -0,0 +1,121 @@
+//===--- MatchFilePath.cpp - Match file path with pattern ---*- 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
+//
+//===--===//
+///
+/// \file
+/// This file implements the functionality of matching a file path name to
+/// a pattern, similar to the POSIX fnmatch() function.
+///
+//===--===//
+
+#include "MatchFilePath.h"
+
+using namespace llvm;
+
+namespace clang {
+namespace format {
+
+// Check whether `FilePath` matches `Pattern` based on POSIX Section 2.13.
+bool matchFilePath(StringRef Pattern, StringRef FilePath) {
+ assert(!Pattern.empty());
+ assert(!FilePath.empty());
+
+ // No match if `Pattern` ends with a non-meta character not equal to the last
+ // character of `FilePath`.
+ if (const auto C = Pattern.back(); !strchr("?*]", C) && C != FilePath.back())
+return false;
+
+ constexpr auto Separator = '/';
+ const auto EOP = Pattern.size(); // End of `Pattern`.
+ const auto End = FilePath.size(); // End of `FilePath`.
+ unsigned I = 0; // Index to `Pattern`.
+
+ for (unsigned J = 0; J < End; ++J) {
+if (I == EOP)
+ return false;
+
+switch (const auto F = FilePath[J]; Pattern[I]) {
+case '\\':
+ if (++I == EOP || F != Pattern[I])
+return false;
+ break;
+case '?':
+ if (F == Separator)
+return false;
+ break;
+case '*': {
+ while (++I < EOP && Pattern[I] == '*') { // Skip consecutive stars.
+ }
+ const auto K = FilePath.find(Separator, J); // Index of next `Separator`.
+ const bool NoMoreSeparatorsInFilePath = K == StringRef::npos;
+ if (I == EOP) // `Pattern` ends with a star.
+return NoMoreSeparatorsInFilePath;
+ // `Pattern` ends with a lone backslash.
+ if (Pattern[I] == '\\' && ++I == EOP)
+return false;
+ // The star is followed by a (possibly escaped) `Separator`.
+ if (Pattern[I] == Separator) {
+if (NoMoreSeparatorsInFilePath)
+ return false;
+J = K; // Skip to next `Separator` in `FilePath`.
+break;
+ }
+ // Recurse.
+ for (auto Pat = Pattern.substr(I); J < End && FilePath[J] != Separator;
+ ++J) {
+if (matchFilePath(Pat, FilePath.substr(J)))
+ return true;
+ }
+ return false;
+}
+case '[':
+ // Skip e.g. `[!]`.
+ if (I + 3 < EOP || (I + 3 == EOP && Pattern[I + 1] != '!')) {
+// Skip unpaired `[`, brackets containing slashes, and `[]`.
+if (const auto K = Pattern.find_first_of("]/", I + 1);
+K != StringRef::npos && Pattern[K] == ']' && K > I + 1) {
+ if (F == Separator)
+return false;
+ ++I; // After the `[`.
+ bool Negated = false;
+ if (Pattern[I] == '!') {
+Negated = true;
+++I; // After the `!`.
+ }
+ bool Match = false;
+ do {
+if (I + 2 < K && Pattern[I + 1] == '-') {
+ Match = Pattern[I] <= F && F <= Pattern[I + 2];
+ I += 3; // After the range, e.g. `A-Z`.
+} else {
+
https://github.com/pav-code created
https://github.com/llvm/llvm-project/pull/76310
Fixes Issue: https://github.com/llvm/llvm-project/issues/61256
>From 82fe20f1ccc2e9129282c71bf5bdfd6cfd4fadf3 Mon Sep 17 00:00:00 2001
From: Pavel Gueorguiev
Date: Sat, 23 Dec 2023 14:19:50 -0500
Subject: [PATCH] [clang] Reword apologetic Clang diagnostic messages
Fixes Issue: https://github.com/llvm/llvm-project/issues/61256
---
clang/include/clang/Basic/DiagnosticCommonKinds.td | 4 ++--
clang/include/clang/Basic/DiagnosticSemaKinds.td | 10 +-
clang/test/CXX/drs/dr16xx.cpp | 2 +-
clang/test/CXX/drs/dr18xx.cpp | 2 +-
clang/test/Lexer/SourceLocationsOverflow.c | 2 +-
clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp | 4 ++--
clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp | 12 ++--
7 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 65a33f61a6948a..aceaa518d55ea9 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -355,9 +355,9 @@ def err_cannot_open_file : Error<"cannot open file '%0':
%1">, DefaultFatal;
def err_file_modified : Error<
"file '%0' modified since it was first processed">, DefaultFatal;
def err_file_too_large : Error<
- "sorry, unsupported: file '%0' is too large for Clang to process">;
+ "file '%0' is too large for Clang to process">;
def err_sloc_space_too_large : Error<
- "sorry, the translation unit is too large for Clang to process: ran out of
source locations">, DefaultFatal;
+ "translation unit is too large for Clang to process: ran out of source
locations">, DefaultFatal;
def err_unsupported_bom : Error<"%0 byte order mark detected in '%1', but "
"encoding is not supported">, DefaultFatal;
def err_unable_to_rename_temp : Error<
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index aebb7d9b945c33..dbe2b16b5fc39f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5135,7 +5135,7 @@ def err_non_type_template_arg_subobject : Error<
def err_non_type_template_arg_addr_label_diff : Error<
"template argument / label address difference / what did you expect?">;
def err_non_type_template_arg_unsupported : Error<
- "sorry, non-type template argument of type %0 is not yet supported">;
+ "non-type template argument of type %0 is not yet supported">;
def err_template_arg_not_convertible : Error<
"non-type template argument of type %0 cannot be converted to a value "
"of type %1">;
@@ -5188,7 +5188,7 @@ def err_template_arg_not_object_or_func : Error<
def err_template_arg_not_pointer_to_member_form : Error<
"non-type template argument is not a pointer to member constant">;
def err_template_arg_member_ptr_base_derived_not_supported : Error<
- "sorry, non-type template argument of pointer-to-member type %1 that refers "
+ "non-type template argument of a pointer to member type %1, that refers "
"to member %q0 of a different class is not supported yet">;
def err_template_arg_invalid : Error<
"non-type template argument '%0' is invalid">;
@@ -9913,11 +9913,11 @@ def warn_new_dangling_initializer_list : Warning<
"will be destroyed at the end of the full-expression">,
InGroup;
def warn_unsupported_lifetime_extension : Warning<
- "sorry, lifetime extension of "
+ "lifetime extension of "
"%select{temporary|backing array of initializer list}0 created "
- "by aggregate initialization using default member initializer "
+ "by aggregate initialization using a default member initializer "
"is not supported; lifetime of %select{temporary|backing array}0 "
- "will end at the end of the full-expression">, InGroup;
+ "will terminate at the end of the full-expression">, InGroup;
// For non-floating point, expressions of the form x == x or x != x
// should result in a warning, since these always evaluate to a constant.
diff --git a/clang/test/CXX/drs/dr16xx.cpp b/clang/test/CXX/drs/dr16xx.cpp
index 3f074c4d57354a..2ffeb5372ec1d2 100644
--- a/clang/test/CXX/drs/dr16xx.cpp
+++ b/clang/test/CXX/drs/dr16xx.cpp
@@ -472,7 +472,7 @@ namespace dr1696 { // dr1696: 7
const A &a = A(); // #dr1696-D1-a
};
D1 d1 = {}; // #dr1696-d1
- // since-cxx14-warning@-1 {{sorry, lifetime extension of temporary created
by aggregate initialization using default member initializer is not supported;
lifetime of temporary will end at the end of the full-expression}}
+ // since-cxx14-warning@-1 {{lifetime extension of temporary created by
aggregate initialization using a default member initializer is not supported;
lifetime of temporary will terminate at the end of the full-expression}}
// since-cxx14-note@#dr1696-D1-a {{initializing field 'a' with default
memb
github-actions[bot] wrote:
Thank you for submitting a Pull Request (PR) to the LLVM Project!
This PR will be automatically labeled and the relevant teams will be
notified.
If you wish to, you can add reviewers by using the "Reviewers" section on this
page.
If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using `@` followed by their GitHub username.
If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from
other developers.
If you have further questions, they may be answered by the [LLVM GitHub User
Guide](https://llvm.org/docs/GitHub.html).
You can also ask questions in a comment on this PR, on the [LLVM
Discord](https://discord.com/invite/xS7Z362) or on the
[forums](https://discourse.llvm.org/).
https://github.com/llvm/llvm-project/pull/76310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Pavel Gueorguiev (pav-code)
Changes
Fixes Issue: https://github.com/llvm/llvm-project/issues/61256
---
Full diff: https://github.com/llvm/llvm-project/pull/76310.diff
7 Files Affected:
- (modified) clang/include/clang/Basic/DiagnosticCommonKinds.td (+2-2)
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+5-5)
- (modified) clang/test/CXX/drs/dr16xx.cpp (+1-1)
- (modified) clang/test/CXX/drs/dr18xx.cpp (+1-1)
- (modified) clang/test/Lexer/SourceLocationsOverflow.c (+1-1)
- (modified) clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp (+2-2)
- (modified) clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp (+6-6)
``diff
diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 65a33f61a6948a..aceaa518d55ea9 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -355,9 +355,9 @@ def err_cannot_open_file : Error<"cannot open file '%0':
%1">, DefaultFatal;
def err_file_modified : Error<
"file '%0' modified since it was first processed">, DefaultFatal;
def err_file_too_large : Error<
- "sorry, unsupported: file '%0' is too large for Clang to process">;
+ "file '%0' is too large for Clang to process">;
def err_sloc_space_too_large : Error<
- "sorry, the translation unit is too large for Clang to process: ran out of
source locations">, DefaultFatal;
+ "translation unit is too large for Clang to process: ran out of source
locations">, DefaultFatal;
def err_unsupported_bom : Error<"%0 byte order mark detected in '%1', but "
"encoding is not supported">, DefaultFatal;
def err_unable_to_rename_temp : Error<
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index aebb7d9b945c33..dbe2b16b5fc39f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5135,7 +5135,7 @@ def err_non_type_template_arg_subobject : Error<
def err_non_type_template_arg_addr_label_diff : Error<
"template argument / label address difference / what did you expect?">;
def err_non_type_template_arg_unsupported : Error<
- "sorry, non-type template argument of type %0 is not yet supported">;
+ "non-type template argument of type %0 is not yet supported">;
def err_template_arg_not_convertible : Error<
"non-type template argument of type %0 cannot be converted to a value "
"of type %1">;
@@ -5188,7 +5188,7 @@ def err_template_arg_not_object_or_func : Error<
def err_template_arg_not_pointer_to_member_form : Error<
"non-type template argument is not a pointer to member constant">;
def err_template_arg_member_ptr_base_derived_not_supported : Error<
- "sorry, non-type template argument of pointer-to-member type %1 that refers "
+ "non-type template argument of a pointer to member type %1, that refers "
"to member %q0 of a different class is not supported yet">;
def err_template_arg_invalid : Error<
"non-type template argument '%0' is invalid">;
@@ -9913,11 +9913,11 @@ def warn_new_dangling_initializer_list : Warning<
"will be destroyed at the end of the full-expression">,
InGroup;
def warn_unsupported_lifetime_extension : Warning<
- "sorry, lifetime extension of "
+ "lifetime extension of "
"%select{temporary|backing array of initializer list}0 created "
- "by aggregate initialization using default member initializer "
+ "by aggregate initialization using a default member initializer "
"is not supported; lifetime of %select{temporary|backing array}0 "
- "will end at the end of the full-expression">, InGroup;
+ "will terminate at the end of the full-expression">, InGroup;
// For non-floating point, expressions of the form x == x or x != x
// should result in a warning, since these always evaluate to a constant.
diff --git a/clang/test/CXX/drs/dr16xx.cpp b/clang/test/CXX/drs/dr16xx.cpp
index 3f074c4d57354a..2ffeb5372ec1d2 100644
--- a/clang/test/CXX/drs/dr16xx.cpp
+++ b/clang/test/CXX/drs/dr16xx.cpp
@@ -472,7 +472,7 @@ namespace dr1696 { // dr1696: 7
const A &a = A(); // #dr1696-D1-a
};
D1 d1 = {}; // #dr1696-d1
- // since-cxx14-warning@-1 {{sorry, lifetime extension of temporary created
by aggregate initialization using default member initializer is not supported;
lifetime of temporary will end at the end of the full-expression}}
+ // since-cxx14-warning@-1 {{lifetime extension of temporary created by
aggregate initialization using a default member initializer is not supported;
lifetime of temporary will terminate at the end of the full-expression}}
// since-cxx14-note@#dr1696-D1-a {{initializing field 'a' with default
member initializer}}
struct D2 {
diff --git a/clang/test/CXX/drs/dr18xx.cpp b/clang/test/CXX/drs/dr18xx.cpp
index fbe67bd0c2f6db..0179d411358902 100644
--- a/clang/test/CXX/drs/dr18xx.cpp
+++ b/clang
@@ -188,3 +188,8 @@ addi a2, ft0, 24 # CHECK: :[[@LINE]]:10: error: invalid
operand for instruction
# fence.tso accepts no operands
fence.tso rw, rw # CHECK: :[[@LINE]]:11: error: invalid operand for instruction
+
+.Ltlsdesc_hi0:
+jalr x5, 0(a1), %tlsdesc_hi(.Ltlsdesc_hi0) # CHECK: :[[@LINE]]:17: error:
operand must be a symbol with %tlsdesc_call modifier
+jalr x1, 0(a1), %tlsdesc_call(.Ltlsdesc_hi0) # CHECK: :[[@LINE]]:12: error:
the output operand must be t0/x5 when using %tlsdesc_call modifier
topperc wrote:
Looks like we are missing MC layer tests that don't generate an error?
https://github.com/llvm/llvm-project/pull/66915
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
pav-code wrote:
As the bot suggested - I am tagging the issue creator!
@tbaederr
https://github.com/llvm/llvm-project/pull/76310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -target-feature +sse2 < %s
| FileCheck %s --check-prefixes=CHECK
phoebewang wrote:
Yes, e.g, `llvm/test/CodeGen/X86/{soft-fp,x87}.ll`, though I doubt if they can
work in reality since they are calling to the same lib functions.
https://github.com/llvm/llvm-project/pull/75156
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -17132,6 +17132,10 @@ static bool ConvertAPValueToString(const APValue &V,
QualType T,
case BuiltinType::WChar_U: {
unsigned TyWidth = Context.getIntWidth(T);
assert(8 <= TyWidth && TyWidth <= 32 && "Unexpected integer width");
+ if (V.getInt() > std::numeric_limits::max() ||
topperc wrote:
Is the right place to fix the original issue? The code right below this seems
to be printing in hex, but the bug report shows a long decimal value. Also I'm
not sure why _BitInt would got through any of the BuiltinTypes listed in this
switch.
Is the later call to V.getInt().toString(Str); on line 17153 the right place?
https://github.com/llvm/llvm-project/pull/75902
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
brad0 wrote:
It's all green lights. Anything holding this up?
https://github.com/llvm/llvm-project/pull/74377
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/felix642 created
https://github.com/llvm/llvm-project/pull/76315
The check currently emits warnings for the following code:
`uint64_t fn() { return 1024 * 1024; }`
But the code generated after applying the notes will look like this:
`uint64_t fn() { return static_cast(1024 * )1024; }`
```
[:5:12: warning: performing an implicit widening conversion to type
'uint64_t' (aka 'unsigned long') of a multiplication performed in type 'int'
[bugprone-implicit-widening-of-multiplication-result]](javascript:;)
5 | return 1024 * 1024;
|^
[:5:12: note: make conversion explicit to silence this
warning](javascript:;)
1 | #include
2 |
3 | uint64_t fn()
4 | {
5 | return 1024 * 1024;
|^~~
|static_cast( )
[:5:12: note: perform multiplication in a wider type](javascript:;)
5 | return 1024 * 1024;
|^~~~
|static_cast()
1 warning generated.
```
This is because when generating the notes the check will use the beginLoc() and
EndLoc() of the subexpr of the implicit cast.
But in some cases the AST Node might not have a beginLoc and EndLoc. This seems
to be true when the Node is composed of only 1 token (for example an integer
literal). Calling the getEndLoc() on this type of node will simply return the
known location which is, in this case, the beginLoc.
Fixes #63070 #56728
I was not able to add tests for this commit and this is because the check
currently emits two notes for every detection.
The first suggestion is to silence the warning by casting the whole operation.
The second suggestion is there to fix the issue (And I think the suggested code
is invalid, but I've opened another issue for that:
https://github.com/llvm/llvm-project/issues/76311).
Since both suggestions are overlapping clang-tidy refuses to apply them and I
was not able to add new tests.
The first suggestion seems useless since we should not suggest a fix-it to
silence an issue (the message seems sufficient to me).
But this requires more discussion and the change is not trivial so I've left it
how it is for now.
From 7d9c4d423c00033e8c4df26b87176e3054976423 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
Date: Sat, 23 Dec 2023 23:17:03 -0500
Subject: [PATCH] =?UTF-8?q?[clang-tidy]=C2=A0Invalid=20Fix-It=20generated?=
=?UTF-8?q?=20for=20implicit-widening-multiplication-result?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The check currently emits warnings for the following code:
uint64_t fn() { return 1024 * 1024; }
But the code generated after applying the notes will look like this:
uint64_t fn() { return static_cast(1024 * )1024; }
This is because when generating the notes the check will use the beginLoc
and EndLoc of the subexpr of the implicit cast.
But in some cases the AST Node might not have a beginLoc and EndLoc. This
seems to be true when the Node is composed of only 1 token (for example
an integer literal). Calling the getEndLoc() on this type of node will
simply return the known location which is, in this case, the beginLoc.
Fixes #63070 #56728
---
...citWideningOfMultiplicationResultCheck.cpp | 27 +--
clang-tools-extra/docs/ReleaseNotes.rst | 4 +++
...tion-result-array-subscript-expression.cpp | 4 +--
...-widening-of-multiplication-result-int.cpp | 8 +++---
...f-multiplication-result-pointer-offset.cpp | 4 +--
5 files changed, 31 insertions(+), 16 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
index 95a7c521eabb0e..6f22f02f301835 100644
---
a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
+++
b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
@@ -9,6 +9,7 @@
#include "ImplicitWideningOfMultiplicationResultCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
#include
using namespace clang::ast_matchers;
@@ -99,15 +100,16 @@ void
ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr(
"make conversion explicit to silence this warning",
DiagnosticIDs::Note)
<< E->getSourceRange();
-
+const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+E->getEndLoc(), 0, *Result->SourceManager, getLangOpts());
if (ShouldUseCXXStaticCast)
Diag << FixItHint::CreateInsertion(
E->getBeginLoc(), "static_cast<" + Ty.getAsString() + ">(")
- << FixItHint::CreateInsertion(E->getEndLoc(), ")");
+ << FixItHint::CreateInsertion(EndLoc, ")");
else
Diag << FixItHint::CreateInsertion(E->getBeginLoc(),
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
Author: Félix-Antoine Constantin (felix642)
Changes
The check currently emits warnings for the following code:
`uint64_t fn() { return 1024 * 1024; }`
But the code generated after applying the notes will look like this:
`uint64_t fn() { return static_cast(1024 * )1024; }`
```
[:5:12: warning: performing an implicit widening conversion to
type 'uint64_t' (aka 'unsigned long') of a multiplication performed in type
'int' [bugprone-implicit-widening-of-multiplication-result]](javascript:;)
5 | return 1024 * 1024;
|^
[:5:12: note: make conversion explicit to silence this
warning](javascript:;)
1 | #include
2 |
3 | uint64_t fn()
4 | {
5 | return 1024 * 1024;
|^~~
|static_cast( )
[:5:12: note: perform multiplication in a wider
type](javascript:;)
5 | return 1024 * 1024;
|^~~~
|static_cast()
1 warning generated.
```
This is because when generating the notes the check will use the beginLoc() and
EndLoc() of the subexpr of the implicit cast.
But in some cases the AST Node might not have a beginLoc and EndLoc. This seems
to be true when the Node is composed of only 1 token (for example an integer
literal). Calling the getEndLoc() on this type of node will simply return the
known location which is, in this case, the beginLoc.
Fixes #63070 #56728
I was not able to add tests for this commit and this is because the check
currently emits two notes for every detection.
The first suggestion is to silence the warning by casting the whole operation.
The second suggestion is there to fix the issue (And I think the suggested code
is invalid, but I've opened another issue for that:
https://github.com/llvm/llvm-project/issues/76311).
Since both suggestions are overlapping clang-tidy refuses to apply them and I
was not able to add new tests.
The first suggestion seems useless since we should not suggest a fix-it to
silence an issue (the message seems sufficient to me).
But this requires more discussion and the change is not trivial so I've left it
how it is for now.
---
Full diff: https://github.com/llvm/llvm-project/pull/76315.diff
5 Files Affected:
- (modified)
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
(+19-8)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp
(+2-2)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
(+4-4)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-pointer-offset.cpp
(+2-2)
``diff
diff --git
a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
index 95a7c521eabb0e..6f22f02f301835 100644
---
a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
+++
b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
@@ -9,6 +9,7 @@
#include "ImplicitWideningOfMultiplicationResultCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
#include
using namespace clang::ast_matchers;
@@ -99,15 +100,16 @@ void
ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr(
"make conversion explicit to silence this warning",
DiagnosticIDs::Note)
<< E->getSourceRange();
-
+const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+E->getEndLoc(), 0, *Result->SourceManager, getLangOpts());
if (ShouldUseCXXStaticCast)
Diag << FixItHint::CreateInsertion(
E->getBeginLoc(), "static_cast<" + Ty.getAsString() + ">(")
- << FixItHint::CreateInsertion(E->getEndLoc(), ")");
+ << FixItHint::CreateInsertion(EndLoc, ")");
else
Diag << FixItHint::CreateInsertion(E->getBeginLoc(),
"(" + Ty.getAsString() + ")(")
- << FixItHint::CreateInsertion(E->getEndLoc(), ")");
+ << FixItHint::CreateInsertion(EndLoc, ")");
Diag << includeStddefHeader(E->getBeginLoc());
}
@@ -137,7 +139,11 @@ void
ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr(
Diag << FixItHint::CreateInsertion(LHS->getBeginLoc(),
"static_cast<" +
WideExprTy.getAsString() + ">(")
- << FixItHint::CreateInsertion(LHS->getEndLoc(), ")");
+ << FixItHint::CreateInsertion(
felix642 wrote:
A patch that proposed a similar fix was previously submitted for this issue but
never landed : https://reviews.llvm.org/D141058
https://github.com/llvm/llvm-project/pull/76315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
