[PATCH] D51464: clang: fix MIPS/N32 triple and paths
wzssyqa updated this revision to Diff 167632. https://reviews.llvm.org/D51464 Files: lib/Basic/Targets/Mips.h lib/Driver/ToolChains/Arch/Mips.cpp lib/Driver/ToolChains/Gnu.cpp lib/Driver/ToolChains/Linux.cpp test/CodeGen/atomics-inlining.c test/CodeGen/mips-zero-sized-struct.c test/CodeGen/target-data.c test/CodeGen/xray-attributes-supported.cpp test/Driver/clang-translation.c Index: test/Driver/clang-translation.c === --- test/Driver/clang-translation.c +++ test/Driver/clang-translation.c @@ -330,6 +330,38 @@ // MIPS64EL: "-target-cpu" "mips64r2" // MIPS64EL: "-mfloat-abi" "hard" +// RUN: %clang -target mips64-linux-gnuabi64 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64-GNUABI64 %s +// MIPS64-GNUABI64: clang +// MIPS64-GNUABI64: "-cc1" +// MIPS64-GNUABI64: "-target-cpu" "mips64r2" +// MIPS64-GNUABI64: "-target-abi" "n64" +// MIPS64-GNUABI64: "-mfloat-abi" "hard" + +// RUN: %clang -target mips64el-linux-gnuabi64 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64EL-GNUABI64 %s +// MIPS64EL-GNUABI64: clang +// MIPS64EL-GNUABI64: "-cc1" +// MIPS64EL-GNUABI64: "-target-cpu" "mips64r2" +// MIPS64EL-GNUABI64: "-target-abi" "n64" +// MIPS64EL-GNUABI64: "-mfloat-abi" "hard" + +// RUN: %clang -target mips64-linux-gnuabin32 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSN32 %s +// MIPSN32: clang +// MIPSN32: "-cc1" +// MIPSN32: "-target-cpu" "mips64r2" +// MIPSN32: "-target-abi" "n32" +// MIPSN32: "-mfloat-abi" "hard" + +// RUN: %clang -target mips64el-linux-gnuabin32 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSN32EL %s +// MIPSN32EL: clang +// MIPSN32EL: "-cc1" +// MIPSN32EL: "-target-cpu" "mips64r2" +// MIPSN32EL: "-target-abi" "n32" +// MIPSN32EL: "-mfloat-abi" "hard" + // RUN: %clang -target mips64el-linux-android -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPS64EL-ANDROID %s // MIPS64EL-ANDROID: clang Index: test/CodeGen/xray-attributes-supported.cpp === --- test/CodeGen/xray-attributes-supported.cpp +++ test/CodeGen/xray-attributes-supported.cpp @@ -11,6 +11,14 @@ // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ // RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips64-unknown-linux-gnuabi64 | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips64el-unknown-linux-gnuabi64 | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips64-unknown-linux-gnuabin32 | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips64el-unknown-linux-gnuabin32 | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ // RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s // Make sure that the LLVM attribute for XRay-annotated functions do show up. Index: test/CodeGen/target-data.c === --- test/CodeGen/target-data.c +++ test/CodeGen/target-data.c @@ -42,18 +42,34 @@ // RUN: FileCheck %s -check-prefix=MIPS-64EL // MIPS-64EL: target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128" +// RUN: %clang_cc1 -triple mips64el-linux-gnuabi64 -o - -emit-llvm %s | \ +// RUN: FileCheck %s -check-prefix=MIPS-64EL +// MIPS-64EL: target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128" + // RUN: %clang_cc1 -triple mips64el-linux-gnu -o - -emit-llvm -target-abi n32 \ // RUN: %s | FileCheck %s -check-prefix=MIPS-64EL-N32 // MIPS-64EL-N32: target datalayout = "e-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128" +// RUN: %clang_cc1 -triple mips64el-linux-gnuabin32 -o - -emit-llvm \ +// RUN: %s | FileCheck %s -check-prefix=MIPS-64EL-N32 +// MIPS-64EL-N32: target datalayout = "e-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128" + // RUN: %clang_cc1 -triple mips64-linux-gnu -o - -emit-llvm %s | \ // RUN: FileCheck %s -check-prefix=MIPS-64EB // MIPS-64EB: target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128" +// RUN: %clang_cc1 -triple mips64-linux-gnuabi64 -o - -emit-llvm %s | \ +// RUN: FileCheck %s -check-prefix=MIPS-64EB +// MIPS-64EB: target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128" + // RUN: %clang_cc1 -triple mips64-linux-gnu -o - -emit-llvm %s -target-abi n32 \ // RUN: | FileCheck %s -check-prefix=MIPS-64EB-N32 // MIPS-64EB-N32: target datalayout = "E-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128" +// RUN: %clang_cc1 -triple mips64-linux-gnuabin32 -o - -emit-llvm %s \ +// RUN: | FileCheck %s -check-prefix=MIPS-64EB-N32 +// MIPS-64EB-N32: target datalayout = "E-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128" + // RUN: %clang_cc1 -triple powerpc64-lv2
[PATCH] D51464: clang: fix MIPS/N32 triple and paths
wzssyqa updated this revision to Diff 167631.
https://reviews.llvm.org/D51464
Files:
lib/Driver/ToolChains/Arch/Mips.cpp
lib/Driver/ToolChains/Gnu.cpp
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -2060,7 +2060,6 @@
case llvm::Triple::mipsel:
LibDirs.append(begin(MIPSELLibDirs), end(MIPSELLibDirs));
TripleAliases.append(begin(MIPSELTriples), end(MIPSELTriples));
-TripleAliases.append(begin(MIPSTriples), end(MIPSTriples));
BiarchLibDirs.append(begin(MIPS64ELLibDirs), end(MIPS64ELLibDirs));
BiarchTripleAliases.append(begin(MIPS64ELTriples), end(MIPS64ELTriples));
BiarchLibDirs.append(begin(MIPSN32ELLibDirs), end(MIPSN32ELLibDirs));
Index: lib/Driver/ToolChains/Arch/Mips.cpp
===
--- lib/Driver/ToolChains/Arch/Mips.cpp
+++ lib/Driver/ToolChains/Arch/Mips.cpp
@@ -82,6 +82,9 @@
}
}
+ if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
+ABIName = "n32";
+
if (ABIName.empty() &&
(Triple.getVendor() == llvm::Triple::MipsTechnologies ||
Triple.getVendor() == llvm::Triple::ImaginationTechnologies)) {
@@ -104,15 +107,8 @@
.Case("octeon", "n64")
.Case("p5600", "o32")
.Default("");
-if (Triple.getEnvironment() == llvm::Triple::GNUABIN32)
- ABIName = llvm::StringSwitch(ABIName)
-.Case("n64", "n32")
-.Default(ABIName);
}
- if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
-ABIName = "n32";
-
if (ABIName.empty()) {
// Deduce ABI name from the target triple.
ABIName = Triple.isMIPS32() ? "o32" : "n64";
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -2060,7 +2060,6 @@
case llvm::Triple::mipsel:
LibDirs.append(begin(MIPSELLibDirs), end(MIPSELLibDirs));
TripleAliases.append(begin(MIPSELTriples), end(MIPSELTriples));
-TripleAliases.append(begin(MIPSTriples), end(MIPSTriples));
BiarchLibDirs.append(begin(MIPS64ELLibDirs), end(MIPS64ELLibDirs));
BiarchTripleAliases.append(begin(MIPS64ELTriples), end(MIPS64ELTriples));
BiarchLibDirs.append(begin(MIPSN32ELLibDirs), end(MIPSN32ELLibDirs));
Index: lib/Driver/ToolChains/Arch/Mips.cpp
===
--- lib/Driver/ToolChains/Arch/Mips.cpp
+++ lib/Driver/ToolChains/Arch/Mips.cpp
@@ -82,6 +82,9 @@
}
}
+ if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
+ABIName = "n32";
+
if (ABIName.empty() &&
(Triple.getVendor() == llvm::Triple::MipsTechnologies ||
Triple.getVendor() == llvm::Triple::ImaginationTechnologies)) {
@@ -104,15 +107,8 @@
.Case("octeon", "n64")
.Case("p5600", "o32")
.Default("");
-if (Triple.getEnvironment() == llvm::Triple::GNUABIN32)
- ABIName = llvm::StringSwitch(ABIName)
-.Case("n64", "n32")
-.Default(ABIName);
}
- if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
-ABIName = "n32";
-
if (ABIName.empty()) {
// Deduce ABI name from the target triple.
ABIName = Triple.isMIPS32() ? "o32" : "n64";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50850: clang: Add triples support for MIPS r6
wzssyqa updated this revision to Diff 167630. https://reviews.llvm.org/D50850 Files: lib/Driver/ToolChains/Arch/Mips.cpp lib/Driver/ToolChains/Gnu.cpp lib/Driver/ToolChains/Linux.cpp test/CodeGen/atomics-inlining.c test/CodeGen/mips-zero-sized-struct.c test/CodeGen/target-data.c test/CodeGen/xray-attributes-supported.cpp test/Driver/clang-translation.c Index: test/Driver/clang-translation.c === --- test/Driver/clang-translation.c +++ test/Driver/clang-translation.c @@ -291,13 +291,27 @@ // MIPS: "-target-cpu" "mips32r2" // MIPS: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa32r6-linux-gnu -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSR6 %s +// MIPSR6: clang +// MIPSR6: "-cc1" +// MIPSR6: "-target-cpu" "mips32r6" +// MIPSR6: "-mfloat-abi" "hard" + // RUN: %clang -target mipsel-linux-gnu -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPSEL %s // MIPSEL: clang // MIPSEL: "-cc1" // MIPSEL: "-target-cpu" "mips32r2" // MIPSEL: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa32r6el-linux-gnu -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSR6EL %s +// MIPSR6EL: clang +// MIPSR6EL: "-cc1" +// MIPSR6EL: "-target-cpu" "mips32r6" +// MIPSR6EL: "-mfloat-abi" "hard" + // RUN: %clang -target mipsel-linux-android -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPSEL-ANDROID %s // MIPSEL-ANDROID: clang @@ -323,45 +337,91 @@ // MIPS64: "-target-cpu" "mips64r2" // MIPS64: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6-linux-gnu -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64R6 %s +// MIPS64R6: clang +// MIPS64R6: "-cc1" +// MIPS64R6: "-target-cpu" "mips64r6" +// MIPS64R6: "-mfloat-abi" "hard" + // RUN: %clang -target mips64el-linux-gnu -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPS64EL %s // MIPS64EL: clang // MIPS64EL: "-cc1" // MIPS64EL: "-target-cpu" "mips64r2" // MIPS64EL: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6el-linux-gnu -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64R6EL %s +// MIPS64R6EL: clang +// MIPS64R6EL: "-cc1" +// MIPS64R6EL: "-target-cpu" "mips64r6" +// MIPS64R6EL: "-mfloat-abi" "hard" + // RUN: %clang -target mips64-linux-gnuabi64 -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPS64-GNUABI64 %s // MIPS64-GNUABI64: clang // MIPS64-GNUABI64: "-cc1" // MIPS64-GNUABI64: "-target-cpu" "mips64r2" // MIPS64-GNUABI64: "-target-abi" "n64" // MIPS64-GNUABI64: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6-linux-gnuabi64 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64R6-GNUABI64 %s +// MIPS64R6-GNUABI64: clang +// MIPS64R6-GNUABI64: "-cc1" +// MIPS64R6-GNUABI64: "-target-cpu" "mips64r6" +// MIPS64R6-GNUABI64: "-target-abi" "n64" +// MIPS64R6-GNUABI64: "-mfloat-abi" "hard" + // RUN: %clang -target mips64el-linux-gnuabi64 -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPS64EL-GNUABI64 %s // MIPS64EL-GNUABI64: clang // MIPS64EL-GNUABI64: "-cc1" // MIPS64EL-GNUABI64: "-target-cpu" "mips64r2" // MIPS64EL-GNUABI64: "-target-abi" "n64" // MIPS64EL-GNUABI64: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6el-linux-gnuabi64 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64R6EL-GNUABI64 %s +// MIPS64R6EL-GNUABI64: clang +// MIPS64R6EL-GNUABI64: "-cc1" +// MIPS64R6EL-GNUABI64: "-target-cpu" "mips64r6" +// MIPS64R6EL-GNUABI64: "-target-abi" "n64" +// MIPS64R6EL-GNUABI64: "-mfloat-abi" "hard" + // RUN: %clang -target mips64-linux-gnuabin32 -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPSN32 %s // MIPSN32: clang // MIPSN32: "-cc1" // MIPSN32: "-target-cpu" "mips64r2" // MIPSN32: "-target-abi" "n32" // MIPSN32: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6-linux-gnuabin32 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSN32R6 %s +// MIPSN32R6: clang +// MIPSN32R6: "-cc1" +// MIPSN32R6: "-target-cpu" "mips64r6" +// MIPSN32R6: "-target-abi" "n32" +// MIPSN32R6: "-mfloat-abi" "hard" + // RUN: %clang -target mips64el-linux-gnuabin32 -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPSN32EL %s // MIPSN32EL: clang // MIPSN32EL: "-cc1" // MIPSN32EL: "-target-cpu" "mips64r2" // MIPSN32EL: "-target-abi" "n32" // MIPSN32EL: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6el-linux-gnuabin32 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSN32R6EL %s +// MIPSN32R6EL: clang +// MIPSN32R6EL: "-cc1" +// MIPSN32R6EL: "-target-cpu" "mips64r6" +// MIPSN32R6EL: "-target-abi" "n32" +// MIPSN32R6EL: "-mfloat-abi" "hard" + // RUN: %clang -target mips64el-linux-android -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPS64EL-ANDROID %s // MIPS64EL-ANDROID: clang Index: test/CodeGen/xray-attributes-supported.cpp === --- test/CodeGen/xray-attributes-supported.cpp +++ test/CodeGen/xray-attributes-supported.cpp @@ -5,20 +5,36 @@ // RUN: %clang_cc1
[PATCH] D50850: clang: Add triples support for MIPS r6
wzssyqa added a comment. This is really for Clang. I guess you mean that compiler-rt directory also need to be patched. Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:115 + if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32)) +ABIName = "n32"; + atanasyan wrote: > It looks like this change is unrelated to introducing new target triples and > can be made by a separate commit. These lines code about N32, is quite close tied with r6 staffs, as they shared lots. Is it ok to update the descriptions? Comment at: lib/Driver/ToolChains/Gnu.cpp:2093 BiarchTripleAliases.append(begin(MIPSELTriples), end(MIPSELTriples)); -BiarchTripleAliases.append(begin(MIPSTriples), end(MIPSTriples)); +BiarchLibDirs.append(begin(MIPSN32ELLibDirs), end(MIPSN32ELLibDirs)); +BiarchTripleAliases.append(begin(MIPSN32ELTriples), end(MIPSN32ELTriples)); atanasyan wrote: > Ditto As a question: why MIPSTriples here? the above mips64 lines don't include any EL Triples. Comment at: lib/Driver/ToolChains/Gnu.cpp:2437 if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 || -getTriple().isAndroid() || -getTriple().isOSFreeBSD() || +getTriple().getEnvironment() == llvm::Triple::GNUABIN32 || +getTriple().isAndroid() || getTriple().isOSFreeBSD() || atanasyan wrote: > Are you sure that integrated LLVM assembler is ready to support N32 code > generation? Anyway this change is for a separate patch. I didn't have so many test, while helloworld programs really works. Comment at: lib/Driver/ToolChains/Linux.cpp:126 return "mips64-linux-gnu"; -if (D.getVFS().exists(SysRoot + "/lib/mips64-linux-gnuabi64")) - return "mips64-linux-gnuabi64"; +MipsCpu = (TargetSubArch == llvm::Triple::MipsSubArch_r6) ? "mipsisa64" + : "mips64"; atanasyan wrote: > Suppose there are both "/lib/mips64-linux-gnu" and > "/lib/mipsisa64-linux-gnuabi64" paths on a disk and provided target triple is > mipsisa64-linux-gnuabi64. Is it good that we return "mips64-linux-gnu" from > this function anyway? No, return `mips64-linux-gnu' is not good, I keep it just for to making sure I don't change the behavior of clang with my patch. In fact, mips64-linux-gnu in gcc is N32, and Debian never use this triple, we use mips*64*-linux-gnuabin32 So, on Debian, mips64-linux-gnu should never appear. https://reviews.llvm.org/D50850 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! If you need this merged on your behalf, I can do so tomorrow or Monday, unless someone else gets to it first. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52696: Update ifunc attribute support documentation
emaste created this revision. emaste added a reviewer: DmitryPolukhin. Herald added a subscriber: krytarowski. We documented GNU binutils and glibc versions required for ifunc support, but our own lld linker and FreeBSD's rtld also support ifuncs. https://reviews.llvm.org/D52696 Files: include/clang/Basic/AttrDocs.td Index: include/clang/Basic/AttrDocs.td === --- include/clang/Basic/AttrDocs.td +++ include/clang/Basic/AttrDocs.td @@ -3370,7 +3370,7 @@ The ``ifunc`` attribute may only be used on a function declaration. A function declaration with an ``ifunc`` attribute is considered to be a definition of the declared entity. The entity must not have weak linkage; for example, in C++, it cannot be applied to a declaration if a definition at that location would be considered inline. -Not all targets support this attribute. ELF targets support this attribute when using binutils v2.20.1 or higher and glibc v2.11.1 or higher. Non-ELF targets currently do not support this attribute. +Not all targets support this attribute. ELF target support depends on both the linker and runtime linker, and is available in at least lld 4.0 and later, binutils 2.20.1 and later, glibc v2.11.1 and later, and FreeBSD 9.1 and later. Non-ELF targets currently do not support this attribute. }]; } Index: include/clang/Basic/AttrDocs.td === --- include/clang/Basic/AttrDocs.td +++ include/clang/Basic/AttrDocs.td @@ -3370,7 +3370,7 @@ The ``ifunc`` attribute may only be used on a function declaration. A function declaration with an ``ifunc`` attribute is considered to be a definition of the declared entity. The entity must not have weak linkage; for example, in C++, it cannot be applied to a declaration if a definition at that location would be considered inline. -Not all targets support this attribute. ELF targets support this attribute when using binutils v2.20.1 or higher and glibc v2.11.1 or higher. Non-ELF targets currently do not support this attribute. +Not all targets support this attribute. ELF target support depends on both the linker and runtime linker, and is available in at least lld 4.0 and later, binutils 2.20.1 and later, glibc v2.11.1 and later, and FreeBSD 9.1 and later. Non-ELF targets currently do not support this attribute. }]; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri marked 8 inline comments as done. lebedev.ri added inline comments. Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:165 + + diag(LiteralLocation, "%0 literal suffix '%1' is not upper-case") + << LiteralType::Name << S.OldSuffix JonasToth wrote: > lebedev.ri wrote: > > JonasToth wrote: > > > JonasToth wrote: > > > > Lets move this `diag` into the true branch of the if stmt and drop the > > > > ´else`. > > > I think the warning should point at the suffix, which can be retrieved > > > from the replacement range. Then you don't need to include the suffix > > > itself in the diagnostic > > If the warnings are aggregated (i.e. not raw `make` dump), with only the > > first line shown, the suffix will still be invisible. > > > > Regarding the pointer direction, i'm not sure. > > For now i have reworded the diag to justify pointing at the literal itself. > I don't understand that. The warning message does include the source location > that would be clearly on the literal suffix and the warning without the > suffix printed is clear as well. Having this slightly simpler diagnostic > would simplify the code significantly. *location*. which will still be a location, if only the first line of the warning is displayed. We won't even win all that much. Sure, we will return `Optional`, but we will still need to do all that internal stuff to find the old suffix, uppercase it, compare them, etc. And we lose a very useful ability to check what it considered to be the old suffix in the tests. It is basically the same question as in D51949. If you insist, sure, i can do that, but i *really* do believe this is **WRONG**. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
JonasToth added a comment.
> All those are UserDefinedLiteral, so we should be good..
> https://godbolt.org/z/PcGi0B
> Also, it seems the suffix can't be set for these constants:
> https://godbolt.org/z/YHTqke
> So i'm not sure what to test. Can you give an example of a test?
I am not suggesting that these should be all uppercase or anything. But the
tests should demonstrate, that user-defined literals do not impact analysis and
give no false positives.
Comment at: clang-tidy/hicpp/HICPPTidyModule.cpp:24
#include "../misc/StaticAssertCheck.h"
-#include "../bugprone/UndelegatedConstructorCheck.h"
#include "../modernize/DeprecatedHeadersCheck.h"
spurious formatting fix, can be committed separate.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:42
+ // What should be skipped before looking for the Suffixes?
+ // Hexadecimal floating-point literals: skip until exponent first.
+ static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("pP");
lebedev.ri wrote:
> JonasToth wrote:
> > The second line of the comment is slightly confusing, please make a full
> > sentence out of it.
> Rewrote, hopefully better?
Yes, few nits.
- `15 in decimal and is` (no comma)
- `skip to` instead of `skip until`
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:52
+AST_MATCHER(clang::IntegerLiteral, intHasSuffix) {
+ const auto *T = dyn_cast(Node.getType().getTypePtr());
+ if (!T)
lebedev.ri wrote:
> JonasToth wrote:
> > as it hit me in my check: what about `(1)ul`? Is this syntactically correct
> > and should be diagnosed too? (Please add tests if so).
> >
> > In this case it should be `Note.getType().IgnoreParens().getTypePtr())`.
> clang says invalid
> https://godbolt.org/z/R8bGe_
fine then.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:53
+ const auto *T = dyn_cast(Node.getType().getTypePtr());
+ if (!T)
+return false;
lebedev.ri wrote:
> JonasToth wrote:
> > Maybe the if could init `T`? It would require a second `return false;` if i
> > am not mistaken, but looks more regular to me. No strong opinion from my
> > side.
> Then we will not have an early-return, which is worse than this.
Ok. Can the `dyn_cast` be null at all? Shouldn't integerliteral always be a
`BuiltinType`?
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:85
+template
+llvm::Optional
+UppercaseLiteralSuffixCheck::shouldReplaceLiteralSuffix(
lebedev.ri wrote:
> JonasToth wrote:
> > These types get really long. Is it possible to put `NewSuffix` into the
> > anonymous namespace as well?
> No, because `shouldReplaceLiteralSuffix()` is a member function which returns
> that type.
> I think it should stay a member function, so theoretically `NewSuffix` could
> be a [second] template param, but that is kinda ugly..
> I also can't simplify it via `using` because the `NewSuffix` is private.
>
> Perhaps we should keep this as-is?
Why does it need to be a member? It looks like it only accesses `NewSuffix`
which does nothing that requires private access to the check class.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:152
+ // Ignore literals that aren't fully written in the source code.
+ if (LiteralLocation.isMacroID() ||
+ Result.SourceManager->isMacroBodyExpansion(LiteralLocation) ||
lebedev.ri wrote:
> JonasToth wrote:
> > Wouldn't it make sense to warn for the literals in macros?
> Very obscure example (like all macros is), but i think it shows the point:
> ```
> #include
> #include
>
> #define xstr(s) str(s)
> #define str(s) #s
>
> #define dump(X) printf("%u is " str(X) "nits", X);
>
> int main () {
> dump(1u);
>
> return 0;
> }
> ```
> will normally print `1 is 1units`
> But if you uppercase it, it will print `1 is 1Units`.
>
> I would honestly prefer to give macros a pass here..
Transformation is excluded of course.
The user can still silence the warning in case it is misplaced.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:165
+
+ diag(LiteralLocation, "%0 literal suffix '%1' is not upper-case")
+ << LiteralType::Name << S.OldSuffix
lebedev.ri wrote:
> JonasToth wrote:
> > JonasToth wrote:
> > > Lets move this `diag` into the true branch of the if stmt and drop the
> > > ´else`.
> > I think the warning should point at the suffix, which can be retrieved from
> > the replacement range. Then you don't need to include the suffix itself in
> > the diagnostic
> If the warnings are aggregated (i.e. not raw `make` dump), with only the
> first line shown, the suffix will still be invisible.
>
> Regarding the pointer direction, i'm not sure.
> For now i have reworded the diag to justify pointing at the literal itself.
I don't understand
[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks
lebedev.ri updated this revision to Diff 167621.
lebedev.ri added a comment.
As requested in https://reviews.llvm.org/D50902#1250110, actually document that
`ICCK_IntegerTruncation` was only emitted by clang 7.
Repository:
rC Clang
https://reviews.llvm.org/D50901
Files:
docs/UndefinedBehaviorSanitizer.rst
include/clang/Basic/Sanitizers.def
lib/CodeGen/CGExprScalar.cpp
test/CodeGen/catch-implicit-integer-conversions-basics.c
test/CodeGen/catch-implicit-integer-truncations-basics-negatives.c
test/CodeGen/catch-implicit-integer-truncations-basics.c
test/CodeGen/catch-implicit-integer-truncations.c
test/CodeGen/catch-implicit-signed-integer-truncations-basics-negatives.c
test/CodeGen/catch-implicit-signed-integer-truncations-basics.c
test/CodeGen/catch-implicit-unsigned-integer-truncations-basics-negatives.c
test/CodeGen/catch-implicit-unsigned-integer-truncations-basics.c
test/CodeGenCXX/catch-implicit-integer-truncations.cpp
test/Driver/fsanitize.c
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -29,22 +29,37 @@
// CHECK-COVERAGE-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
// RUN: %clang -target x86_64-linux-gnu -fsanitize=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTEGER -implicit-check-not="-fsanitize-address-use-after-scope"
-// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-integer-truncation),?){6}"}}
+// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){7}"}}
// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fno-sanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-NORECOVER
// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-trap=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-TRAP
-// CHECK-implicit-conversion: "-fsanitize={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-NORECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}} // ???
-// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-TRAP: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-TRAP-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-TRAP-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion: "-fsanitize={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){2}"}}
+// CHECK-implicit-conversion-RECOVER: "-fsanitize-recover={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){2}"}}
+// CHECK-implicit-conversion-RECOVER-NOT: "-fno-sanitize-recover={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){2}"}}
+// CHECK-implicit-conversion-RECOVER-NOT: "-fsanitize-trap={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){2}"}}
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fno-sanitize-recover={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){2}"}} // ???
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-recover={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){2}"}}
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-trap={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){2}"}}
+// CHECK-implicit-conversion-TRAP: "-fsanitize-trap={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){2}"}}
+// CHECK-implicit-conversion-TRAP-NOT: "-fsanitize-recover={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){2}"}}
+//
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri updated this revision to Diff 167620.
lebedev.ri marked 13 inline comments as done.
lebedev.ri added a comment.
Thank you for taking a look!
- Rebased
- Added an alias in `hicpp` module
- Addressed //most// of the review comments.
In https://reviews.llvm.org/D52670#1249898, @JonasToth wrote:
> Thanks for working on this!
> Related as well:
> http://www.codingstandard.com/rule/4-2-1-ensure-that-the-u-suffix-is-applied-to-a-literal-used-in-a-context-requiring-an-unsigned-integral-expression/
> I think its wort a alias is hicpp as well
Hmm, after digging but, i think you meant to link to
https://www.codingstandard.com/rule/4-3-1-do-not-convert-an-expression-of-wider-floating-point-type-to-a-narrower-floating-point-type/
With some leeway, i think it does talk about the suffix being uppercase.
Added.
> Please add tests that use user-defined literals and ensure there are no
> collision and that they are not diagnosed. Some examples:
> https://en.cppreference.com/w/cpp/language/user_literal
All those are `UserDefinedLiteral`, so we should be good..
https://godbolt.org/z/PcGi0B
Also, it seems the suffix can't be set for these constants:
https://godbolt.org/z/YHTqke
So i'm not sure what to test. Can you give an example of a test?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52670
Files:
clang-tidy/cert/CERTTidyModule.cpp
clang-tidy/cert/CMakeLists.txt
clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MagicNumbersCheck.cpp
clang-tidy/readability/MagicNumbersCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
clang-tidy/readability/UppercaseLiteralSuffixCheck.h
clang-tidy/utils/ASTUtils.cpp
clang-tidy/utils/ASTUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp
test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
test/clang-tidy/readability-uppercase-literal-suffix.h
Index: test/clang-tidy/readability-uppercase-literal-suffix.h
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix.h
@@ -0,0 +1,16 @@
+template
+struct integral_constant {
+ static constexpr T value = v;
+ typedef T value_type;
+ typedef integral_constant type; // using injected-class-name
+ constexpr operator value_type() const noexcept { return value; }
+};
+
+using false_type = integral_constant;
+using true_type = integral_constant;
+
+template
+struct is_same : false_type {};
+
+template
+struct is_same : true_type {};
Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -0,0 +1,203 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S
+
+#include "readability-uppercase-literal-suffix.h"
+
+void integer_suffix() {
+ static constexpr auto v0 = __LINE__; // synthetic
+ static_assert(v0 == 9 || v0 == 5, "");
+
+ static constexpr auto v1 = __cplusplus; // synthetic, long
+
+ static constexpr auto v2 = 1; // no literal
+ static_assert(is_same::value, "");
+ static_assert(v2 == 1, "");
+
+ // Unsigned
+
+ static constexpr auto v3 = 1u;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'u', which is not upper-case
+ // CHECK-MESSAGES-NEXT: static constexpr auto v3 = 1u;
+ // CHECK-MESSAGES-NEXT: ^~
+ // CHECK-MESSAGES-NEXT: {{^ *}}U{{$}}
+ // CHECK-FIXES: static constexpr auto v3 = 1U;
+ static_assert(is_same::value, "");
+ static_assert(v3 == 1, "");
+
+ static constexpr auto v4 = 1U; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v4 == 1, "");
+
+ // Long
+
+ static constexpr auto v5 = 1l;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'l', which is not upper-case
+ // CHECK-MESSAGES-NEXT: static constexpr auto v5 = 1l;
+ // CHECK-MESSAGES-NEXT: ^~
+ // CHECK-MESSAGES-NEXT: {{^ *}}L{{$}}
+ // CHECK-FIXES: static constexpr auto v5 = 1L;
+ static_assert(is_same::value, "");
+ static_assert(v5 == 1, "");
+
+ static constexpr auto v6 = 1L; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v6 == 1, "");
+
+ // Long Long
+
+ static constexpr auto v7 = 1ll;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'll', which is not
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri added inline comments.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:33
+};
+constexpr llvm::StringLiteral IntegerLiteral::Name;
+constexpr llvm::StringLiteral IntegerLiteral::SkipFirst;
JonasToth wrote:
> why are these declarations necessary?
Well, because these are *declarations*.
Else the linker complains, since i'm using these in non-`constexpr` context,
but only provided definitions.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:42
+ // What should be skipped before looking for the Suffixes?
+ // Hexadecimal floating-point literals: skip until exponent first.
+ static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("pP");
JonasToth wrote:
> The second line of the comment is slightly confusing, please make a full
> sentence out of it.
Rewrote, hopefully better?
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:52
+AST_MATCHER(clang::IntegerLiteral, intHasSuffix) {
+ const auto *T = dyn_cast(Node.getType().getTypePtr());
+ if (!T)
JonasToth wrote:
> as it hit me in my check: what about `(1)ul`? Is this syntactically correct
> and should be diagnosed too? (Please add tests if so).
>
> In this case it should be `Note.getType().IgnoreParens().getTypePtr())`.
clang says invalid
https://godbolt.org/z/R8bGe_
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:53
+ const auto *T = dyn_cast(Node.getType().getTypePtr());
+ if (!T)
+return false;
JonasToth wrote:
> Maybe the if could init `T`? It would require a second `return false;` if i
> am not mistaken, but looks more regular to me. No strong opinion from my side.
Then we will not have an early-return, which is worse than this.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:85
+template
+llvm::Optional
+UppercaseLiteralSuffixCheck::shouldReplaceLiteralSuffix(
JonasToth wrote:
> These types get really long. Is it possible to put `NewSuffix` into the
> anonymous namespace as well?
No, because `shouldReplaceLiteralSuffix()` is a member function which returns
that type.
I think it should stay a member function, so theoretically `NewSuffix` could be
a [second] template param, but that is kinda ugly..
I also can't simplify it via `using` because the `NewSuffix` is private.
Perhaps we should keep this as-is?
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:95
+ // Get the whole Integer Literal from the source buffer.
+ const StringRef LiteralSourceText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(S.Range), SM, getLangOpts());
JonasToth wrote:
> Please check if the source text could be retrieved, with a final `bool`
> parameter, thats in/out and at least `assert` on that.
I looked at a randomly-selected few dozen calls to this function within
clang-tidy, and none of those do this.
But `assert` added, better than nothing.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:129
+ if (S.OldSuffix == S.NewSuffix)
+return llvm::None; // The suffix was already fully uppercase.
+
JonasToth wrote:
> Could this function return a `Optional`? That would include the
> range and the relacement-text. I feel that is would simplify the code,
> especially the amount of state that one has to keep track of.
I still want to see the old suffix, but i think we can form the FixitHint here,
and store it.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:135
+void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ stmt(integerLiteral(intHasSuffix())).bind(IntegerLiteral::Name), this);
JonasToth wrote:
> I think you can merge this matcher to
> `stmt(eachOf(integerLiteral(intHasSuffix()).bind(),
> floatLiteral(fpHasSuffix()).bind()))`
>
> `eachOf` because we want to match all, and not short-circuit.
`bind()` still wants the name though.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:152
+ // Ignore literals that aren't fully written in the source code.
+ if (LiteralLocation.isMacroID() ||
+ Result.SourceManager->isMacroBodyExpansion(LiteralLocation) ||
JonasToth wrote:
> Wouldn't it make sense to warn for the literals in macros?
Very obscure example (like all macros is), but i think it shows the point:
```
#include
#include
#define xstr(s) str(s)
#define str(s) #s
#define dump(X) printf("%u is " str(X) "nits", X);
int main () {
dump(1u);
return 0;
}
```
will normally print `1 is 1units`
But if you uppercase it, it will print `1 is 1Units`.
I would honestly prefer to give macros a pass here..
Comment at:
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
steveire marked an inline comment as done. steveire added a comment. > Once I'm happy, I will accept it. If @alexfh has further comments, then they > can be addressed at that time (pre or post commit). That's very reasonable, thanks. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
steveire updated this revision to Diff 167619.
steveire added a comment.
Doc fix
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
Files:
CMakeLists.txt
clang-tidy/CMakeLists.txt
clang-tidy/ClangTidy.cpp
clang-tidy/plugin/CMakeLists.txt
clang-tidy/plugin/ClangTidyPlugin.cpp
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
docs/clang-tidy/index.rst
test/CMakeLists.txt
test/clang-tidy/enable-alpha-checks.cpp
test/clang-tidy/mpi-buffer-deref.cpp
test/clang-tidy/mpi-type-mismatch.cpp
test/clang-tidy/nolint.cpp
test/clang-tidy/read_file_config.cpp
test/clang-tidy/static-analyzer-config.cpp
test/clang-tidy/static-analyzer.cpp
test/clang-tidy/temporaries.cpp
test/lit.cfg
unittests/CMakeLists.txt
Index: unittests/CMakeLists.txt
===
--- unittests/CMakeLists.txt
+++ unittests/CMakeLists.txt
@@ -18,8 +18,6 @@
add_subdirectory(clang-apply-replacements)
add_subdirectory(clang-move)
add_subdirectory(clang-query)
-if(CLANG_ENABLE_STATIC_ANALYZER)
- add_subdirectory(clang-tidy)
-endif()
+add_subdirectory(clang-tidy)
add_subdirectory(clangd)
add_subdirectory(include-fixer)
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -119,24 +119,22 @@
if config.clang_staticanalyzer:
config.available_features.add('static-analyzer')
-check_clang_tidy = os.path.join(
-config.test_source_root, "clang-tidy", "check_clang_tidy.py")
-config.substitutions.append(
-('%check_clang_tidy',
- '%s %s' % (config.python_executable, check_clang_tidy)) )
-clang_tidy_diff = os.path.join(
-config.test_source_root, "..", "clang-tidy", "tool", "clang-tidy-diff.py")
-config.substitutions.append(
-('%clang_tidy_diff',
- '%s %s' % (config.python_executable, clang_tidy_diff)) )
-run_clang_tidy = os.path.join(
-config.test_source_root, "..", "clang-tidy", "tool", "run-clang-tidy.py")
-config.substitutions.append(
-('%run_clang_tidy',
- '%s %s' % (config.python_executable, run_clang_tidy)) )
-else:
-# exclude the clang-tidy test directory
-config.excludes.append('clang-tidy')
+
+check_clang_tidy = os.path.join(
+config.test_source_root, "clang-tidy", "check_clang_tidy.py")
+config.substitutions.append(
+('%check_clang_tidy',
+ '%s %s' % (config.python_executable, check_clang_tidy)) )
+clang_tidy_diff = os.path.join(
+config.test_source_root, "..", "clang-tidy", "tool", "clang-tidy-diff.py")
+config.substitutions.append(
+('%clang_tidy_diff',
+ '%s %s' % (config.python_executable, clang_tidy_diff)) )
+run_clang_tidy = os.path.join(
+config.test_source_root, "..", "clang-tidy", "tool", "run-clang-tidy.py")
+config.substitutions.append(
+('%run_clang_tidy',
+ '%s %s' % (config.python_executable, run_clang_tidy)) )
clangd_benchmarks_dir = os.path.join(os.path.dirname(config.clang_tools_dir),
"tools", "clang", "tools", "extra",
Index: test/clang-tidy/temporaries.cpp
===
--- test/clang-tidy/temporaries.cpp
+++ test/clang-tidy/temporaries.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy -checks='-*,clang-analyzer-core.NullDereference' %s -- | FileCheck %s
struct NoReturnDtor {
Index: test/clang-tidy/static-analyzer.cpp
===
--- test/clang-tidy/static-analyzer.cpp
+++ test/clang-tidy/static-analyzer.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy %s -checks='-*,clang-analyzer-*' -- | FileCheck %s
extern void *malloc(unsigned long);
extern void free(void *);
Index: test/clang-tidy/static-analyzer-config.cpp
===
--- test/clang-tidy/static-analyzer-config.cpp
+++ test/clang-tidy/static-analyzer-config.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy %s -checks='-*,clang-analyzer-unix.Malloc' -config='{CheckOptions: [{ key: "clang-analyzer-unix.Malloc:Optimistic", value: true}]}' -- | FileCheck %s
typedef __typeof(sizeof(int)) size_t;
void *malloc(size_t);
Index: test/clang-tidy/read_file_config.cpp
===
--- test/clang-tidy/read_file_config.cpp
+++ test/clang-tidy/read_file_config.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: mkdir -p %T/read-file-config/
// RUN: cp %s %T/read-file-config/test.cpp
// RUN: echo 'Checks: "-*,modernize-use-nullptr"' > %T/read-file-config/.clang-tidy
Index: test/clang-tidy/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: %check_clang_tidy %s
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
steveire updated this revision to Diff 167618.
steveire added a comment.
Format docs
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
Files:
CMakeLists.txt
clang-tidy/CMakeLists.txt
clang-tidy/ClangTidy.cpp
clang-tidy/plugin/CMakeLists.txt
clang-tidy/plugin/ClangTidyPlugin.cpp
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
docs/clang-tidy/index.rst
test/CMakeLists.txt
test/clang-tidy/enable-alpha-checks.cpp
test/clang-tidy/mpi-buffer-deref.cpp
test/clang-tidy/mpi-type-mismatch.cpp
test/clang-tidy/nolint.cpp
test/clang-tidy/read_file_config.cpp
test/clang-tidy/static-analyzer-config.cpp
test/clang-tidy/static-analyzer.cpp
test/clang-tidy/temporaries.cpp
test/lit.cfg
unittests/CMakeLists.txt
Index: unittests/CMakeLists.txt
===
--- unittests/CMakeLists.txt
+++ unittests/CMakeLists.txt
@@ -18,8 +18,6 @@
add_subdirectory(clang-apply-replacements)
add_subdirectory(clang-move)
add_subdirectory(clang-query)
-if(CLANG_ENABLE_STATIC_ANALYZER)
- add_subdirectory(clang-tidy)
-endif()
+add_subdirectory(clang-tidy)
add_subdirectory(clangd)
add_subdirectory(include-fixer)
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -119,24 +119,22 @@
if config.clang_staticanalyzer:
config.available_features.add('static-analyzer')
-check_clang_tidy = os.path.join(
-config.test_source_root, "clang-tidy", "check_clang_tidy.py")
-config.substitutions.append(
-('%check_clang_tidy',
- '%s %s' % (config.python_executable, check_clang_tidy)) )
-clang_tidy_diff = os.path.join(
-config.test_source_root, "..", "clang-tidy", "tool", "clang-tidy-diff.py")
-config.substitutions.append(
-('%clang_tidy_diff',
- '%s %s' % (config.python_executable, clang_tidy_diff)) )
-run_clang_tidy = os.path.join(
-config.test_source_root, "..", "clang-tidy", "tool", "run-clang-tidy.py")
-config.substitutions.append(
-('%run_clang_tidy',
- '%s %s' % (config.python_executable, run_clang_tidy)) )
-else:
-# exclude the clang-tidy test directory
-config.excludes.append('clang-tidy')
+
+check_clang_tidy = os.path.join(
+config.test_source_root, "clang-tidy", "check_clang_tidy.py")
+config.substitutions.append(
+('%check_clang_tidy',
+ '%s %s' % (config.python_executable, check_clang_tidy)) )
+clang_tidy_diff = os.path.join(
+config.test_source_root, "..", "clang-tidy", "tool", "clang-tidy-diff.py")
+config.substitutions.append(
+('%clang_tidy_diff',
+ '%s %s' % (config.python_executable, clang_tidy_diff)) )
+run_clang_tidy = os.path.join(
+config.test_source_root, "..", "clang-tidy", "tool", "run-clang-tidy.py")
+config.substitutions.append(
+('%run_clang_tidy',
+ '%s %s' % (config.python_executable, run_clang_tidy)) )
clangd_benchmarks_dir = os.path.join(os.path.dirname(config.clang_tools_dir),
"tools", "clang", "tools", "extra",
Index: test/clang-tidy/temporaries.cpp
===
--- test/clang-tidy/temporaries.cpp
+++ test/clang-tidy/temporaries.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy -checks='-*,clang-analyzer-core.NullDereference' %s -- | FileCheck %s
struct NoReturnDtor {
Index: test/clang-tidy/static-analyzer.cpp
===
--- test/clang-tidy/static-analyzer.cpp
+++ test/clang-tidy/static-analyzer.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy %s -checks='-*,clang-analyzer-*' -- | FileCheck %s
extern void *malloc(unsigned long);
extern void free(void *);
Index: test/clang-tidy/static-analyzer-config.cpp
===
--- test/clang-tidy/static-analyzer-config.cpp
+++ test/clang-tidy/static-analyzer-config.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy %s -checks='-*,clang-analyzer-unix.Malloc' -config='{CheckOptions: [{ key: "clang-analyzer-unix.Malloc:Optimistic", value: true}]}' -- | FileCheck %s
typedef __typeof(sizeof(int)) size_t;
void *malloc(size_t);
Index: test/clang-tidy/read_file_config.cpp
===
--- test/clang-tidy/read_file_config.cpp
+++ test/clang-tidy/read_file_config.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: mkdir -p %T/read-file-config/
// RUN: cp %s %T/read-file-config/test.cpp
// RUN: echo 'Checks: "-*,modernize-use-nullptr"' > %T/read-file-config/.clang-tidy
Index: test/clang-tidy/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: %check_clang_tidy %s
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
aaron.ballman added a comment. In https://reviews.llvm.org/D52334#1250166, @steveire wrote: > > I think a reasonable place would be docs/clang-tidy/index.rst in the > > "Getting Involved" area. > > Thanks, that's at least actionable, but not very specific. I've added docs > now. If they don't say what you want them to say, then please be specific > about what you want them to say. There's no reason to have 5 more rounds of > review here if being specific means only one more round is needed. It's a reasonable, if terse, start to documenting how to build clang-tidy. Mostly some mechanical changes to format the new docs the same as the other docs in the file. > Are you going to 'accept' this patch eventually, or will I still be waiting > for @alexfh once you're happy with it? Once I'm happy, I will accept it. If @alexfh has further comments, then they can be addressed at that time (pre or post commit). Comment at: docs/clang-tidy/index.rst:340-341 +If CMake is configured with `CLANG_ENABLE_STATIC_ANALYZER`, clang tidy will +not have the `static-analyzer` checks or the `mpi` checks. + Double backticks around `CLANG_ENABLE_STATIC_ANALYZER` Change "clang tidy" into :program:`clang-tidy` (with single backticks around clang-tidy) Change `static-analyzer` into `clang-analyzer-*` with double backticks Change `mpi` into `mpi-*` with double backticks will not have -> will not be built with support for Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52665: [X86] Add more of the icc unaligned load/store to/from 128 bit vector intrinsics
This revision was automatically updated to reflect the committed changes.
Closed by commit rC343388: [X86] Add more of the icc unaligned load/store
to/from 128 bit vector intrinsics (authored by ctopper, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D52665
Files:
lib/Headers/emmintrin.h
test/CodeGen/sse2-builtins.c
Index: lib/Headers/emmintrin.h
===
--- lib/Headers/emmintrin.h
+++ lib/Headers/emmintrin.h
@@ -1675,7 +1675,49 @@
long long __v;
} __attribute__((__packed__, __may_alias__));
long long __u = ((struct __loadu_si64*)__a)->__v;
- return __extension__ (__m128i)(__v2di){__u, 0L};
+ return __extension__ (__m128i)(__v2di){__u, 0LL};
+}
+
+/// Loads a 32-bit integer value to the low element of a 128-bit integer
+///vector and clears the upper element.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the VMOVD / MOVD instruction.
+///
+/// \param __a
+///A pointer to a 32-bit memory location. The address of the memory
+///location does not have to be aligned.
+/// \returns A 128-bit vector of [4 x i32] containing the loaded value.
+static __inline__ __m128i __DEFAULT_FN_ATTRS
+_mm_loadu_si32(void const *__a)
+{
+ struct __loadu_si32 {
+int __v;
+ } __attribute__((__packed__, __may_alias__));
+ int __u = ((struct __loadu_si32*)__a)->__v;
+ return __extension__ (__m128i)(__v4si){__u, 0, 0, 0};
+}
+
+/// Loads a 16-bit integer value to the low element of a 128-bit integer
+///vector and clears the upper element.
+///
+/// \headerfile
+///
+/// This intrinsic does not correspond to a specific instruction.
+///
+/// \param __a
+///A pointer to a 16-bit memory location. The address of the memory
+///location does not have to be aligned.
+/// \returns A 128-bit vector of [8 x i16] containing the loaded value.
+static __inline__ __m128i __DEFAULT_FN_ATTRS
+_mm_loadu_si16(void const *__a)
+{
+ struct __loadu_si16 {
+short __v;
+ } __attribute__((__packed__, __may_alias__));
+ short __u = ((struct __loadu_si16*)__a)->__v;
+ return __extension__ (__m128i)(__v8hi){__u, 0, 0, 0, 0, 0, 0, 0};
}
/// Loads a 64-bit double-precision value to the low element of a
@@ -3993,6 +4035,69 @@
((struct __storeu_si128*)__p)->__v = __b;
}
+/// Stores a 64-bit integer value from the low element of a 128-bit integer
+///vector.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the VMOVQ / MOVQ instruction.
+///
+/// \param __p
+///A pointer to a 64-bit memory location. The address of the memory
+///location does not have to be algned.
+/// \param __b
+///A 128-bit integer vector containing the value to be stored.
+static __inline__ void __DEFAULT_FN_ATTRS
+_mm_storeu_si64(void const *__p, __m128i __b)
+{
+ struct __storeu_si64 {
+long long __v;
+ } __attribute__((__packed__, __may_alias__));
+ ((struct __storeu_si64*)__p)->__v = ((__v2di)__b)[0];
+}
+
+/// Stores a 32-bit integer value from the low element of a 128-bit integer
+///vector.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the VMOVD / MOVD instruction.
+///
+/// \param __p
+///A pointer to a 32-bit memory location. The address of the memory
+///location does not have to be aligned.
+/// \param __b
+///A 128-bit integer vector containing the value to be stored.
+static __inline__ void __DEFAULT_FN_ATTRS
+_mm_storeu_si32(void const *__p, __m128i __b)
+{
+ struct __storeu_si32 {
+int __v;
+ } __attribute__((__packed__, __may_alias__));
+ ((struct __storeu_si32*)__p)->__v = ((__v4si)__b)[0];
+}
+
+/// Stores a 16-bit integer value from the low element of a 128-bit integer
+///vector.
+///
+/// \headerfile
+///
+/// This intrinsic does not correspond to a specific instruction.
+///
+/// \param __p
+///A pointer to a 16-bit memory location. The address of the memory
+///location does not have to be aligned.
+/// \param __b
+///A 128-bit integer vector containing the value to be stored.
+static __inline__ void __DEFAULT_FN_ATTRS
+_mm_storeu_si16(void const *__p, __m128i __b)
+{
+ struct __storeu_si16 {
+short __v;
+ } __attribute__((__packed__, __may_alias__));
+ ((struct __storeu_si16*)__p)->__v = ((__v8hi)__b)[0];
+}
+
/// Moves bytes selected by the mask from the first operand to the
///specified unaligned memory location. When a mask bit is 1, the
///corresponding byte is written, otherwise it is not written.
Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -721,6 +721,30 @@
return _mm_loadu_si64(A);
}
+__m128i test_mm_loadu_si32(void const* A) {
+ // CHECK-LABEL: test_mm_loadu_si32
+ // CHECK: load i32, i32* %{{.*}}, align 1{{$}}
+ // CHECK: insertelement <4 x i32> undef, i32 %{{.*}}, i32 0
+ // CHECK: insertelement <4 x i32> %{{.*}}, i32 0, i32 1
+ // CHECK:
r343388 - [X86] Add more of the icc unaligned load/store to/from 128 bit vector intrinsics
Author: ctopper
Date: Sat Sep 29 10:49:42 2018
New Revision: 343388
URL: http://llvm.org/viewvc/llvm-project?rev=343388=rev
Log:
[X86] Add more of the icc unaligned load/store to/from 128 bit vector intrinsics
Summary:
This patch adds
_mm_loadu_si32
_mm_loadu_si16
_mm_storeu_si64
_mm_storeu_si32
_mm_storeu_si16
We already had _mm_load_si64.
Reviewers: spatel, RKSimon
Reviewed By: RKSimon
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52665
Modified:
cfe/trunk/lib/Headers/emmintrin.h
cfe/trunk/test/CodeGen/sse2-builtins.c
Modified: cfe/trunk/lib/Headers/emmintrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/emmintrin.h?rev=343388=343387=343388=diff
==
--- cfe/trunk/lib/Headers/emmintrin.h (original)
+++ cfe/trunk/lib/Headers/emmintrin.h Sat Sep 29 10:49:42 2018
@@ -1675,7 +1675,49 @@ _mm_loadu_si64(void const *__a)
long long __v;
} __attribute__((__packed__, __may_alias__));
long long __u = ((struct __loadu_si64*)__a)->__v;
- return __extension__ (__m128i)(__v2di){__u, 0L};
+ return __extension__ (__m128i)(__v2di){__u, 0LL};
+}
+
+/// Loads a 32-bit integer value to the low element of a 128-bit integer
+///vector and clears the upper element.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the VMOVD / MOVD instruction.
+///
+/// \param __a
+///A pointer to a 32-bit memory location. The address of the memory
+///location does not have to be aligned.
+/// \returns A 128-bit vector of [4 x i32] containing the loaded value.
+static __inline__ __m128i __DEFAULT_FN_ATTRS
+_mm_loadu_si32(void const *__a)
+{
+ struct __loadu_si32 {
+int __v;
+ } __attribute__((__packed__, __may_alias__));
+ int __u = ((struct __loadu_si32*)__a)->__v;
+ return __extension__ (__m128i)(__v4si){__u, 0, 0, 0};
+}
+
+/// Loads a 16-bit integer value to the low element of a 128-bit integer
+///vector and clears the upper element.
+///
+/// \headerfile
+///
+/// This intrinsic does not correspond to a specific instruction.
+///
+/// \param __a
+///A pointer to a 16-bit memory location. The address of the memory
+///location does not have to be aligned.
+/// \returns A 128-bit vector of [8 x i16] containing the loaded value.
+static __inline__ __m128i __DEFAULT_FN_ATTRS
+_mm_loadu_si16(void const *__a)
+{
+ struct __loadu_si16 {
+short __v;
+ } __attribute__((__packed__, __may_alias__));
+ short __u = ((struct __loadu_si16*)__a)->__v;
+ return __extension__ (__m128i)(__v8hi){__u, 0, 0, 0, 0, 0, 0, 0};
}
/// Loads a 64-bit double-precision value to the low element of a
@@ -3993,6 +4035,69 @@ _mm_storeu_si128(__m128i *__p, __m128i _
((struct __storeu_si128*)__p)->__v = __b;
}
+/// Stores a 64-bit integer value from the low element of a 128-bit integer
+///vector.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the VMOVQ / MOVQ instruction.
+///
+/// \param __p
+///A pointer to a 64-bit memory location. The address of the memory
+///location does not have to be algned.
+/// \param __b
+///A 128-bit integer vector containing the value to be stored.
+static __inline__ void __DEFAULT_FN_ATTRS
+_mm_storeu_si64(void const *__p, __m128i __b)
+{
+ struct __storeu_si64 {
+long long __v;
+ } __attribute__((__packed__, __may_alias__));
+ ((struct __storeu_si64*)__p)->__v = ((__v2di)__b)[0];
+}
+
+/// Stores a 32-bit integer value from the low element of a 128-bit integer
+///vector.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the VMOVD / MOVD instruction.
+///
+/// \param __p
+///A pointer to a 32-bit memory location. The address of the memory
+///location does not have to be aligned.
+/// \param __b
+///A 128-bit integer vector containing the value to be stored.
+static __inline__ void __DEFAULT_FN_ATTRS
+_mm_storeu_si32(void const *__p, __m128i __b)
+{
+ struct __storeu_si32 {
+int __v;
+ } __attribute__((__packed__, __may_alias__));
+ ((struct __storeu_si32*)__p)->__v = ((__v4si)__b)[0];
+}
+
+/// Stores a 16-bit integer value from the low element of a 128-bit integer
+///vector.
+///
+/// \headerfile
+///
+/// This intrinsic does not correspond to a specific instruction.
+///
+/// \param __p
+///A pointer to a 16-bit memory location. The address of the memory
+///location does not have to be aligned.
+/// \param __b
+///A 128-bit integer vector containing the value to be stored.
+static __inline__ void __DEFAULT_FN_ATTRS
+_mm_storeu_si16(void const *__p, __m128i __b)
+{
+ struct __storeu_si16 {
+short __v;
+ } __attribute__((__packed__, __may_alias__));
+ ((struct __storeu_si16*)__p)->__v = ((__v8hi)__b)[0];
+}
+
/// Moves bytes selected by the mask from the first operand to the
///specified unaligned memory location. When a mask bit is 1, the
///corresponding byte is written, otherwise it is not written.
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
steveire marked 2 inline comments as done. steveire added a comment. > I think a reasonable place would be docs/clang-tidy/index.rst in the "Getting > Involved" area. That's at least actionable, but not very specific. I've added docs now. If they don't say what you want them to say, then please be specific about what you want them to say. There's no reason to have 5 more rounds of review here if being specific means only one more round is needed. Are you going to 'accept' this patch eventually, or will I still be waiting for @alexfh once you're happy with it? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
steveire updated this revision to Diff 167615.
steveire added a comment.
Herald added a subscriber: arphaman.
Docs
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
Files:
CMakeLists.txt
clang-tidy/CMakeLists.txt
clang-tidy/ClangTidy.cpp
clang-tidy/plugin/CMakeLists.txt
clang-tidy/plugin/ClangTidyPlugin.cpp
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
docs/clang-tidy/index.rst
test/CMakeLists.txt
test/clang-tidy/enable-alpha-checks.cpp
test/clang-tidy/mpi-buffer-deref.cpp
test/clang-tidy/mpi-type-mismatch.cpp
test/clang-tidy/nolint.cpp
test/clang-tidy/read_file_config.cpp
test/clang-tidy/static-analyzer-config.cpp
test/clang-tidy/static-analyzer.cpp
test/clang-tidy/temporaries.cpp
test/lit.cfg
unittests/CMakeLists.txt
Index: unittests/CMakeLists.txt
===
--- unittests/CMakeLists.txt
+++ unittests/CMakeLists.txt
@@ -18,8 +18,6 @@
add_subdirectory(clang-apply-replacements)
add_subdirectory(clang-move)
add_subdirectory(clang-query)
-if(CLANG_ENABLE_STATIC_ANALYZER)
- add_subdirectory(clang-tidy)
-endif()
+add_subdirectory(clang-tidy)
add_subdirectory(clangd)
add_subdirectory(include-fixer)
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -119,24 +119,22 @@
if config.clang_staticanalyzer:
config.available_features.add('static-analyzer')
-check_clang_tidy = os.path.join(
-config.test_source_root, "clang-tidy", "check_clang_tidy.py")
-config.substitutions.append(
-('%check_clang_tidy',
- '%s %s' % (config.python_executable, check_clang_tidy)) )
-clang_tidy_diff = os.path.join(
-config.test_source_root, "..", "clang-tidy", "tool", "clang-tidy-diff.py")
-config.substitutions.append(
-('%clang_tidy_diff',
- '%s %s' % (config.python_executable, clang_tidy_diff)) )
-run_clang_tidy = os.path.join(
-config.test_source_root, "..", "clang-tidy", "tool", "run-clang-tidy.py")
-config.substitutions.append(
-('%run_clang_tidy',
- '%s %s' % (config.python_executable, run_clang_tidy)) )
-else:
-# exclude the clang-tidy test directory
-config.excludes.append('clang-tidy')
+
+check_clang_tidy = os.path.join(
+config.test_source_root, "clang-tidy", "check_clang_tidy.py")
+config.substitutions.append(
+('%check_clang_tidy',
+ '%s %s' % (config.python_executable, check_clang_tidy)) )
+clang_tidy_diff = os.path.join(
+config.test_source_root, "..", "clang-tidy", "tool", "clang-tidy-diff.py")
+config.substitutions.append(
+('%clang_tidy_diff',
+ '%s %s' % (config.python_executable, clang_tidy_diff)) )
+run_clang_tidy = os.path.join(
+config.test_source_root, "..", "clang-tidy", "tool", "run-clang-tidy.py")
+config.substitutions.append(
+('%run_clang_tidy',
+ '%s %s' % (config.python_executable, run_clang_tidy)) )
clangd_benchmarks_dir = os.path.join(os.path.dirname(config.clang_tools_dir),
"tools", "clang", "tools", "extra",
Index: test/clang-tidy/temporaries.cpp
===
--- test/clang-tidy/temporaries.cpp
+++ test/clang-tidy/temporaries.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy -checks='-*,clang-analyzer-core.NullDereference' %s -- | FileCheck %s
struct NoReturnDtor {
Index: test/clang-tidy/static-analyzer.cpp
===
--- test/clang-tidy/static-analyzer.cpp
+++ test/clang-tidy/static-analyzer.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy %s -checks='-*,clang-analyzer-*' -- | FileCheck %s
extern void *malloc(unsigned long);
extern void free(void *);
Index: test/clang-tidy/static-analyzer-config.cpp
===
--- test/clang-tidy/static-analyzer-config.cpp
+++ test/clang-tidy/static-analyzer-config.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy %s -checks='-*,clang-analyzer-unix.Malloc' -config='{CheckOptions: [{ key: "clang-analyzer-unix.Malloc:Optimistic", value: true}]}' -- | FileCheck %s
typedef __typeof(sizeof(int)) size_t;
void *malloc(size_t);
Index: test/clang-tidy/read_file_config.cpp
===
--- test/clang-tidy/read_file_config.cpp
+++ test/clang-tidy/read_file_config.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: mkdir -p %T/read-file-config/
// RUN: cp %s %T/read-file-config/test.cpp
// RUN: echo 'Checks: "-*,modernize-use-nullptr"' > %T/read-file-config/.clang-tidy
Index: test/clang-tidy/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN:
[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)
lebedev.ri created this revision.
lebedev.ri added reviewers: rsmith, aaron.ballman, efriedma.
clang has `-Wextra-semi` (https://reviews.llvm.org/D43162), which is not
dictated by the currently selected standard.
While that is great, there is at least one more source of need-less semis -
'null statements'.
Sometimes, they are needed:
for(int x = 0; continueToDoWork(x); x++)
; // Ugly code, but the semi is needed here.
But sometimes they are just there for no reason:
switch(X) {
case 0:
return -2345;
case 5:
return 0;
default:
return 42;
}; // <- oops
;;; <- PS, still not diagnosed. Clearly this is junk.
As usual, things may or may not go sideways in the presence of macros.
While evaluating this diag on my codebase of interest, it was unsurprisingly
discovered that Google Test macros are *very* prone to this.
And it seems many issues are deep within the GTest itself, not
in the snippets passed from the codebase that uses GTest.
So after some thought, i decided not do issue a diagnostic if the semi
is within *any* macro, be it either from the normal header, or system header.
Fixes PR39111
Repository:
rC Clang
https://reviews.llvm.org/D52695
Files:
docs/ReleaseNotes.rst
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticParseKinds.td
lib/Parse/ParseStmt.cpp
test/Parser/extra-semi-resulting-in-nullstmt.cpp
Index: test/Parser/extra-semi-resulting-in-nullstmt.cpp
===
--- /dev/null
+++ test/Parser/extra-semi-resulting-in-nullstmt.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -fsyntax-only -Wextra-semi-pedantic -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wextra-semi-pedantic -std=c++17 -verify %s
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -Wextra-semi-pedantic -fixit %t
+// RUN: %clang_cc1 -x c++ -Wextra-semi-pedantic -Werror %t
+
+#define GOODMACRO(varname) int varname
+#define BETTERMACRO(varname) GOODMACRO(varname);
+#define NULLMACRO(varname)
+
+void test() {
+ ; // expected-warning {{';' with no preceding expression is a null statement}}
+ ; // expected-warning {{';' with no preceding expression is a null statement}}
+
+ // clang-format: off
+ ;; // expected-warning 2 {{';' with no preceding expression is a null statement}}
+ // clang-format: on
+
+ {}; // expected-warning {{';' with no preceding expression is a null statement}}
+
+ {
+; // expected-warning {{';' with no preceding expression is a null statement}}
+ }
+
+ if (true) {
+; // expected-warning {{';' with no preceding expression is a null statement}}
+ }
+
+ GOODMACRO(v0); // OK
+
+ GOODMACRO(v1;) // OK
+
+ BETTERMACRO(v2) // OK
+
+ BETTERMACRO(v3;) // Extra ';', but within macro expansion, so ignored.
+
+ BETTERMACRO(v4); // expected-warning {{';' with no preceding expression is a null statement}}
+
+ BETTERMACRO(v5;); // expected-warning {{';' with no preceding expression is a null statement}}
+
+ NULLMACRO(v6) // OK
+
+ NULLMACRO(v7); // OK. This could be either GOODMACRO() or BETTERMACRO() situation, so we can't know we can drop it.
+
+ if (true)
+; // OK
+
+ while (true)
+; // OK
+
+ do
+; // OK
+ while (true);
+
+ for (;;) // OK
+; // OK
+
+#if __cplusplus >= 201703L
+ if (; true) // OK
+; // OK
+#endif
+}
Index: lib/Parse/ParseStmt.cpp
===
--- lib/Parse/ParseStmt.cpp
+++ lib/Parse/ParseStmt.cpp
@@ -233,7 +233,13 @@
return ParseCompoundStatement();
case tok::semi: { // C99 6.8.3p3: expression[opt] ';'
bool HasLeadingEmptyMacro = Tok.hasLeadingEmptyMacro();
-return Actions.ActOnNullStmt(ConsumeToken(), HasLeadingEmptyMacro);
+SourceLocation SemiLocation = ConsumeToken();
+if (!HasLeadingEmptyMacro && getCurScope()->isCompoundStmtScope() &&
+!SemiLocation.isMacroID()) {
+ Diag(SemiLocation, diag::warn_null_statement)
+ << FixItHint::CreateRemoval(SemiLocation);
+}
+return Actions.ActOnNullStmt(SemiLocation, HasLeadingEmptyMacro);
}
case tok::kw_if: // C99 6.8.4.1: if-statement
Index: include/clang/Basic/DiagnosticParseKinds.td
===
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -53,6 +53,9 @@
def warn_extra_semi_after_mem_fn_def : Warning<
"extra ';' after member function definition">,
InGroup, DefaultIgnore;
+def warn_null_statement : Warning<
+ "';' with no preceding expression is a null statement">,
+ InGroup, DefaultIgnore;
def ext_thread_before : Extension<"'__thread' before '%0'">;
def ext_keyword_as_ident : ExtWarn<
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
aaron.ballman added a comment. In https://reviews.llvm.org/D52334#1249875, @steveire wrote: > @aaron.ballman Happy to add a note to the docs, but your request leaves me > wondering where? > > AFAIK, the fact that `clang-tidy` wasn't built at all when using > `CLANG_ENABLE_STATIC_ANALYZER` is not documented, so I can't just update that. I think a reasonable place would be `docs/clang-tidy/index.rst` in the "Getting Involved" area. We don't currently have any building instructions there, but this at least gets us started with something that may not be obvious to someone trying to get involved in the project. > Also AFAIK, other build switches don't have such exhaustive documentation > that they mention things like this. Is this so important to document that it > needs to be done here? We do document these sort of switches in LLVM and Clang (e.g., https://llvm.org/docs/CMake.html). That we don't for extra tools is more a bug than a feature, IMO. > I don't think it is that important. I think it's just a bug that clang-tidy > is entirely excluded from the build when not using > `CLANG_ENABLE_STATIC_ANALYZER`. This commit fixes that bug, so it would be > great to get a LGTM from someone/anyone so I can commit it.. This patch > definitely does not need as much discussion as it is getting. It's just > enabling the build of clang-tidy when not building the static analyzer. It turns out that at least one of the people reviewing your patch disagrees. :-) It's hard to argue that this is fixing a bug when we never documented what you would get in the first place and the status quo is no static analyzer == no clang-tidy. I see this as implementing a new feature: allowing clang-tidy to be built without static analyzer support. Part of new features is documenting them. I'm sorry that you find this process frustrating. I think the documentation is important here because we have package maintainers that need to know how to build things, and this mysterious undocumented flag changes the behavior of the resulting executable in pretty fundamental ways. It's unfortunate that we didn't document it when it was introduced, and now that we're touching it in interesting ways, we should fix that. Comment at: clang-tidy/CMakeLists.txt:29 +if(CLANG_ENABLE_STATIC_ANALYZER) +target_link_libraries(clangTidy PRIVATE + clangStaticAnalyzerCore steveire wrote: > aaron.ballman wrote: > > Indentation looks off here, same below. > Yes, I used the same indentation (none) as the previous code. I can indent > this as you wish. I think it's more clear to indent the body of the if statements, like you've switched to. Someday we should probably figure out `cmake-format` and try to make use of it to solve this kind of thing. Comment at: clang-tidy/plugin/CMakeLists.txt:32-33 +if(CLANG_ENABLE_STATIC_ANALYZER) +target_link_libraries(clangTidyPlugin PRIVATE + clangTidyMPIModule +) Indentation. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50403: [clang-format]AlignConsecutiveAssignments
owenpan requested changes to this revision.
owenpan added inline comments.
This revision now requires changes to proceed.
Comment at: include/clang/Format/Format.h:91-93
+ /// int ddd += 12;
+ /// int ee += 22;
+ /// int f += 23;
These are invalid C++ examples.
Comment at: lib/Format/WhitespaceManager.cpp:435-451
+ std::vector assignment_tokens =
+{tok::equal, tok::pipeequal, tok::caretequal, tok::percentequal,
+ tok::ampequal, tok::plusequal, tok::minusequal, tok::starequal,
+ tok::slashequal, tok::lesslessequal, tok::greatergreaterequal};
+ for (auto assignment_token : assignment_tokens)
+ {
+AlignTokens(Style,
It would be simpler to just use `isOneOf(tok::equal, tok::pipeequal, ...)` here.
Repository:
rC Clang
https://reviews.llvm.org/D50403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52665: [X86] Add more of the icc unaligned load/store to/from 128 bit vector intrinsics
RKSimon accepted this revision. RKSimon added a comment. This revision is now accepted and ready to land. Thanks for checking - LGTM with the equivalent updates to sse2-intrinsics-fast-isel.ll https://reviews.llvm.org/D52665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52665: [X86] Add more of the icc unaligned load/store to/from 128 bit vector intrinsics
craig.topper added a comment. Looks like the load and store 64 are supported in 32-bit mode in icc https://godbolt.org/z/Wpezeu https://reviews.llvm.org/D52665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
oleg.smolsky added a comment.
In https://reviews.llvm.org/D52676#1250071, @krasimir wrote:
> In https://reviews.llvm.org/D52676#1249828, @oleg.smolsky wrote:
>
> > In https://reviews.llvm.org/D52676#1249784, @krasimir wrote:
> >
> > > IMO `BinPackArguments==false` does not imply that there should be a line
> > > break before the first arguments, only that there should not be two
> > > arguments from the same argument list that appear on the same line.
> >
> >
> > That's right. However consider the following points:
> >
> > 1. a lambda function is already placed onto its own line when it is the
> > first arg (as you can see in the first test)
>
>
> I believe that a newline before a first arg lambda is controlled by a
> different mechanism independent of bin packing, probably by a rule that is
> more general than just "newline before first arg lambda". I think djasper@
> could know more about this case.
Hmm... perhaps. I have not been able to find an explicit rule for that.
>> 1. the other args are placed onto a distinct line
>> 2. this behavior looks very close to "bin packing"
>> 3. adding a new option for the minor cases seems to be an overkill
>>
>> Having said that, I can add a new explicit style option. Do you think that
>> will improve consensus? Would you expect test cases for positive and
>> negative values of the option?
>
> I don't see how the example before:
>
> void f() {
> something->Method2s(1,
> [this] {
> Do1();
> Do2();
> },
> 1);
> }
>
>
> is inconsistent, as not adding a newline before the first argument is
> typical, as in:
>
> $ clang-format -style='{BasedOnStyle: google, BinPackArguments: false}'
> test.cc ~
> void f() {
> something->Method2s("11",
>
> "",
> 3);
> }
Right, it's consistent with your example but inconsistent with with the
following two:
lambda at arg0:
void f() {
something->Method2s(
[this] {
Do1();
Do2();
},
1);
}
and two lambdas:
// Multiple lambdas in the same parentheses change indentation rules.
verifyFormat("SomeFunction(\n"
"[]() {\n"
" int i = 42;\n"
" return i;\n"
"},\n"
"[]() {\n"
" int j = 43;\n"
" return j;\n"
"});");
Repository:
rC Clang
https://reviews.llvm.org/D52676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
oleg.smolsky updated this revision to Diff 167611.
oleg.smolsky marked an inline comment as done.
oleg.smolsky added a comment.
Tweaked if/else/return structure, added comments. No functional changes.
Repository:
rC Clang
https://reviews.llvm.org/D52676
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11785,6 +11785,60 @@
" return j;\n"
"});");
+ // A lambda passed as arg0 is always pushed to the next line.
+ verifyFormat("void f() {\n"
+ " something->Method1(\n"
+ " [this] {\n"
+ "Do1();\n"
+ "Do2();\n"
+ " },\n"
+ " 1);\n"
+ "}\n");
+
+ // A lambda passed as arg1 forces arg0 to be pushed out when
+ // BinPackArguments=false.
+ {
+auto Style = getGoogleStyle();
+Style.BinPackArguments = false;
+verifyFormat("void f() {\n"
+ " something->Method2s(\n"
+ " 1,\n"
+ " [this] {\n"
+ "Do1();\n"
+ "Do2();\n"
+ " },\n"
+ " 1);\n"
+ "}\n",
+ Style);
+ }
+
+ // A lambda with a very long line forces arg0 to be pushed out irrespective of
+ // the BinPackArguments value.
+ verifyFormat("void f() {\n"
+ " something->Method2l(\n"
+ " 1,\n"
+ " [this] {\n"
+ "Do1();\n"
+ "D01();\n"
+ " },\n"
+ " 1);\n"
+ "}\n");
+
+ // Multiple lambdas are treated correctly even when there is a short arg0.
+ verifyFormat("void f() {\n"
+ " something->Method3(\n"
+ " 1,\n"
+ " [this] {\n"
+ "Do1();\n"
+ "Do2();\n"
+ " },\n"
+ " [this] {\n"
+ "Do1();\n"
+ "Do2();\n"
+ " },\n"
+ " 1);\n"
+ "}\n");
+
// More complex introducers.
verifyFormat("return [i, args...] {};");
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -988,18 +988,17 @@
Path.push_front(Best);
Best = Best->Previous;
}
-for (std::deque::iterator I = Path.begin(), E = Path.end();
- I != E; ++I) {
+for (auto I = Path.begin(), E = Path.end(); I != E; ++I) {
unsigned Penalty = 0;
formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
LLVM_DEBUG({
printLineState((*I)->Previous->State);
if ((*I)->NewLine) {
llvm::dbgs() << "Penalty for placing "
- << (*I)->Previous->State.NextToken->Tok.getName() << ": "
- << Penalty << "\n";
+ << (*I)->Previous->State.NextToken->Tok.getName()
+ << " on a new line: " << Penalty << "\n";
}
});
}
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -3043,6 +3043,22 @@
return true;
}
+ // Deal with lambda arguments in C++ - we want consistent line breaks whether
+ // they happen to be at arg0, arg1 or argN. The selection is a bit nuanced
+ // as aggressive line breaks only make sense when the user had set
+ // Style.BinPackArguments to 'false'.
+ if ((Style.Language == FormatStyle::LK_Cpp ||
+ Style.Language == FormatStyle::LK_ObjC) &&
+ Left.is(tok::l_paren) && Left.BlockParameterCount > 0 &&
+ !Right.isOneOf(tok::l_paren, TT_LambdaLSquare)) {
+// Always break lines in the "multiple lumbdas" case.
+if (Left.BlockParameterCount > 1)
+ return true;
+// Break lines with a lambda when the style disallows arg packing.
+if (!Style.BinPackArguments)
+ return true;
+ }
+
return false;
}
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1135,7 +1135,8 @@
// }, a, b, c);
if (Current.isNot(tok::comment) && Previous &&
Previous->isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) &&
-
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri added inline comments. Comment at: docs/ReleaseNotes.rst:96 +- New alias :doc:`cert-dcl16-c + ` to :doc:`readability-uppercase-literal-suffix JonasToth wrote: > lebedev.ri wrote: > > Eugene.Zelenko wrote: > > > Please move alias after new checks. > > BTW, is there some tool to actually add this alias? I didn't find it, and > > had to do it by hand.. > So far there is nothing, but would be a nice addition :) Oh, i was really hoping i simply did not find it :/ Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
krasimir added a comment.
In https://reviews.llvm.org/D52676#1249828, @oleg.smolsky wrote:
> In https://reviews.llvm.org/D52676#1249784, @krasimir wrote:
>
> > IMO `BinPackArguments==false` does not imply that there should be a line
> > break before the first arguments, only that there should not be two
> > arguments from the same argument list that appear on the same line.
>
>
> That's right. However consider the following points:
>
> 1. a lambda function is already placed onto its own line when it is the first
> arg (as you can see in the first test)
I believe that a newline before a first arg lambda is controlled by a different
mechanism independent of bin packing, probably by a rule that is more general
than just "newline before first arg lambda". I think djasper@ could know more
about this case.
> 1. the other args are placed onto a distinct line
> 2. this behavior looks very close to "bin packing"
> 3. adding a new option for the minor cases seems to be an overkill
>
> Having said that, I can add a new explicit style option. Do you think that
> will improve consensus? Would you expect test cases for positive and negative
> values of the option?
I don't see how the example before:
void f() {
something->Method2s(1,
[this] {
Do1();
Do2();
},
1);
}
is inconsistent, as not adding a newline before the first argument is typical,
as in:
$ clang-format -style='{BasedOnStyle: google, BinPackArguments: false}'
test.cc ~
void f() {
something->Method2s("11",
"",
3);
}
Repository:
rC Clang
https://reviews.llvm.org/D52676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52691: [clang-tidy] NFC use CHECK-NOTES in tests for performance-move-constructor-init
JonasToth created this revision.
JonasToth added reviewers: alexfh, aaron.ballman, hokein.
Herald added subscribers: cfe-commits, xazax.hun.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52691
Files:
test/clang-tidy/performance-move-constructor-init.cpp
Index: test/clang-tidy/performance-move-constructor-init.cpp
===
--- test/clang-tidy/performance-move-constructor-init.cpp
+++ test/clang-tidy/performance-move-constructor-init.cpp
@@ -30,9 +30,9 @@
struct D : B {
D() : B() {}
D(const D ) : B(RHS) {}
- // CHECK-MESSAGES: :[[@LINE+3]]:16: warning: move constructor initializes
base class by calling a copy constructor [performance-move-constructor-init]
- // CHECK-MESSAGES: 26:3: note: copy constructor being called
- // CHECK-MESSAGES: 27:3: note: candidate move constructor here
+ // CHECK-NOTES: :[[@LINE+3]]:16: warning: move constructor initializes base
class by calling a copy constructor [performance-move-constructor-init]
+ // CHECK-NOTES: 26:3: note: copy constructor being called
+ // CHECK-NOTES: 27:3: note: candidate move constructor here
D(D &) : B(RHS) {}
};
@@ -75,8 +75,10 @@
struct M {
B Mem;
- // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: move constructor initializes
class member by calling a copy constructor [performance-move-constructor-init]
+ // CHECK-NOTES: :[[@LINE+1]]:16: warning: move constructor initializes class
member by calling a copy constructor [performance-move-constructor-init]
M(M &) : Mem(RHS.Mem) {}
+ // CHECK-NOTES: 26:3: note: copy constructor being called
+ // CHECK-NOTES: 27:3: note: candidate move constructor here
};
struct N {
@@ -109,7 +111,10 @@
struct Positive {
Positive(Movable M) : M_(M) {}
- // CHECK-MESSAGES: [[@LINE-1]]:12: warning: pass by value and use std::move
[modernize-pass-by-value]
+ // CHECK-NOTES: [[@LINE-1]]:12: warning: pass by value and use std::move
[modernize-pass-by-value]
+ // CHECK-NOTES: 7:1: note: FIX-IT applied suggested code changes
+ // CHECK-NOTES: 113:28: note: FIX-IT applied suggested code changes
+ // CHECK-NOTES: 113:29: note: FIX-IT applied suggested code changes
// CHECK-FIXES: Positive(Movable M) : M_(std::move(M)) {}
Movable M_;
};
Index: test/clang-tidy/performance-move-constructor-init.cpp
===
--- test/clang-tidy/performance-move-constructor-init.cpp
+++ test/clang-tidy/performance-move-constructor-init.cpp
@@ -30,9 +30,9 @@
struct D : B {
D() : B() {}
D(const D ) : B(RHS) {}
- // CHECK-MESSAGES: :[[@LINE+3]]:16: warning: move constructor initializes base class by calling a copy constructor [performance-move-constructor-init]
- // CHECK-MESSAGES: 26:3: note: copy constructor being called
- // CHECK-MESSAGES: 27:3: note: candidate move constructor here
+ // CHECK-NOTES: :[[@LINE+3]]:16: warning: move constructor initializes base class by calling a copy constructor [performance-move-constructor-init]
+ // CHECK-NOTES: 26:3: note: copy constructor being called
+ // CHECK-NOTES: 27:3: note: candidate move constructor here
D(D &) : B(RHS) {}
};
@@ -75,8 +75,10 @@
struct M {
B Mem;
- // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: move constructor initializes class member by calling a copy constructor [performance-move-constructor-init]
+ // CHECK-NOTES: :[[@LINE+1]]:16: warning: move constructor initializes class member by calling a copy constructor [performance-move-constructor-init]
M(M &) : Mem(RHS.Mem) {}
+ // CHECK-NOTES: 26:3: note: copy constructor being called
+ // CHECK-NOTES: 27:3: note: candidate move constructor here
};
struct N {
@@ -109,7 +111,10 @@
struct Positive {
Positive(Movable M) : M_(M) {}
- // CHECK-MESSAGES: [[@LINE-1]]:12: warning: pass by value and use std::move [modernize-pass-by-value]
+ // CHECK-NOTES: [[@LINE-1]]:12: warning: pass by value and use std::move [modernize-pass-by-value]
+ // CHECK-NOTES: 7:1: note: FIX-IT applied suggested code changes
+ // CHECK-NOTES: 113:28: note: FIX-IT applied suggested code changes
+ // CHECK-NOTES: 113:29: note: FIX-IT applied suggested code changes
// CHECK-FIXES: Positive(Movable M) : M_(std::move(M)) {}
Movable M_;
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52690: [clang-tidy] NFC use CHECK-NOTES in tests for misc-misplaced-const
JonasToth created this revision.
JonasToth added reviewers: alexfh, aaron.ballman, hokein.
Herald added subscribers: cfe-commits, xazax.hun.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52690
Files:
test/clang-tidy/misc-misplaced-const.c
test/clang-tidy/misc-misplaced-const.cpp
Index: test/clang-tidy/misc-misplaced-const.cpp
===
--- test/clang-tidy/misc-misplaced-const.cpp
+++ test/clang-tidy/misc-misplaced-const.cpp
@@ -12,7 +12,8 @@
if (const cip i = 0)
;
- // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: 'i' declared with a
const-qualified typedef type; results in the type being 'int *const' instead of
'const int *'
+ // CHECK-NOTES: :[[@LINE+2]]:16: warning: 'i' declared with a
const-qualified typedef type; results in the type being 'int *const' instead of
'const int *'
+ // CHECK-NOTES: :[[@LINE-12]]:14: note: typedef declared here
if (const ip i = 0)
;
}
Index: test/clang-tidy/misc-misplaced-const.c
===
--- test/clang-tidy/misc-misplaced-const.c
+++ test/clang-tidy/misc-misplaced-const.c
@@ -14,28 +14,33 @@
// Not ok
const ip i3 = 0;
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'i3' declared with a
const-qualified typedef type; results in the type being 'int *const' instead of
'const int *'
+ // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i3' declared with a
const-qualified typedef type; results in the type being 'int *const' instead of
'const int *'
+ // CHECK-NOTES: :[[@LINE-14]]:14: note: typedef declared here
ip const i4 = 0;
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'i4' declared with a
const-qualified
+ // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i4' declared with a
const-qualified
+ // CHECK-NOTES: :[[@LINE-18]]:14: note: typedef declared here
const volatile ip i5 = 0;
- // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'i5' declared with a
const-qualified typedef type; results in the type being 'int *const volatile'
instead of 'const int *volatile'
+ // CHECK-NOTES: :[[@LINE-1]]:21: warning: 'i5' declared with a
const-qualified typedef type; results in the type being 'int *const volatile'
instead of 'const int *volatile'
+ // CHECK-NOTES: :[[@LINE-22]]:14: note: typedef declared here
}
void func2(const plain_i *i1,
const cip i2,
const ip i3,
- // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'i3' declared with a
const-qualified
+ // CHECK-NOTES: :[[@LINE-1]]:21: warning: 'i3' declared with a
const-qualified
+ // CHECK-NOTES: :[[@LINE-29]]:14: note: typedef declared here
const int *i4) {
}
struct S {
const int *i0;
const plain_i *i1;
const cip i2;
const ip i3;
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'i3' declared with a
const-qualified
+ // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i3' declared with a
const-qualified
+ // CHECK-NOTES: :[[@LINE-39]]:14: note: typedef declared here
};
// Function pointers should not be diagnosed because a function
Index: test/clang-tidy/misc-misplaced-const.cpp
===
--- test/clang-tidy/misc-misplaced-const.cpp
+++ test/clang-tidy/misc-misplaced-const.cpp
@@ -12,7 +12,8 @@
if (const cip i = 0)
;
- // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: 'i' declared with a const-qualified typedef type; results in the type being 'int *const' instead of 'const int *'
+ // CHECK-NOTES: :[[@LINE+2]]:16: warning: 'i' declared with a const-qualified typedef type; results in the type being 'int *const' instead of 'const int *'
+ // CHECK-NOTES: :[[@LINE-12]]:14: note: typedef declared here
if (const ip i = 0)
;
}
Index: test/clang-tidy/misc-misplaced-const.c
===
--- test/clang-tidy/misc-misplaced-const.c
+++ test/clang-tidy/misc-misplaced-const.c
@@ -14,28 +14,33 @@
// Not ok
const ip i3 = 0;
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'i3' declared with a const-qualified typedef type; results in the type being 'int *const' instead of 'const int *'
+ // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i3' declared with a const-qualified typedef type; results in the type being 'int *const' instead of 'const int *'
+ // CHECK-NOTES: :[[@LINE-14]]:14: note: typedef declared here
ip const i4 = 0;
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'i4' declared with a const-qualified
+ // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i4' declared with a const-qualified
+ // CHECK-NOTES: :[[@LINE-18]]:14: note: typedef declared here
const volatile ip i5 = 0;
- // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'i5' declared with a const-qualified typedef type; results in the type being 'int *const volatile' instead of 'const int *volatile'
+ // CHECK-NOTES: :[[@LINE-1]]:21: warning: 'i5' declared with a const-qualified typedef
[PATCH] D52688: [clang-tidy] NFC use CHECK-NOTES in tests for fuchsia-default-arguments
JonasToth created this revision.
JonasToth added reviewers: alexfh, aaron.ballman, hokein.
Herald added subscribers: cfe-commits, xazax.hun.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52688
Files:
test/clang-tidy/fuchsia-default-arguments.cpp
Index: test/clang-tidy/fuchsia-default-arguments.cpp
===
--- test/clang-tidy/fuchsia-default-arguments.cpp
+++ test/clang-tidy/fuchsia-default-arguments.cpp
@@ -1,14 +1,14 @@
// RUN: %check_clang_tidy %s fuchsia-default-arguments %t
int foo(int value = 5) { return value; }
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: declaring a parameter with a
default argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-1]]:9: warning: declaring a parameter with a default
argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-2]]:18: note: FIX-IT applied suggested code changes
// CHECK-FIXES: int foo(int value) { return value; }
int f() {
foo();
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: calling a function that uses a
default argument is disallowed [fuchsia-default-arguments]
- // CHECK-NEXT: note: default parameter was declared here:
- // CHECK-NEXT: int foo(int value = 5) { return value; }
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: calling a function that uses a
default argument is disallowed [fuchsia-default-arguments]
+ // CHECK-NOTES: [[@LINE-8]]:9: note: default parameter was declared here
}
int bar(int value) { return value; }
@@ -21,16 +21,18 @@
class Baz {
public:
int a(int value = 5) { return value; }
- // CHECK-MESSAGES: [[@LINE-1]]:9: warning: declaring a parameter with a
default argument is disallowed [fuchsia-default-arguments]
+ // CHECK-NOTES: [[@LINE-1]]:9: warning: declaring a parameter with a default
argument is disallowed [fuchsia-default-arguments]
+ // CHECK-NOTES: [[@LINE-2]]:18: note: FIX-IT applied suggested code changes
// CHECK-FIXES: int a(int value) { return value; }
int b(int value) { return value; }
};
class Foo {
// Fix should be suggested in declaration
int a(int value = 53);
- // CHECK-MESSAGES: [[@LINE-1]]:9: warning: declaring a parameter with a
default argument is disallowed [fuchsia-default-arguments]
+ // CHECK-NOTES: [[@LINE-1]]:9: warning: declaring a parameter with a default
argument is disallowed [fuchsia-default-arguments]
+ // CHECK-NOTES: [[@LINE-2]]:18: note: FIX-IT applied suggested code changes
// CHECK-FIXES: int a(int value);
};
@@ -41,21 +43,23 @@
// Elided functions
void f(int = 5) {};
-// CHECK-MESSAGES: [[@LINE-1]]:8: warning: declaring a parameter with a
default argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-1]]:8: warning: declaring a parameter with a default
argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-2]]:11: note: FIX-IT applied suggested code changes
// CHECK-FIXES: void f(int) {};
void g(int) {};
// Should not suggest fix for macro-defined parameters
#define D(val) = val
void h(int i D(5));
-// CHECK-MESSAGES: [[@LINE-1]]:8: warning: declaring a parameter with a
default argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-1]]:8: warning: declaring a parameter with a default
argument is disallowed [fuchsia-default-arguments]
// CHECK-FIXES-NOT: void h(int i);
void x(int i);
void x(int i = 12);
-// CHECK-MESSAGES: [[@LINE-1]]:8: warning: declaring a parameter with a
default argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-1]]:8: warning: declaring a parameter with a default
argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-2]]:13: note: FIX-IT applied suggested code changes
// CHECK-FIXES: void x(int i);
void x(int i) {}
@@ -65,17 +69,18 @@
};
void S::x(int i = 12) {}
-// CHECK-MESSAGES: [[@LINE-1]]:11: warning: declaring a parameter with a
default argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-1]]:11: warning: declaring a parameter with a default
argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-2]]:16: note: FIX-IT applied suggested code changes
// CHECK-FIXES: void S::x(int i) {}
int main() {
S s;
s.x();
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: calling a function that uses a
default argument is disallowed [fuchsia-default-arguments]
- // CHECK-NEXT: note: default parameter was declared here:
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: calling a function that uses a
default argument is disallowed [fuchsia-default-arguments]
+ // CHECK-NOTES: [[@LINE-9]]:11: note: default parameter was declared here
// CHECK-NEXT: void S::x(int i = 12) {}
x();
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: calling a function that uses a
default argument is disallowed [fuchsia-default-arguments]
- // CHECK-NEXT: note: default parameter was declared here:
+ // CHECK-NOTES: [[@LINE-1]]:3: warning:
[PATCH] D52687: [clang-tidy] NFC use CHECK-NOTES in tests for cppcoreguidelines-owning-memory
JonasToth created this revision.
JonasToth added reviewers: alexfh, aaron.ballman, hokein.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52687
Files:
test/clang-tidy/cppcoreguidelines-owning-memory-containers.cpp
test/clang-tidy/cppcoreguidelines-owning-memory.cpp
Index: test/clang-tidy/cppcoreguidelines-owning-memory.cpp
===
--- test/clang-tidy/cppcoreguidelines-owning-memory.cpp
+++ test/clang-tidy/cppcoreguidelines-owning-memory.cpp
@@ -36,17 +36,17 @@
int *returns_no_owner1() { return nullptr; }
int *returns_no_owner2() {
return new int(42);
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a newly created resource of type 'int *' or 'gsl::owner<>' from a function whose return type is not 'gsl::owner<>'
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: returning a newly created resource of type 'int *' or 'gsl::owner<>' from a function whose return type is not 'gsl::owner<>'
}
int *returns_no_owner3() {
int *should_be_owner = new int(42);
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
return should_be_owner;
}
int *returns_no_owner4() {
gsl::owner owner = new int(42);
return owner;
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a newly created resource of type 'int *' or 'gsl::owner<>' from a function whose return type is not 'gsl::owner<>'
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: returning a newly created resource of type 'int *' or 'gsl::owner<>' from a function whose return type is not 'gsl::owner<>'
}
unique_ptr returns_no_owner5() {
@@ -71,11 +71,11 @@
int stack_int2;
gsl::owner owned_int1 = _int1; // BAD
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
gsl::owner owned_int2;
owned_int2 = _int2; // BAD since no owner, bad since uninitialized
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'int *'
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'int *'
gsl::owner owned_int3 = new int(42); // Good
owned_int3 = nullptr; // Good
@@ -92,41 +92,41 @@
owned_int6 = owned_int3; // BAD, because reassignment without resource release
auto owned_int7 = returns_owner1(); // Bad, since type deduction eliminates the owner wrapper
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
- // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+ // CHECK-NOTES: [[@LINE-2]]:3: note: type deduction did not result in an owner
const auto owned_int8 = returns_owner2(); // Bad, since type deduction eliminates the owner wrapper
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *const' with a newly created 'gsl::owner<>'
- // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: initializing non-owner 'int *const' with a newly created 'gsl::owner<>'
+ // CHECK-NOTES: [[@LINE-2]]:3: note: type deduction did not result in an owner
gsl::owner owned_int9 = returns_owner1(); // Ok
int *unowned_int3 = returns_owner1();// Bad
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
gsl::owner owned_int10;
owned_int10 = returns_owner1(); // Ok
int *unowned_int4;
unowned_int4 = returns_owner1(); // Bad
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: assigning newly created 'gsl::owner<>' to non-owner 'int *'
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: assigning newly created 'gsl::owner<>' to non-owner 'int *'
gsl::owner owned_int11 = returns_no_owner1(); // Bad since no owner
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
gsl::owner owned_int12;
owned_int12 = returns_no_owner1(); // Bad since no owner
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'int *'
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'int *'
int *unowned_int5
[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length
Charusso added inline comments.
Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:1038
+
+ SmallString<128> NewAddNullTermExprStr;
+ NewAddNullTermExprStr = "\n";
aaron.ballman wrote:
> JonasToth wrote:
> > whisperity wrote:
> > > aaron.ballman wrote:
> > > > This should be done using a `Twine`.
> > > Can you please give an example of how to well-approach this? We had
> > > issues with using Twines on the stack and then later on running into
> > > weird undefined behaviour. The documentation is not clear to a lot of our
> > > colleagues on where the `Twine`'s usage barriers are... (Asking this not
> > > just for @Charusso but also for a few more colleagues,
> > > @baloghadamsoftware e.g.)
> > naive approach:
> >
> > ```
> > std::string New =
> > Twine("\n").concat(SpaceBeforeStmtStr).concat(exprToStr(..))...).str();
> > ```
> > I think retrieving a `SmallString` is not supported. Please note, that
> > `Twine` does support taking integer and floats directly without prior
> > conversion.
> > You can use `operator+` instead of `concat` too.
> >
> > Did this help or did you have more specific issues?
> The important bit is -- don't try to store a Twine (locally or otherwise).
> It's fine as a parameter in a function, but otherwise you should try to use
> it only as part of an expression. e.g., `(Twine("foo") + "bar" + baz).str()`
> is a good pattern to get used to.
+1 for @aaron.ballman's example, it is an unspoken standard.
https://reviews.llvm.org/D45050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length
Charusso updated this revision to Diff 167597.
Charusso marked 37 inline comments as done.
Charusso edited the summary of this revision.
Charusso added a comment.
Thanks for the very in-depth review @aaron.ballman!
I have not found a way to obtain the instance of Preprocessor nicely, but it is
working well. Changes:
- Huge refactor `.`
- Macro definition checking is rely on Preprocessor.
- `isUnjectUL()` is now a function, not a static variable.
- `WantToUseSafeFunctions` is the new integer specifying the
`AreSafeFunctionsAvailable` variable without the override feature rely on the
necessary macros has been defined.
- Added a test case in
`bugprone-not-null-terminated-result-memcpy-before-safe.c` if the user wants to
use safe functions but it is unavailable.
- Extra: the LOC is now a round number.
https://reviews.llvm.org/D45050
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
clang-tidy/bugprone/NotNullTerminatedResultCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-not-null-terminated-result-in-initialization-strlen.c
test/clang-tidy/bugprone-not-null-terminated-result-memcpy-before-safe.c
test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-cxx.cpp
test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-other.c
test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe.c
test/clang-tidy/bugprone-not-null-terminated-result-strlen.c
test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp
test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
Index: test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
@@ -0,0 +1,136 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++11
+
+#define __STDC_LIB_EXT1__ 1
+#define __STDC_WANT_LIB_EXT1__ 1
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+template
+errno_t wcsncpy_s(wchar_t ()[size], const wchar_t *src, size_t length);
+errno_t wcsncpy_s(wchar_t *, size_t, const wchar_t *, size_t);
+
+template
+wchar_t *wcsncpy(wchar_t ()[size], const wchar_t *src, size_t length);
+wchar_t *wcsncpy(wchar_t *, const wchar_t *, size_t);
+
+template
+errno_t wcscpy_s(wchar_t ()[size], const wchar_t *);
+errno_t wcscpy_s(wchar_t *, size_t, const wchar_t *);
+
+template
+wchar_t *wcscpy(wchar_t ()[size], const wchar_t *);
+wchar_t *wcscpy(wchar_t *, const wchar_t *);
+
+errno_t wmemcpy_s(wchar_t *, size_t, const wchar_t *, size_t);
+wchar_t *wmemcpy(wchar_t *, const wchar_t *, size_t);
+
+
+//===--===//
+// wmemcpy() - destination array tests
+//===--===//
+
+void bad_wmemcpy_known_dest(const wchar_t *src) {
+ wchar_t dest01[13];
+ wmemcpy(dest01, src, wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wchar_t dest01[14];
+ // CHECK-FIXES-NEXT: wcscpy_s(dest01, src);
+}
+
+void good_wmemcpy_known_dest(const wchar_t *src) {
+ wchar_t dst01[14];
+ wcscpy_s(dst01, src);
+}
+
+//===--===//
+// wmemcpy() - length tests
+//===--===//
+
+void bad_wmemcpy_full_source_length(const wchar_t *src) {
+ wchar_t dest20[13];
+ wmemcpy(dest20, src, wcslen(src));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wchar_t dest20[14];
+ // CHECK-FIXES-NEXT: wcscpy_s(dest20, src);
+}
+
+void good_wmemcpy_full_source_length(const wchar_t *src) {
+ wchar_t dst20[14];
+ wcscpy_s(dst20, src);
+}
+
+void bad_wmemcpy_partial_source_length(const wchar_t *src) {
+ wchar_t dest21[13];
+ wmemcpy(dest21, src, wcslen(src) - 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+ // CHECK-FIXES: wchar_t dest21[14];
+ // CHECK-FIXES-NEXT: wcsncpy_s(dest21, src, wcslen(src) - 1);
+}
+
+void good_wmemcpy_partial_source_length(const wchar_t *src) {
+ wchar_t dst21[14];
+ wcsncpy_s(dst21, src, wcslen(src) - 1);
+}
+
+//===--===//
+// wmemcpy_s() - destination array tests
+//===--===//
+
+void
[PATCH] D52686: [clang-tidy] use CHECK-NOTES in test for cppgoreguidelines-avoid-goto
JonasToth created this revision.
JonasToth added reviewers: alexfh, aaron.ballman, hokein.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52686
Files:
test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
Index: test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
===
--- test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
+++ test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
@@ -5,23 +5,23 @@
int main() {
noop();
goto jump_to_me;
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow
control
- // CHECK-MESSAGES: [[@LINE+3]]:1: note: label defined here
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+ // CHECK-NOTES: [[@LINE+3]]:1: note: label defined here
noop();
jump_to_me:;
jump_backwards:;
noop();
goto jump_backwards;
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow
control
- // CHECK-MESSAGES: [[@LINE-4]]:1: note: label defined here
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+ // CHECK-NOTES: [[@LINE-4]]:1: note: label defined here
goto jump_in_line;
;
jump_in_line:;
- // CHECK-MESSAGES: [[@LINE-3]]:3: warning: avoid using 'goto' for flow
control
- // CHECK-MESSAGES: [[@LINE-2]]:1: note: label defined here
+ // CHECK-NOTES: [[@LINE-3]]:3: warning: avoid using 'goto' for flow control
+ // CHECK-NOTES: [[@LINE-2]]:1: note: label defined here
// Test the GNU extension
https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
some_label:;
@@ -132,8 +132,8 @@
for (int j = 0; j < 10; ++j) {
if (i * j > 80)
goto before_the_loop;
- // CHECK-MESSAGES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow
control
- // CHECK-MESSAGES: [[@LINE-8]]:1: note: label defined here
+ // CHECK-NOTES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow
control
+ // CHECK-NOTES: [[@LINE-8]]:1: note: label defined here
}
}
}
Index: test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
===
--- test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
+++ test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
@@ -5,23 +5,23 @@
int main() {
noop();
goto jump_to_me;
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
- // CHECK-MESSAGES: [[@LINE+3]]:1: note: label defined here
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+ // CHECK-NOTES: [[@LINE+3]]:1: note: label defined here
noop();
jump_to_me:;
jump_backwards:;
noop();
goto jump_backwards;
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
- // CHECK-MESSAGES: [[@LINE-4]]:1: note: label defined here
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+ // CHECK-NOTES: [[@LINE-4]]:1: note: label defined here
goto jump_in_line;
;
jump_in_line:;
- // CHECK-MESSAGES: [[@LINE-3]]:3: warning: avoid using 'goto' for flow control
- // CHECK-MESSAGES: [[@LINE-2]]:1: note: label defined here
+ // CHECK-NOTES: [[@LINE-3]]:3: warning: avoid using 'goto' for flow control
+ // CHECK-NOTES: [[@LINE-2]]:1: note: label defined here
// Test the GNU extension https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
some_label:;
@@ -132,8 +132,8 @@
for (int j = 0; j < 10; ++j) {
if (i * j > 80)
goto before_the_loop;
- // CHECK-MESSAGES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow control
- // CHECK-MESSAGES: [[@LINE-8]]:1: note: label defined here
+ // CHECK-NOTES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow control
+ // CHECK-NOTES: [[@LINE-8]]:1: note: label defined here
}
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r343374 - Attempt to fix a -Wdocumentation-html warning. NFCI.
Author: rksimon Date: Sat Sep 29 06:30:43 2018 New Revision: 343374 URL: http://llvm.org/viewvc/llvm-project?rev=343374=rev Log: Attempt to fix a -Wdocumentation-html warning. NFCI. Modified: cfe/trunk/include/clang/Lex/CodeCompletionHandler.h Modified: cfe/trunk/include/clang/Lex/CodeCompletionHandler.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/CodeCompletionHandler.h?rev=343374=343373=343374=diff == --- cfe/trunk/include/clang/Lex/CodeCompletionHandler.h (original) +++ cfe/trunk/include/clang/Lex/CodeCompletionHandler.h Sat Sep 29 06:30:43 2018 @@ -64,7 +64,7 @@ public: /// Callback invoked when performing code completion inside the filename /// part of an #include directive. (Also #import, #include_next, etc). - /// \p Dir is the directory relative to the include path, e.g. "a" for http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52684: [clang-tidy] NFC refactor lexer-utils slightly to be easier to use
JonasToth updated this revision to Diff 167592.
JonasToth added a comment.
- fix missing file
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52684
Files:
clang-tidy/bugprone/ArgumentCommentCheck.cpp
clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tidy/utils/FixItHintUtils.cpp
clang-tidy/utils/LexerUtils.cpp
clang-tidy/utils/LexerUtils.h
Index: clang-tidy/utils/LexerUtils.h
===
--- clang-tidy/utils/LexerUtils.h
+++ clang-tidy/utils/LexerUtils.h
@@ -19,8 +19,8 @@
namespace lexer {
/// Returns previous token or ``tok::unknown`` if not found.
-Token getPreviousToken(const ASTContext , SourceLocation Location,
- bool SkipComments = true);
+Token getPreviousToken(SourceLocation Location, const SourceManager ,
+ const LangOptions , bool SkipComments = true);
} // namespace lexer
} // namespace utils
Index: clang-tidy/utils/LexerUtils.cpp
===
--- clang-tidy/utils/LexerUtils.cpp
+++ clang-tidy/utils/LexerUtils.cpp
@@ -14,19 +14,15 @@
namespace utils {
namespace lexer {
-Token getPreviousToken(const ASTContext , SourceLocation Location,
- bool SkipComments) {
- const auto = Context.getSourceManager();
+Token getPreviousToken(SourceLocation Location, const SourceManager ,
+ const LangOptions , bool SkipComments) {
Token Token;
Token.setKind(tok::unknown);
Location = Location.getLocWithOffset(-1);
- auto StartOfFile =
- SourceManager.getLocForStartOfFile(SourceManager.getFileID(Location));
+ auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
while (Location != StartOfFile) {
-Location = Lexer::GetBeginningOfToken(Location, SourceManager,
- Context.getLangOpts());
-if (!Lexer::getRawToken(Location, Token, SourceManager,
-Context.getLangOpts()) &&
+Location = Lexer::GetBeginningOfToken(Location, SM, LangOpts);
+if (!Lexer::getRawToken(Location, Token, SM, LangOpts) &&
(!SkipComments || !Token.is(tok::comment))) {
break;
}
Index: clang-tidy/utils/FixItHintUtils.cpp
===
--- clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tidy/utils/FixItHintUtils.cpp
@@ -18,7 +18,8 @@
FixItHint changeVarDeclToReference(const VarDecl , ASTContext ) {
SourceLocation AmpLocation = Var.getLocation();
- auto Token = utils::lexer::getPreviousToken(Context, AmpLocation);
+ auto Token = utils::lexer::getPreviousToken(
+ AmpLocation, Context.getSourceManager(), Context.getLangOpts());
if (!Token.is(tok::unknown))
AmpLocation = Lexer::getLocForEndOfToken(Token.getLocation(), 0,
Context.getSourceManager(),
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -120,12 +120,14 @@
switch (Placement) {
case InitializerPlacement::New:
Location = utils::lexer::getPreviousToken(
- Context, Constructor.getBody()->getBeginLoc())
+ Constructor.getBody()->getBeginLoc(),
+ Context.getSourceManager(), Context.getLangOpts())
.getLocation();
break;
case InitializerPlacement::Before:
Location = utils::lexer::getPreviousToken(
- Context, Where->getSourceRange().getBegin())
+ Where->getSourceRange().getBegin(),
+ Context.getSourceManager(), Context.getLangOpts())
.getLocation();
break;
case InitializerPlacement::After:
Index: clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
===
--- clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
+++ clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
@@ -40,7 +40,8 @@
return;
ASTContext = *Result.Context;
- auto Token = utils::lexer::getPreviousToken(Ctxt, LocStart);
+ auto Token = utils::lexer::getPreviousToken(LocStart, Ctxt.getSourceManager(),
+ Ctxt.getLangOpts());
auto = *Result.SourceManager;
unsigned SemicolonLine = SM.getSpellingLineNumber(LocStart);
Index: clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -91,8 +91,9 @@
getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) {
std::vector> Comments;
while (Loc.isValid()) {
-
[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check
JonasToth added a comment. Anything new here? Is there a LLVM foundation lawyer or something like that we can ask? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D36836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'
JonasToth updated this revision to Diff 167591.
JonasToth added a comment.
- remove changes from lexerutils refactoring in other checks
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51949
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IsolateDeclarationCheck.cpp
clang-tidy/readability/IsolateDeclarationCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/utils/FixItHintUtils.cpp
clang-tidy/utils/LexerUtils.cpp
clang-tidy/utils/LexerUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-isolate-declaration.rst
test/clang-tidy/readability-isolate-declaration-cxx17.cpp
test/clang-tidy/readability-isolate-declaration-fixing.cpp
test/clang-tidy/readability-isolate-declaration.c
test/clang-tidy/readability-isolate-declaration.cpp
Index: test/clang-tidy/readability-isolate-declaration.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-declaration.cpp
@@ -0,0 +1,412 @@
+// RUN: %check_clang_tidy %s readability-isolate-declaration %t
+
+void f() {
+ int i;
+}
+
+void f2() {
+ int i, j, *k, lala = 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int i;
+ // CHECK-FIXES: {{^ }}int j;
+ // CHECK-FIXES: {{^ }}int *k;
+ // CHECK-FIXES: {{^ }}int lala = 42;
+
+ int normal, weird = /* comment */ 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int normal;
+ // CHECK-FIXES: {{^ }}int weird = /* comment */ 42;
+
+ int /* here is a comment */ v1,
+ // another comment
+ v2 = 42 // Ok, more comments
+ ;
+ // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int /* here is a comment */ v1;
+ // CHECK-FIXES: {{^ }}int /* here is a comment */ // another comment
+ // CHECK-FIXES: {{^ }}v2 = 42 // Ok, more comments
+ // CHECK-FIXES: {{^ }};
+
+ auto int1 = 42, int2 = 0, int3 = 43;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: auto int1 = 42;
+ // CHECK-FIXES: {{^ }}auto int2 = 0;
+ // CHECK-FIXES: {{^ }}auto int3 = 43;
+
+ decltype(auto) ptr1 = , ptr2 =
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: decltype(auto) ptr1 =
+ // CHECK-FIXES: {{^ }}decltype(auto) ptr2 =
+
+ decltype(k) ptr3 = , ptr4 =
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: decltype(k) ptr3 =
+ // CHECK-FIXES: {{^ }}decltype(k) ptr4 =
+}
+
+void f3() {
+ int i, *pointer1;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int i;
+ // CHECK-FIXES: {{^ }}int *pointer1;
+ //
+ int *pointer2 = nullptr, *pointer3 =
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int *pointer2 = nullptr;
+ // CHECK-FIXES: {{^ }}int *pointer3 =
+
+ int *(i_ptr) = nullptr, *((i_ptr2));
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int *(i_ptr) = nullptr;
+ // CHECK-FIXES: {{^ }}int *((i_ptr2));
+
+ float(*f_ptr)[42], (((f_value))) = 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (*f_ptr)[42];
+ // CHECK-FIXES: {{^ }}float (((f_value))) = 42;
+
+ float(((*f_ptr2)))[42], ((*f_ptr3)), f_value2 = 42.f;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (((*f_ptr2)))[42];
+ // CHECK-FIXES: {{^ }}float ((*f_ptr3));
+ // CHECK-FIXES: {{^ }}float f_value2 = 42.f;
+
+ float(((*f_ptr4)))[42], *f_ptr5, ((f_value3));
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (((*f_ptr4)))[42];
+ // CHECK-FIXES: {{^ }}float *f_ptr5;
+ // CHECK-FIXES: {{^ }}float ((f_value3));
+
+ void(((*f2))(int)), (*g2)(int, float);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: void (((*f2))(int));
+ // CHECK-FIXES: {{^ }}void (*g2)(int, float);
+
+ float(*(*(*f_ptr6)))[42], (*f_ptr7);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (*(*(*f_ptr6)))[42];
+ // CHECK-FIXES: {{^ }}float (*f_ptr7);
+}
+
+void f4() {
+ double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /* */, l = 2.;
+ //
[PATCH] D52684: [clang-tidy] NFC refactor lexer-utils slightly to be easier to use
JonasToth created this revision.
JonasToth added reviewers: alexfh, aaron.ballman, hokein.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai.
JonasToth added a project: clang-tools-extra.
This patch is a small refactoring necessary for
'readability-isolate-declaration' and does not introduce functional changes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52684
Files:
clang-tidy/bugprone/ArgumentCommentCheck.cpp
clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tidy/utils/LexerUtils.cpp
clang-tidy/utils/LexerUtils.h
Index: clang-tidy/utils/LexerUtils.h
===
--- clang-tidy/utils/LexerUtils.h
+++ clang-tidy/utils/LexerUtils.h
@@ -19,8 +19,8 @@
namespace lexer {
/// Returns previous token or ``tok::unknown`` if not found.
-Token getPreviousToken(const ASTContext , SourceLocation Location,
- bool SkipComments = true);
+Token getPreviousToken(SourceLocation Location, const SourceManager ,
+ const LangOptions , bool SkipComments = true);
} // namespace lexer
} // namespace utils
Index: clang-tidy/utils/LexerUtils.cpp
===
--- clang-tidy/utils/LexerUtils.cpp
+++ clang-tidy/utils/LexerUtils.cpp
@@ -14,19 +14,15 @@
namespace utils {
namespace lexer {
-Token getPreviousToken(const ASTContext , SourceLocation Location,
- bool SkipComments) {
- const auto = Context.getSourceManager();
+Token getPreviousToken(SourceLocation Location, const SourceManager ,
+ const LangOptions , bool SkipComments) {
Token Token;
Token.setKind(tok::unknown);
Location = Location.getLocWithOffset(-1);
- auto StartOfFile =
- SourceManager.getLocForStartOfFile(SourceManager.getFileID(Location));
+ auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
while (Location != StartOfFile) {
-Location = Lexer::GetBeginningOfToken(Location, SourceManager,
- Context.getLangOpts());
-if (!Lexer::getRawToken(Location, Token, SourceManager,
-Context.getLangOpts()) &&
+Location = Lexer::GetBeginningOfToken(Location, SM, LangOpts);
+if (!Lexer::getRawToken(Location, Token, SM, LangOpts) &&
(!SkipComments || !Token.is(tok::comment))) {
break;
}
Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -120,12 +120,14 @@
switch (Placement) {
case InitializerPlacement::New:
Location = utils::lexer::getPreviousToken(
- Context, Constructor.getBody()->getBeginLoc())
+ Constructor.getBody()->getBeginLoc(),
+ Context.getSourceManager(), Context.getLangOpts())
.getLocation();
break;
case InitializerPlacement::Before:
Location = utils::lexer::getPreviousToken(
- Context, Where->getSourceRange().getBegin())
+ Where->getSourceRange().getBegin(),
+ Context.getSourceManager(), Context.getLangOpts())
.getLocation();
break;
case InitializerPlacement::After:
Index: clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
===
--- clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
+++ clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
@@ -40,7 +40,8 @@
return;
ASTContext = *Result.Context;
- auto Token = utils::lexer::getPreviousToken(Ctxt, LocStart);
+ auto Token = utils::lexer::getPreviousToken(LocStart,
Ctxt.getSourceManager(),
+ Ctxt.getLangOpts());
auto = *Result.SourceManager;
unsigned SemicolonLine = SM.getSpellingLineNumber(LocStart);
Index: clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -91,8 +91,9 @@
getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) {
std::vector> Comments;
while (Loc.isValid()) {
-clang::Token Tok =
-utils::lexer::getPreviousToken(*Ctx, Loc, /*SkipComments=*/false);
+clang::Token Tok = utils::lexer::getPreviousToken(
+Loc, Ctx->getSourceManager(), Ctx->getLangOpts(),
+/*SkipComments=*/false);
if (Tok.isNot(tok::comment))
break;
Loc = Tok.getLocation();
Index: clang-tidy/utils/LexerUtils.h
===
--- clang-tidy/utils/LexerUtils.h
+++ clang-tidy/utils/LexerUtils.h
@@
[PATCH] D52434: [OpenMP] Make default distribute schedule for NVPTX target regions in SPMD mode achieve coalescing
Hahnfeld added a comment. In https://reviews.llvm.org/D52434#1249399, @gtbercea wrote: > In https://reviews.llvm.org/D52434#1249186, @Hahnfeld wrote: > > > In https://reviews.llvm.org/D52434#1249102, @gtbercea wrote: > > > > > You report a slow down which I am not able to reproduce actually. Do you > > > use any additional clauses not present in your previous post? > > > > > > No, only `dist_schedule(static)` which is faster. Tested on a `Tesla P100` > > with today's trunk version: > > > > | `#pragma omp target teams distribute parallel for` (new defaults) > > | 190 - 250 GB/s | > > | adding clauses for old defaults: `schedule(static) dist_schedule(static)` > > | 30 - 50 GB/s | > > | same directive with only `dist_schedule(static)` added (fewer registers) > > | 320 - 400 GB/s | > > | > > > Which loop size you're using ? What runtime does nvprof report for these > kernels? Sorry, forgot to mention: I'm using the original STREAM code with 80,000,000 `double` elements in each vector. Output from `nvprof`: Type Time(%) Time Calls Avg Min Max Name GPU activities: 70.05% 676.71ms 9 75.191ms 1.3760us 248.09ms [CUDA memcpy DtoH] 7.67% 74.102ms10 7.4102ms 7.3948ms 7.4220ms __omp_offloading_34_b871a7d5_main_l307 7.63% 73.679ms10 7.3679ms 7.3457ms 7.3811ms __omp_offloading_34_b871a7d5_main_l301 6.78% 65.516ms10 6.5516ms 6.5382ms 6.5763ms __omp_offloading_34_b871a7d5_main_l295 6.77% 65.399ms10 6.5399ms 6.5319ms 6.5495ms __omp_offloading_34_b871a7d5_main_l289 0.68% 6.6106ms 1 6.6106ms 6.6106ms 6.6106ms __omp_offloading_34_b871a7d5_main_l264 0.41% 3.9659ms 1 3.9659ms 3.9659ms 3.9659ms __omp_offloading_34_b871a7d5_main_l245 0.00% 1.1200us 1 1.1200us 1.1200us 1.1200us [CUDA memcpy HtoD] API calls: 51.12% 678.90ms 9 75.434ms 24.859us 248.70ms cuMemcpyDtoH 22.40% 297.51ms42 7.0835ms 4.0042ms 7.6802ms cuCtxSynchronize 20.31% 269.72ms 1 269.72ms 269.72ms 269.72ms cuCtxCreate 5.32% 70.631ms 1 70.631ms 70.631ms 70.631ms cuCtxDestroy 0.46% 6.1607ms 1 6.1607ms 6.1607ms 6.1607ms cuModuleLoadDataEx 0.28% 3.7628ms 1 3.7628ms 3.7628ms 3.7628ms cuModuleUnload 0.10% 1.2977ms42 30.898us 13.930us 60.092us cuLaunchKernel 0.00% 56.142us42 1.3360us 677ns 2.0930us cuFuncGetAttribute 0.00% 43.957us46 955ns 454ns 1.7670us cuCtxSetCurrent 0.00% 15.179us 1 15.179us 15.179us 15.179us cuMemcpyHtoD 0.00% 7.2780us10 727ns 358ns 1.4760us cuModuleGetGlobal 0.00% 6.9910us 2 3.4950us 2.2660us 4.7250us cuDeviceGetPCIBusId 0.00% 5.7500us 6 958ns 333ns 3.5270us cuModuleGetFunction 0.00% 3.7530us 9 417ns 184ns 1.0850us cuDeviceGetAttribute 0.00% 2.6790us 3 893ns 370ns 1.9300us cuDeviceGetCount 0.00% 2.0090us 3 669ns 484ns 767ns cuDeviceGet The memcpy comes from a `target update` to verify the results on the host. It's not included in the measurement itself, so STREAM only evaluates the kernel execution time: FunctionBest Rate MB/s Avg time Min time Max time Copy: 190819.6 0.006781 0.006708 0.006841 Scale: 189065.7 0.006800 0.006770 0.006831 Add: 253831.7 0.007616 0.007564 0.007646 Triad: 253432.3 0.007668 0.007576 0.007737 Repository: rC Clang https://reviews.llvm.org/D52434 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'
JonasToth updated this revision to Diff 167589.
JonasToth added a comment.
- fix spurious changes
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51949
Files:
clang-tidy/bugprone/ArgumentCommentCheck.cpp
clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IsolateDeclarationCheck.cpp
clang-tidy/readability/IsolateDeclarationCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/utils/FixItHintUtils.cpp
clang-tidy/utils/LexerUtils.cpp
clang-tidy/utils/LexerUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-isolate-declaration.rst
test/clang-tidy/readability-isolate-declaration-cxx17.cpp
test/clang-tidy/readability-isolate-declaration-fixing.cpp
test/clang-tidy/readability-isolate-declaration.c
test/clang-tidy/readability-isolate-declaration.cpp
Index: test/clang-tidy/readability-isolate-declaration.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-declaration.cpp
@@ -0,0 +1,412 @@
+// RUN: %check_clang_tidy %s readability-isolate-declaration %t
+
+void f() {
+ int i;
+}
+
+void f2() {
+ int i, j, *k, lala = 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int i;
+ // CHECK-FIXES: {{^ }}int j;
+ // CHECK-FIXES: {{^ }}int *k;
+ // CHECK-FIXES: {{^ }}int lala = 42;
+
+ int normal, weird = /* comment */ 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int normal;
+ // CHECK-FIXES: {{^ }}int weird = /* comment */ 42;
+
+ int /* here is a comment */ v1,
+ // another comment
+ v2 = 42 // Ok, more comments
+ ;
+ // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int /* here is a comment */ v1;
+ // CHECK-FIXES: {{^ }}int /* here is a comment */ // another comment
+ // CHECK-FIXES: {{^ }}v2 = 42 // Ok, more comments
+ // CHECK-FIXES: {{^ }};
+
+ auto int1 = 42, int2 = 0, int3 = 43;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: auto int1 = 42;
+ // CHECK-FIXES: {{^ }}auto int2 = 0;
+ // CHECK-FIXES: {{^ }}auto int3 = 43;
+
+ decltype(auto) ptr1 = , ptr2 =
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: decltype(auto) ptr1 =
+ // CHECK-FIXES: {{^ }}decltype(auto) ptr2 =
+
+ decltype(k) ptr3 = , ptr4 =
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: decltype(k) ptr3 =
+ // CHECK-FIXES: {{^ }}decltype(k) ptr4 =
+}
+
+void f3() {
+ int i, *pointer1;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int i;
+ // CHECK-FIXES: {{^ }}int *pointer1;
+ //
+ int *pointer2 = nullptr, *pointer3 =
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int *pointer2 = nullptr;
+ // CHECK-FIXES: {{^ }}int *pointer3 =
+
+ int *(i_ptr) = nullptr, *((i_ptr2));
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int *(i_ptr) = nullptr;
+ // CHECK-FIXES: {{^ }}int *((i_ptr2));
+
+ float(*f_ptr)[42], (((f_value))) = 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (*f_ptr)[42];
+ // CHECK-FIXES: {{^ }}float (((f_value))) = 42;
+
+ float(((*f_ptr2)))[42], ((*f_ptr3)), f_value2 = 42.f;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (((*f_ptr2)))[42];
+ // CHECK-FIXES: {{^ }}float ((*f_ptr3));
+ // CHECK-FIXES: {{^ }}float f_value2 = 42.f;
+
+ float(((*f_ptr4)))[42], *f_ptr5, ((f_value3));
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (((*f_ptr4)))[42];
+ // CHECK-FIXES: {{^ }}float *f_ptr5;
+ // CHECK-FIXES: {{^ }}float ((f_value3));
+
+ void(((*f2))(int)), (*g2)(int, float);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: void (((*f2))(int));
+ // CHECK-FIXES: {{^ }}void (*g2)(int, float);
+
+ float(*(*(*f_ptr6)))[42], (*f_ptr7);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (*(*(*f_ptr6)))[42];
+ // CHECK-FIXES: {{^
[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'
JonasToth updated this revision to Diff 167588.
JonasToth marked 2 inline comments as done.
JonasToth added a comment.
- address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51949
Files:
clang-tidy/bugprone/ArgumentCommentCheck.cpp
clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IsolateDeclarationCheck.cpp
clang-tidy/readability/IsolateDeclarationCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/utils/FixItHintUtils.cpp
clang-tidy/utils/LexerUtils.cpp
clang-tidy/utils/LexerUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-isolate-declaration.rst
test/clang-tidy/readability-isolate-declaration-cxx17.cpp
test/clang-tidy/readability-isolate-declaration-fixing.cpp
test/clang-tidy/readability-isolate-declaration.c
test/clang-tidy/readability-isolate-declaration.cpp
Index: test/clang-tidy/readability-isolate-declaration.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-declaration.cpp
@@ -0,0 +1,412 @@
+// RUN: %check_clang_tidy %s readability-isolate-declaration %t
+
+void f() {
+ int i;
+}
+
+void f2() {
+ int i, j, *k, lala = 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int i;
+ // CHECK-FIXES: {{^ }}int j;
+ // CHECK-FIXES: {{^ }}int *k;
+ // CHECK-FIXES: {{^ }}int lala = 42;
+
+ int normal, weird = /* comment */ 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int normal;
+ // CHECK-FIXES: {{^ }}int weird = /* comment */ 42;
+
+ int /* here is a comment */ v1,
+ // another comment
+ v2 = 42 // Ok, more comments
+ ;
+ // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int /* here is a comment */ v1;
+ // CHECK-FIXES: {{^ }}int /* here is a comment */ // another comment
+ // CHECK-FIXES: {{^ }}v2 = 42 // Ok, more comments
+ // CHECK-FIXES: {{^ }};
+
+ auto int1 = 42, int2 = 0, int3 = 43;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: auto int1 = 42;
+ // CHECK-FIXES: {{^ }}auto int2 = 0;
+ // CHECK-FIXES: {{^ }}auto int3 = 43;
+
+ decltype(auto) ptr1 = , ptr2 =
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: decltype(auto) ptr1 =
+ // CHECK-FIXES: {{^ }}decltype(auto) ptr2 =
+
+ decltype(k) ptr3 = , ptr4 =
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: decltype(k) ptr3 =
+ // CHECK-FIXES: {{^ }}decltype(k) ptr4 =
+}
+
+void f3() {
+ int i, *pointer1;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int i;
+ // CHECK-FIXES: {{^ }}int *pointer1;
+ //
+ int *pointer2 = nullptr, *pointer3 =
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int *pointer2 = nullptr;
+ // CHECK-FIXES: {{^ }}int *pointer3 =
+
+ int *(i_ptr) = nullptr, *((i_ptr2));
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: int *(i_ptr) = nullptr;
+ // CHECK-FIXES: {{^ }}int *((i_ptr2));
+
+ float(*f_ptr)[42], (((f_value))) = 42;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (*f_ptr)[42];
+ // CHECK-FIXES: {{^ }}float (((f_value))) = 42;
+
+ float(((*f_ptr2)))[42], ((*f_ptr3)), f_value2 = 42.f;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (((*f_ptr2)))[42];
+ // CHECK-FIXES: {{^ }}float ((*f_ptr3));
+ // CHECK-FIXES: {{^ }}float f_value2 = 42.f;
+
+ float(((*f_ptr4)))[42], *f_ptr5, ((f_value3));
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float (((*f_ptr4)))[42];
+ // CHECK-FIXES: {{^ }}float *f_ptr5;
+ // CHECK-FIXES: {{^ }}float ((f_value3));
+
+ void(((*f2))(int)), (*g2)(int, float);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: void (((*f2))(int));
+ // CHECK-FIXES: {{^ }}void (*g2)(int, float);
+
+ float(*(*(*f_ptr6)))[42], (*f_ptr7);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+ // CHECK-FIXES: float
[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'
JonasToth marked 15 inline comments as done.
JonasToth added inline comments.
Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:52
+ const LangOptions ) {
+ assert(Indirections >= 0 && "Indirections must be non-negative");
+ if (Indirections == 0)
aaron.ballman wrote:
> This assertion suggests that `Indirections` should be `unsigned` and that you
> perform the assertion before doing a decrement to ensure it isn't trying to
> decrement past 0.
Because the decrement is post-fix it will decrement past 0 on the breaking
condition.
Having `unsigned` will result in a wrap (is defined, but still slightly
non-obvious).
I could make a `do - while`, then the condition can be `--Indirections != 0`. I
would just like to follow the CPPCG that say 'don't use unsigned unless you
need modulo arithmetic'.
Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:230
+ for (const auto : Ranges) {
+auto CharRange = Lexer::getAsCharRange(
+CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()), SM,
aaron.ballman wrote:
> Use of `auto`?
I can use `CharSourceRange` too. kbobryev wanted `auto` :)
Comment at: clang-tidy/utils/LexerUtils.cpp:85
+llvm::Optional Tok = Lexer::findNextToken(Loc, SM, LangOpts);
+assert(Tok && "Could not retrieve next token");
+
aaron.ballman wrote:
> This seems like a bad assertion -- it's an optional for a reason, isn't it?
In the context of the check it is expected that this range is valid and all
tokens can be found. I adjusted the naming of the function to better reflect
the purpose.
Comment at: clang-tidy/utils/LexerUtils.h:48
+Token T;
+bool FailedRetrievingToken = Lexer::getRawToken(L, T, SM, LangOpts);
+
aaron.ballman wrote:
> No real need for this local.
I found it unexpected, that failure is signaled with `true`, so for readability
I added this longer tmp variable. Are there other ways to signal `true ==
failure`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
JonasToth added a comment. Please upload the patch with full context (i belive `diff -c 999`) Comment at: lib/Format/TokenAnnotator.cpp:3054 + return true; +else if (!Style.BinPackArguments) + return true; please no `else` after return. You can safely use ``` if (Left) return true; if (!Style...) return true; ``` Repository: rC Clang https://reviews.llvm.org/D52676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52665: [X86] Add more of the icc unaligned load/store to/from 128 bit vector intrinsics
RKSimon added a comment. Just to confirm - does icc provide the si64 intrinsics on 32-bit builds? https://reviews.llvm.org/D52665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
JonasToth added a comment.
Thanks for working on this!
Related as well:
http://www.codingstandard.com/rule/4-2-1-ensure-that-the-u-suffix-is-applied-to-a-literal-used-in-a-context-requiring-an-unsigned-integral-expression/
I think its wort a alias is hicpp as well
Please add tests that use user-defined literals and ensure there are no
collision and that they are not diagnosed. Some examples:
https://en.cppreference.com/w/cpp/language/user_literal
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:26
+struct IntegerLiteral {
+ using ClangType = clang::IntegerLiteral;
+ static constexpr llvm::StringLiteral Name = llvm::StringLiteral("integer");
maybe `ClangType` is not a good name, how about just `type` to be consistent
with e.g. std::vector convention.
The use-case in your template makes it clear, that we are talking about
`LiteralType`s.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:33
+};
+constexpr llvm::StringLiteral IntegerLiteral::Name;
+constexpr llvm::StringLiteral IntegerLiteral::SkipFirst;
why are these declarations necessary?
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:42
+ // What should be skipped before looking for the Suffixes?
+ // Hexadecimal floating-point literals: skip until exponent first.
+ static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("pP");
The second line of the comment is slightly confusing, please make a full
sentence out of it.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:52
+AST_MATCHER(clang::IntegerLiteral, intHasSuffix) {
+ const auto *T = dyn_cast(Node.getType().getTypePtr());
+ if (!T)
as it hit me in my check: what about `(1)ul`? Is this syntactically correct and
should be diagnosed too? (Please add tests if so).
In this case it should be `Note.getType().IgnoreParens().getTypePtr())`.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:53
+ const auto *T = dyn_cast(Node.getType().getTypePtr());
+ if (!T)
+return false;
Maybe the if could init `T`? It would require a second `return false;` if i am
not mistaken, but looks more regular to me. No strong opinion from my side.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:69
+AST_MATCHER(clang::FloatingLiteral, fpHasSuffix) {
+ const auto *T = dyn_cast(Node.getType().getTypePtr());
+ if (!T)
Same comment as above
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:85
+template
+llvm::Optional
+UppercaseLiteralSuffixCheck::shouldReplaceLiteralSuffix(
These types get really long. Is it possible to put `NewSuffix` into the
anonymous namespace as well?
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:90
+
+ NewSuffix S;
+
GIven that this variable is used mutliple times in a longer function, i feel it
shuold get a longer, more descriptive name
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:95
+ // Get the whole Integer Literal from the source buffer.
+ const StringRef LiteralSourceText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(S.Range), SM, getLangOpts());
Please check if the source text could be retrieved, with a final `bool`
parameter, thats in/out and at least `assert` on that.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:106
+Skip = LiteralSourceText.find_first_of(LiteralType::SkipFirst);
+if (Skip == StringRef::npos) // We could be in non-hexadecimal fp literal.
+ Skip = 0;
please use `flloating-point` instead of `fp` (same in other places)
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:129
+ if (S.OldSuffix == S.NewSuffix)
+return llvm::None; // The suffix was already fully uppercase.
+
Could this function return a `Optional`? That would include the
range and the relacement-text. I feel that is would simplify the code,
especially the amount of state that one has to keep track of.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:135
+void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ stmt(integerLiteral(intHasSuffix())).bind(IntegerLiteral::Name), this);
I think you can merge this matcher to
`stmt(eachOf(integerLiteral(intHasSuffix()).bind(),
floatLiteral(fpHasSuffix()).bind()))`
`eachOf` because we want to match all, and not short-circuit.
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:152
+ // Ignore literals that aren't fully written in the source code.
+ if
r343372 - [clang][www] Fix typo. NFC
Author: kristina Date: Sat Sep 29 02:45:21 2018 New Revision: 343372 URL: http://llvm.org/viewvc/llvm-project?rev=343372=rev Log: [clang][www] Fix typo. NFC Fix a one letter typo in diagnostics.html. (Wanted to try it with arcanist). Patch by king6cong Differential Revision: https://reviews.llvm.org/D52511 Modified: cfe/trunk/www/diagnostics.html Modified: cfe/trunk/www/diagnostics.html URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/www/diagnostics.html?rev=343372=343371=343372=diff == --- cfe/trunk/www/diagnostics.html (original) +++ cfe/trunk/www/diagnostics.html Sat Sep 29 02:45:21 2018 @@ -244,7 +244,7 @@ Default: template diff with type elision t.cc:4:5: note: candidate function not viable: no known conversion for 1st argument; vector map - [...], + [...], [float != double] -fdiagnostics-show-template-tree -fno-elide-type: template tree printing with no elision @@ -252,7 +252,7 @@ Default: template diff with type elision t.cc:4:5: note: candidate function not viable: no known conversion for 1st argument; vector map - int, + int, [float != double] @@ -292,7 +292,7 @@ implements the "wwopen" class of APIs):< In practice, we've found that Clang's treatment of macros is actually more useful in multiply nested -macros that in simple ones. +macros than in simple ones. Quality of Implementation and Attention to Detail ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
steveire marked 2 inline comments as done. steveire added a comment. @aaron.ballman Happy to add a note to the docs, but your request leaves me wondering where? AFAIK, the fact that `clang-tidy` wasn't built at all when using `CLANG_ENABLE_STATIC_ANALYZER` is not documented, so I can't just update that. Also AFAIK, other build switches don't have such exhaustive documentation that they mention things like this. Is this so important to document that it needs to be done here? I don't think it is that important. I think it's just a bug that clang-tidy is entirely excluded from the build when not using `CLANG_ENABLE_STATIC_ANALYZER`. This commit fixes that bug, so it would be great to get a LGTM from someone/anyone so I can commit it.. This patch definitely does not need as much discussion as it is getting. It's just enabling the build of clang-tidy when not building the static analyzer. Comment at: clang-tidy/CMakeLists.txt:29 +if(CLANG_ENABLE_STATIC_ANALYZER) +target_link_libraries(clangTidy PRIVATE + clangStaticAnalyzerCore aaron.ballman wrote: > Indentation looks off here, same below. Yes, I used the same indentation (none) as the previous code. I can indent this as you wish. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
steveire updated this revision to Diff 167584.
steveire added a comment.
Whitespace etc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
Files:
CMakeLists.txt
clang-tidy/CMakeLists.txt
clang-tidy/ClangTidy.cpp
clang-tidy/plugin/CMakeLists.txt
clang-tidy/plugin/ClangTidyPlugin.cpp
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
test/CMakeLists.txt
test/clang-tidy/enable-alpha-checks.cpp
test/clang-tidy/mpi-buffer-deref.cpp
test/clang-tidy/mpi-type-mismatch.cpp
test/clang-tidy/nolint.cpp
test/clang-tidy/read_file_config.cpp
test/clang-tidy/static-analyzer-config.cpp
test/clang-tidy/static-analyzer.cpp
test/clang-tidy/temporaries.cpp
test/lit.cfg
unittests/CMakeLists.txt
Index: unittests/CMakeLists.txt
===
--- unittests/CMakeLists.txt
+++ unittests/CMakeLists.txt
@@ -18,8 +18,6 @@
add_subdirectory(clang-apply-replacements)
add_subdirectory(clang-move)
add_subdirectory(clang-query)
-if(CLANG_ENABLE_STATIC_ANALYZER)
- add_subdirectory(clang-tidy)
-endif()
+add_subdirectory(clang-tidy)
add_subdirectory(clangd)
add_subdirectory(include-fixer)
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -119,24 +119,22 @@
if config.clang_staticanalyzer:
config.available_features.add('static-analyzer')
-check_clang_tidy = os.path.join(
-config.test_source_root, "clang-tidy", "check_clang_tidy.py")
-config.substitutions.append(
-('%check_clang_tidy',
- '%s %s' % (config.python_executable, check_clang_tidy)) )
-clang_tidy_diff = os.path.join(
-config.test_source_root, "..", "clang-tidy", "tool", "clang-tidy-diff.py")
-config.substitutions.append(
-('%clang_tidy_diff',
- '%s %s' % (config.python_executable, clang_tidy_diff)) )
-run_clang_tidy = os.path.join(
-config.test_source_root, "..", "clang-tidy", "tool", "run-clang-tidy.py")
-config.substitutions.append(
-('%run_clang_tidy',
- '%s %s' % (config.python_executable, run_clang_tidy)) )
-else:
-# exclude the clang-tidy test directory
-config.excludes.append('clang-tidy')
+
+check_clang_tidy = os.path.join(
+config.test_source_root, "clang-tidy", "check_clang_tidy.py")
+config.substitutions.append(
+('%check_clang_tidy',
+ '%s %s' % (config.python_executable, check_clang_tidy)) )
+clang_tidy_diff = os.path.join(
+config.test_source_root, "..", "clang-tidy", "tool", "clang-tidy-diff.py")
+config.substitutions.append(
+('%clang_tidy_diff',
+ '%s %s' % (config.python_executable, clang_tidy_diff)) )
+run_clang_tidy = os.path.join(
+config.test_source_root, "..", "clang-tidy", "tool", "run-clang-tidy.py")
+config.substitutions.append(
+('%run_clang_tidy',
+ '%s %s' % (config.python_executable, run_clang_tidy)) )
clangd_benchmarks_dir = os.path.join(os.path.dirname(config.clang_tools_dir),
"tools", "clang", "tools", "extra",
Index: test/clang-tidy/temporaries.cpp
===
--- test/clang-tidy/temporaries.cpp
+++ test/clang-tidy/temporaries.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy -checks='-*,clang-analyzer-core.NullDereference' %s -- | FileCheck %s
struct NoReturnDtor {
Index: test/clang-tidy/static-analyzer.cpp
===
--- test/clang-tidy/static-analyzer.cpp
+++ test/clang-tidy/static-analyzer.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy %s -checks='-*,clang-analyzer-*' -- | FileCheck %s
extern void *malloc(unsigned long);
extern void free(void *);
Index: test/clang-tidy/static-analyzer-config.cpp
===
--- test/clang-tidy/static-analyzer-config.cpp
+++ test/clang-tidy/static-analyzer-config.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: clang-tidy %s -checks='-*,clang-analyzer-unix.Malloc' -config='{CheckOptions: [{ key: "clang-analyzer-unix.Malloc:Optimistic", value: true}]}' -- | FileCheck %s
typedef __typeof(sizeof(int)) size_t;
void *malloc(size_t);
Index: test/clang-tidy/read_file_config.cpp
===
--- test/clang-tidy/read_file_config.cpp
+++ test/clang-tidy/read_file_config.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: mkdir -p %T/read-file-config/
// RUN: cp %s %T/read-file-config/test.cpp
// RUN: echo 'Checks: "-*,modernize-use-nullptr"' > %T/read-file-config/.clang-tidy
Index: test/clang-tidy/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: static-analyzer
// RUN: %check_clang_tidy %s
