[PATCH] D38679: [libunwind] Support dwarf unwinding on i386 windows

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: src/AddressSpace.hpp:521
 unw_word_t *offset) {
-#ifndef _LIBUNWIND_IS_BAREMETAL
+#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32)
   Dl_info dyldInfo;

Would it work to implement the win32 side of this via `SymFromAddr`?



Comment at: src/UnwindRegistersRestore.S:29
 #  +   +
+#if !defined(_WIN32)
   movl   4(%esp), %eax

Please invert the condition, and swap the if/else on this. That will make it 
more straightforward to adjust this for other platforms later.


https://reviews.llvm.org/D38679



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

[PATCH] D38679: [libunwind] Support dwarf unwinding on i386 windows

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: src/AddressSpace.hpp:521
 unw_word_t *offset) {
-#ifndef _LIBUNWIND_IS_BAREMETAL
+#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32)
   Dl_info dyldInfo;

mstorsjo wrote:
> mstorsjo wrote:
> > jroelofs wrote:
> > > Would it work to implement the win32 side of this via `SymFromAddr`?
> > Hmm, I guess that would work.
> ... actually, I'm not sure how useful it is - it requires initializing the 
> symbol handler with `SymInitialize` and point to a path to find the symbols. 
> Plus that the symbol handler is single threaded and any calls to that would 
> need to be guarded with a global mutex. So I think I'd defer that for now at 
> least.
alright, fine with me.


https://reviews.llvm.org/D38679



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

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs resigned from this revision.
jroelofs added a comment.

I'm not sure I'm the right person to review this.


Repository:
  rL LLVM

https://reviews.llvm.org/D38599



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

[PATCH] D38711: typos in documentation?

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs accepted this revision.
jroelofs added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D38711



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

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

That reminds me... this does need a testcase or two.


Repository:
  rL LLVM

https://reviews.llvm.org/D38599



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

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D38599#893985, @danalbert wrote:

> In https://reviews.llvm.org/D38599#893903, @jroelofs wrote:
>
> > That reminds me... this does need a testcase or two.
>
>
> Oh, also, any test I add is going to fail, since the case I'm trying to 
> account for here is not the default behavior.


That's what an `available_feature` + `// REQUIRES:` is for.


https://reviews.llvm.org/D38599



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

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D38599#893990, @jroelofs wrote:

> In https://reviews.llvm.org/D38599#893985, @danalbert wrote:
>
> > In https://reviews.llvm.org/D38599#893903, @jroelofs wrote:
> >
> > > That reminds me... this does need a testcase or two.
> >
> >
> > Oh, also, any test I add is going to fail, since the case I'm trying to 
> > account for here is not the default behavior.
>
>
> That's what an `available_feature` + `// REQUIRES:` is for.


Which should be set (along with -D_LIBCXX_DYNAMIC_FALLBACK (possibly renamed to 
_LIBCXXABI_DYNAMIC_FALLBACK)) via a CMake flag... We shouldn't just be defining 
macros like this in CMAKE_CXX_FLAGS.


https://reviews.llvm.org/D38599



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

[PATCH] D38679: [libunwind] Support dwarf unwinding on i386 windows

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs accepted this revision.
jroelofs added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D38679



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

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

Needs a docs entry for the new flag (in libcxx's BuildingLibcxx.rst). Other 
than that, all the stuff I've asked you to add LGTM. I'd still appreciate 
@EricWF/@mclow's opinion on the meat of the functional change part of this 
though... I don't know all the implications relaxing the search here.


https://reviews.llvm.org/D38599



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

[PATCH] D51354: Fix the -print-multi-directory flag to print the selected multilib.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs accepted this revision.
jroelofs added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for fixing this!


Repository:
  rC Clang

https://reviews.llvm.org/D51354



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

[PATCH] D43159: Modernize: Use nullptr more.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

Is it worth adding `-Werror=zero-as-null-pointer-constant` to the build?


Repository:
  rCXX libc++

https://reviews.llvm.org/D43159



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

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 100131.
jroelofs added a comment.

implement feedback


https://reviews.llvm.org/D33259

Files:
  cmake/caches/BaremetalARM.cmake
  lib/Driver/CMakeLists.txt
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/BareMetal.cpp
  lib/Driver/ToolChains/BareMetal.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Linux.cpp
  test/Driver/Inputs/baremetal_arm/include/c++/5.0.0/.keep
  test/Driver/Inputs/baremetal_arm/include/c++/6.0.0/.keep
  test/Driver/Inputs/baremetal_arm/include/c++/v1/.keep
  test/Driver/baremetal.cpp

Index: test/Driver/baremetal.cpp
===
--- /dev/null
+++ test/Driver/baremetal.cpp
@@ -0,0 +1,77 @@
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: -T semihosted.lds \
+// RUN: -L some/directory/user/asked/for \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-C %s
+// CHECK-V6M-C: "[[PREFIX_DIR:.*]]/bin/clang" "-cc1" "-triple" "thumbv6m-none--eabi"
+// CHECK-V6M-C-SAME: "-resource-dir" "[[PREFIX_DIR]]/lib/clang/[[VERSION:[^"]*]]"
+// CHECK-V6M-C-SAME: "-isysroot" "[[SYSROOT:[^"]*]]"
+// CHECK-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]/include/c++/v1"
+// CHECk-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]/include"
+// CHECK-V6M-C-SAME: "-x" "c++" "{{.*}}baremetal.cpp"
+// CHECK-V6M-C-NEXT: "[[PREFIX_DIR:.*]]/bin/ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-C-SAME: "-L[[PREFIX_DIR]]/lib/clang/[[VERSION]]/lib/baremetal"
+// CHECK-V6M-C-SAME: "-T" "semihosted.lds" "-Lsome/directory/user/asked/for"
+// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: -nostdlibinc -nobuiltininc \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBINC %s
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: -nostdinc \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBINC %s
+// CHECK-V6M-LIBINC-NOT: "-internal-isystem"
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-DEFAULTCXX %s
+// CHECK-V6M-DEFAULTCXX: "[[PREFIX_DIR:.*]]/bin/ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-DEFAULTCXX-SAME: "-L[[PREFIX_DIR]]/lib/clang/{{.*}}/lib/baremetal"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-DEFAULTCXX-SAME: "-o" "{{.*}}.o"
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN: -stdlib=libc++ \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBCXX %s
+// CHECK-V6M-LIBCXX-NOT: "-internal-isystem" "{{[^"]+}}/include/c++/{{[^v].*}}"
+// CHECK-V6M-LIBCXX: "-internal-isystem" "{{[^"]+}}/include/c++/v1"
+// CHECK-V6M-LIBCXX: "[[PREFIX_DIR:.*]]/bin/ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-LIBCXX-SAME: "-L[[PREFIX_DIR]]/lib/clang/{{.*}}/lib/baremetal"
+// CHECK-V6M-LIBCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
+// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-LIBCXX-SAME: "-o" "{{.*}}.o"
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN: -stdlib=libstdc++ \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBSTDCXX %s
+// CHECK-V6M-LIBSTDCXX-NOT: "-internal-isystem" "{{[^"]+}}/include/c++/v1"
+// CHECK-V6M-LIBSTDCXX: "-internal-isystem" "{{[^"]+}}/include/c++/6.0.0"
+// CHECK-V6M-LIBSTDCXX: "[[PREFIX_DIR:.*]]/bin/ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-LIBSTDCXX-SAME: "-L[[PREFIX_DIR]]/lib/clang/{{.*}}/lib/baremetal"
+// CHECK-V6M-LIBSTDCXX-SAME: "-lstdc++" "-lsupc++" "-lunwind"
+// CHECK-V6M-LIBSTDCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-LIBSTDCXX-SAME: "-o" "{{.*}}.o"
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN: -nodefaultlibs \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-NDL %s
+// CHECK-V6M-NDL: "[[PREFIX_DIR:.*]]/bin/ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-NDL-SAME: "-L[[PREFIX_DIR]]/lib/clang/{{.*}}/lib/baremetal" "-o" "{{.*}}.o"
+
+// RUN: %clangxx -target arm-none-eabi -v 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-THREAD-MODEL
+// CHECK-THREAD-MODEL: Thread model: single
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Lin

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked 2 inline comments as done.
jroelofs added inline comments.



Comment at: lib/Driver/ToolChains/BareMetal.cpp:110
+  SmallString<128> Dir(SysRoot);
+  llvm::sys::path::append(Dir, "include", "c++", "v1");
+  return Dir.str();

compnerd wrote:
> Is this layout consistent between libc++ and libstdc++?
Damn, no it's not.



Comment at: lib/Driver/ToolChains/BareMetal.cpp:130-133
+if (Value == "libc++")
+  return ToolChain::CST_Libcxx;
+else if (Value == "libstdc++")
+  return ToolChain::CST_Libstdcxx;

compnerd wrote:
> Use `StringSwitch`?
`StringSwitch` isn't great when you want to error out of the default case. That 
being said, this whole function is unnecessary: I can just defer to the base 
class' implementation, which does almost the same thing.



Comment at: lib/Driver/ToolChains/BareMetal.h:39
+  bool isPICDefaultForced() const override { return false; }
+  bool SupportsProfiling() const override { return true; }
+  bool SupportsObjCGC() const override { return false; }

compnerd wrote:
> Is the profiler support in compiler-rt sufficiently standalone to build it 
> for baremetal?
IIRC, there was a test case that wanted this on for one of the arm-none-eabi 
triples. This is when my patches were against 4.0. After rebasing to trunk, I 
think that particular test is gone (split up into separate ones, I guess?).

I'll leave it off for now until someone confirms it is actually supported.



Comment at: lib/Driver/ToolChains/Linux.cpp:379
-/*static*/
-Generic_GCC::GCCVersion Linux::GCCVersion::Parse(StringRef VersionText) {
-  const GCCVersion BadVersion = {VersionText.str(), -1, -1, -1, "", "", ""};

Noticed that this was in the wrong place, and also, surprisingly, that it was 
incorrectly qualified.


https://reviews.llvm.org/D33259



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

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 100177.
jroelofs marked an inline comment as done.
jroelofs added a comment.

Fix a cmake warning:

  Platform/baremetal to use this system, please send your config file to 
cm...@www.cmake.org so it can be added to cmake
  Your CMakeCache.txt file was copied to CopyOfCMakeCache.txt. Please send that 
file to cm...@www.cmake.org.

Generic does what we want already, it just has a name that's not useful.


https://reviews.llvm.org/D33259

Files:
  cmake/caches/BaremetalARM.cmake
  lib/Driver/CMakeLists.txt
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/BareMetal.cpp
  lib/Driver/ToolChains/BareMetal.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Linux.cpp
  test/Driver/Inputs/baremetal_arm/include/c++/5.0.0/.keep
  test/Driver/Inputs/baremetal_arm/include/c++/6.0.0/.keep
  test/Driver/Inputs/baremetal_arm/include/c++/v1/.keep
  test/Driver/baremetal.cpp

Index: test/Driver/baremetal.cpp
===
--- /dev/null
+++ test/Driver/baremetal.cpp
@@ -0,0 +1,77 @@
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: -T semihosted.lds \
+// RUN: -L some/directory/user/asked/for \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-C %s
+// CHECK-V6M-C: "[[PREFIX_DIR:.*]]/bin/clang" "-cc1" "-triple" "thumbv6m-none--eabi"
+// CHECK-V6M-C-SAME: "-resource-dir" "[[PREFIX_DIR]]/lib/clang/[[VERSION:[^"]*]]"
+// CHECK-V6M-C-SAME: "-isysroot" "[[SYSROOT:[^"]*]]"
+// CHECK-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]/include/c++/v1"
+// CHECk-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]/include"
+// CHECK-V6M-C-SAME: "-x" "c++" "{{.*}}baremetal.cpp"
+// CHECK-V6M-C-NEXT: "[[PREFIX_DIR:.*]]/bin/ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-C-SAME: "-L[[PREFIX_DIR]]/lib/clang/[[VERSION]]/lib/baremetal"
+// CHECK-V6M-C-SAME: "-T" "semihosted.lds" "-Lsome/directory/user/asked/for"
+// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: -nostdlibinc -nobuiltininc \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBINC %s
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: -nostdinc \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBINC %s
+// CHECK-V6M-LIBINC-NOT: "-internal-isystem"
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-DEFAULTCXX %s
+// CHECK-V6M-DEFAULTCXX: "[[PREFIX_DIR:.*]]/bin/ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-DEFAULTCXX-SAME: "-L[[PREFIX_DIR]]/lib/clang/{{.*}}/lib/baremetal"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-DEFAULTCXX-SAME: "-o" "{{.*}}.o"
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN: -stdlib=libc++ \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBCXX %s
+// CHECK-V6M-LIBCXX-NOT: "-internal-isystem" "{{[^"]+}}/include/c++/{{[^v].*}}"
+// CHECK-V6M-LIBCXX: "-internal-isystem" "{{[^"]+}}/include/c++/v1"
+// CHECK-V6M-LIBCXX: "[[PREFIX_DIR:.*]]/bin/ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-LIBCXX-SAME: "-L[[PREFIX_DIR]]/lib/clang/{{.*}}/lib/baremetal"
+// CHECK-V6M-LIBCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
+// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-LIBCXX-SAME: "-o" "{{.*}}.o"
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN: -stdlib=libstdc++ \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBSTDCXX %s
+// CHECK-V6M-LIBSTDCXX-NOT: "-internal-isystem" "{{[^"]+}}/include/c++/v1"
+// CHECK-V6M-LIBSTDCXX: "-internal-isystem" "{{[^"]+}}/include/c++/6.0.0"
+// CHECK-V6M-LIBSTDCXX: "[[PREFIX_DIR:.*]]/bin/ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-LIBSTDCXX-SAME: "-L[[PREFIX_DIR]]/lib/clang/{{.*}}/lib/baremetal"
+// CHECK-V6M-LIBSTDCXX-SAME: "-lstdc++" "-lsupc++" "-lunwind"
+// CHECK-V6M-LIBSTDCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-LIBSTDCXX-SAME: "-o" "{{.*}}.o"
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target armv6m-none-eabi \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN: -nodefaultlibs \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-NDL %s
+// CHECK-V6M-NDL: "[[PREFIX_DIR:.*]]/bin/ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-NDL-SAME: "-L[[PR

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs closed this revision.
jroelofs added a comment.

r303873


https://reviews.llvm.org/D33259



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

[PATCH] D33561: [CMake] Add Android toolchain CMake cache files.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

what about the builtins?


https://reviews.llvm.org/D33561



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

[PATCH] D33877: [test] Fix baremetal test to allow any -resource-dir

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs accepted this revision.
jroelofs added a comment.
This revision is now accepted and ready to land.

LGTM... thank you!


Repository:
  rL LLVM

https://reviews.llvm.org/D33877



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

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D33259#772184, @mgorny wrote:

> This causes a test failure with non-standard CLANG_RESOURCE_DIR:


https://reviews.llvm.org/D33877

(thanks for the patch)


https://reviews.llvm.org/D33259



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

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

I'd much rather provide no implementation than one that lies. Broken builds are 
much safer than problems at runtime.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41316



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

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

I say that because there are contexts where it is **absolutely critical** that 
https://xkcd.com/221/ not be the implementation of an RNG.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41316



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

[PATCH] D41733: [Driver] Suggest correctly spelled driver options

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: lib/Driver/Driver.cpp:191
 if (A->getOption().hasFlag(options::Unsupported)) {
-  Diag(diag::err_drv_unsupported_opt) << A->getAsString(Args);
-  ContainsError |= Diags.getDiagnosticLevel(diag::err_drv_unsupported_opt,
-SourceLocation()) >
+  unsigned DiagID;
+  auto ArgString = A->getAsString(Args);

No need for this variable.



Comment at: lib/Driver/Driver.cpp:218
   for (const Arg *A : Args.filtered(options::OPT_UNKNOWN)) {
-auto ID = IsCLMode() ? diag::warn_drv_unknown_argument_clang_cl
- : diag::err_drv_unknown_argument;
-
-Diags.Report(ID) << A->getAsString(Args);
-ContainsError |= Diags.getDiagnosticLevel(ID, SourceLocation()) >
+unsigned DiagID;
+auto ArgString = A->getAsString(Args);

Likewise.


Repository:
  rC Clang

https://reviews.llvm.org/D41733



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

[PATCH] D41733: [Driver] Suggest correctly spelled driver options

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: lib/Driver/Driver.cpp:191
 if (A->getOption().hasFlag(options::Unsupported)) {
-  Diag(diag::err_drv_unsupported_opt) << A->getAsString(Args);
-  ContainsError |= Diags.getDiagnosticLevel(diag::err_drv_unsupported_opt,
-SourceLocation()) >
+  unsigned DiagID;
+  auto ArgString = A->getAsString(Args);

modocache wrote:
> jroelofs wrote:
> > No need for this variable.
> There's a call below to `Diags.getDiagnosticsLevel`, which takes this ID as 
> an argument. Are you suggesting I leave that line alone, and have it use the 
> diagnostic level of `err_drv_unsupported_opt`, even if 
> `err_drv_unsupported_opt_with_suggestion` is actually emitted here? That 
> seems reasonable to me, but just want to make sure that's what you had in 
> mind.
Oh, I didn't see that use... never mind.


Repository:
  rC Clang

https://reviews.llvm.org/D41733



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

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

> Should we go with current patch?

You'll need a bunch of `UNSUPPORTED: has-no-random-device` or something like 
it, but other than that, I'm happy with the patch as-is.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41316



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

[PATCH] D61040: [Fuchsia] Support multilib for -fsanitize=address and -fno-exceptions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:122
+
+/// \p Flag must be a flag accepted by the driver with its leading '-' removed,
+// otherwise '-print-multi-lib' will not emit them correctly.

Can we enforce this precondition with an assert? The '-'-prefix-not-there part 
is easy, but what about the "it's a driver flag" part?



Comment at: clang/test/Driver/fuchsia.cpp:53
+
+// RUN: %clang %s -### --target=x86_64-fuchsia -fno-exceptions \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \

What about a test for `-fsanitize=address` + `-fno-exceptions`?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61040/new/

https://reviews.llvm.org/D61040



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

[PATCH] D52153: scan-build: Add support of the option --exclude like in scan-build-py

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs accepted this revision.
jroelofs added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D52153



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

[PATCH] D52151: Also manages clang-X as tool for scan-build

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

`$triple-clang`, also


Repository:
  rC Clang

https://reviews.llvm.org/D52151



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

[PATCH] D52530: [analyzer] scan-build: if --status-bugs is passed, don't forget about the exit status of the actual build

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs accepted this revision.
jroelofs added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D52530



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

[PATCH] D37493: Fix ARM bare metal driver to support atomics

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

Sure. I'll commit it for you once this build/test cycle is finished.


https://reviews.llvm.org/D37493



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

[PATCH] D37493: Fix ARM bare metal driver to support atomics

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs closed this revision.
jroelofs added a comment.

r312651


https://reviews.llvm.org/D37493



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

[PATCH] D37496: Fix validation of the -mthread-model flag in the Clang driver

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs closed this revision.
jroelofs added a comment.

r312748


https://reviews.llvm.org/D37496



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

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

I'm not sure it's better than writing the if/elseif/elseif/elseif out 
explicitly :/


Repository:
  rL LLVM

https://reviews.llvm.org/D37629



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

[PATCH] D40775: [libcxx] Add underscores to win32 locale headers.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: src/support/win32/locale_win32.cpp:90
+__libcpp_locale_guard __current(__loc);
 va_list ap;
+va_start( ap, __format );

__ap



Comment at: src/support/win32/locale_win32.cpp:99
 {
 va_list ap;
+va_start( ap, __format );

__ap



Comment at: src/support/win32/locale_win32.cpp:101
+va_start( ap, __format );
+int result = vasprintf_l( __ret, __loc, __format, ap );
 va_end(ap);

s.result.__res.


Repository:
  rCXX libc++

https://reviews.llvm.org/D40775



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

[PATCH] D40775: [libcxx] Add underscores to win32 locale headers.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

looks fine to me, but this is the sort of thing that @EricWF usually wants the 
final say on.


https://reviews.llvm.org/D40775



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

[PATCH] D40816: [libunwind] Use the correct variable name for target triple in lit

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

There's a: `set(TARGET_TRIPLE ...)` in each of these runtimes' CMakeLists.txt...


Repository:
  rL LLVM

https://reviews.llvm.org/D40816



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

[PATCH] D40816: [libunwind] Use the correct variable name for target triple in lit

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

(i.e. you should delete that, since this is the last dependency on it AFAICT)


Repository:
  rL LLVM

https://reviews.llvm.org/D40816



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

[PATCH] D40816: [libunwind] Use the correct variable name for target triple in lit

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

I think just remove it from them.


Repository:
  rL LLVM

https://reviews.llvm.org/D40816



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

[PATCH] D28820: Warn when calling a non interrupt function from an interrupt on ARM

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D28820#944365, @efriedma wrote:

> > What is the best way to modify the code for this compiler change ?
>
> Currently, the "interrupt" attribute only has an effect on functions, not 
> function pointers, so your code won't work the way you want.  It's a bug that 
> we don't emit a warning for this.


https://bugs.llvm.org/show_bug.cgi?id=35527

> Currently, this warning doesn't have its own warning flag, instead being 
> lumped under -Wextra.  This is also a bug.

https://bugs.llvm.org/show_bug.cgi?id=35528

> We don't emit the warning if your code is compiled for a target which doesn't 
> support floating-point (-mfpu=none or -msoft-float); see 
> https://reviews.llvm.org/D32918. But otherwise, if you're sure your code is 
> actually correct, you can turn off the warning with -Wno-extra or something 
> like that.  (The whole thing is kind of awkward because the implementation of 
> the interrupt attribute in clang is buggy: the frontend lies to the backend 
> about the calling convention, so the backend can't save/restore VFP registers 
> correctly.)

https://i.imgur.com/BFRoEUO.gif

In https://reviews.llvm.org/D28820#944706, @kc.austin2017 wrote:

> so is there any plan to make the "interrupt" attribute not just support on 
> functions?


I'll fix these. Give me a few weeks though.


https://reviews.llvm.org/D28820



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

[PATCH] D35542: libcxxabi: Suppress LLVM_ENABLE_MODULES

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

Does the unwinder need this too?


Repository:
  rL LLVM

https://reviews.llvm.org/D35542



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

[PATCH] D74812: [Sema] Teach -Warm-interrupt-safety about func ptrs

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs created this revision.
jroelofs added reviewers: efriedma, weimingz, EricWF.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a project: clang.

Fixes:

  https://bugs.llvm.org/show_bug.cgi?id=35527
  https://bugs.llvm.org/show_bug.cgi?id=35528


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74812

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/arm-interrupt-attr.c

Index: clang/test/Sema/arm-interrupt-attr.c
===
--- clang/test/Sema/arm-interrupt-attr.c
+++ clang/test/Sema/arm-interrupt-attr.c
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 %s -triple arm-apple-darwin  -target-feature +vfp2 -verify -fsyntax-only
-// RUN: %clang_cc1 %s -triple thumb-apple-darwin  -target-feature +vfp3 -verify -fsyntax-only
-// RUN: %clang_cc1 %s -triple armeb-none-eabi  -target-feature +vfp4 -verify -fsyntax-only
-// RUN: %clang_cc1 %s -triple thumbeb-none-eabi  -target-feature +neon -verify -fsyntax-only
-// RUN: %clang_cc1 %s -triple thumbeb-none-eabi -target-feature +neon -target-feature +soft-float -DSOFT -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Warm-interrupt-safety -triple arm-apple-darwin  -target-feature +vfp2 -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Warm-interrupt-safety -triple thumb-apple-darwin  -target-feature +vfp3 -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Warm-interrupt-safety -triple armeb-none-eabi  -target-feature +vfp4 -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Warm-interrupt-safety -triple thumbeb-none-eabi  -target-feature +neon -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Warm-interrupt-safety -triple thumbeb-none-eabi -target-feature +neon -target-feature +soft-float -DSOFT -verify -fsyntax-only
 
 __attribute__((interrupt(IRQ))) void foo() {} // expected-error {{'interrupt' attribute requires a string}}
 __attribute__((interrupt("irq"))) void foo1() {} // expected-warning {{'interrupt' attribute argument not supported: irq}}
@@ -26,24 +26,36 @@
   callee2();
 }
 
-#ifndef SOFT
 __attribute__((interrupt("IRQ"))) void caller2() {
+#ifndef SOFT
   callee1(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
-  callee2();
-}
-
-void (*callee3)();
-__attribute__((interrupt("IRQ"))) void caller3() {
-  callee3(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
-}
 #else
-__attribute__((interrupt("IRQ"))) void caller2() {
   callee1();
+#endif
   callee2();
 }
 
 void (*callee3)();
 __attribute__((interrupt("IRQ"))) void caller3() {
+#ifndef SOFT
+  callee3(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
+#else
   callee3();
+#endif
 }
+
+void __attribute__((interrupt("IRQ"))) bugzilla35527() {
+  typedef void __attribute__((interrupt("IRQ"))) (*interrupt_callback_t)(int i, void *ctx);
+  interrupt_callback_t interrupt_callee;
+  interrupt_callee(42, 0);
+  (interrupt_callee)(42, 0);
+  (*interrupt_callee)(42, 0);
+
+  typedef void (*callback_t)(int i, void *ctx);
+  callback_t callee;
+#ifndef SOFT
+  callee(37, 0); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
+#else
+  callee(37, 0);
 #endif
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5925,12 +5925,20 @@
   // so there's some risk when calling out to non-interrupt handler functions
   // that the callee might not preserve them. This is easy to diagnose here,
   // but can be very challenging to debug.
-  if (auto *Caller = getCurFunctionDecl())
-if (Caller->hasAttr()) {
-  bool VFP = Context.getTargetInfo().hasFeature("vfp");
-  if (VFP && (!FDecl || !FDecl->hasAttr()))
-Diag(Fn->getExprLoc(), diag::warn_arm_interrupt_calling_convention);
-}
+  if (Context.getTargetInfo().hasFeature("vfp"))
+if (auto *Caller = getCurFunctionDecl())
+  if (Caller->hasAttr()) {
+const Decl *CalleeDecl = FDecl;
+if (const auto *UO = dyn_cast(Fn->IgnoreParens())) {
+  if (const auto *TT =
+  dyn_cast(UO->getSubExpr()->getType()))
+CalleeDecl = TT->getDecl();
+} else if (const auto *TT = Fn->getType()->getAs()) {
+  CalleeDecl = TT->getDecl();
+}
+if (!CalleeDecl || !CalleeDecl->hasAttr())
+  Diag(Fn->getExprLoc(), diag::warn_arm_interrupt_calling_convention);
+  }
 
   // Promote the function operand.
   // We special-case function promotion here because we only allow promoting
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+

[PATCH] D74812: [Sema] Teach -Warm-interrupt-safety about func ptrs

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked an inline comment as done.
jroelofs added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:5931
+  if (Caller->hasAttr()) {
+const Decl *CalleeDecl = FDecl;
+if (const auto *UO = dyn_cast(Fn->IgnoreParens())) {

This feels very fragile, and I know is missing at least one case. Is there a 
better way to reliably get at the typedef's attributes from here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74812/new/

https://reviews.llvm.org/D74812



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

[PATCH] D28820: Warn when calling a non interrupt function from an interrupt on ARM

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.
Herald added a subscriber: kristof.beyls.

In D28820#945118 , @kc.austin2017 wrote:

> In D28820#944865 , @jroelofs wrote:
>
> > https://bugs.llvm.org/show_bug.cgi?id=35527
> >
> > https://bugs.llvm.org/show_bug.cgi?id=35528
> >
> > I'll fix these. Give me a few weeks though.
>
>
> Thank you very much for recording the bug.


patch to fix these: https://reviews.llvm.org/D74812


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D28820/new/

https://reviews.llvm.org/D28820



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

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 246604.
jroelofs added a comment.

Implement review feedback:

- Re-purpose `HeaderFileExtensionsUtils.h` to support headers *and* sources.
- Surround file extension in diagnostic with `''`s.

I have these as two separate patches locally, but I don't know how to represent 
that in phabricator, so I've uploaded the diff across both. If this gets 
approved, I intend to push them as separate commits.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #import of file with '.c' extension
+#import "c.c"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension
+#  include  
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
-#include "../utils/HeaderFileExtensionsUtils.h"
+#include "../utils/FileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
@@ -29,8 +29,8 @@
   : ClangTidyCheck(Name, Context),
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
-utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
- HeaderFileExtensions, ',');
+utils::parseFileExtensions(RawStringHeaderFileExtensions,
+   HeaderFileExtensions, ',');
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
@@ -54,7 +54,7 @@
 
 private:
   std::string RawStringHeaderFileExtensions;
-  utils::HeaderFileExtensionsSet HeaderFileExtensions;
+  utils::FileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -273,13 +273,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef Fi

[PATCH] D75489: [clang-tidy] Generalize HeaderFileExtensions.{h, cpp}. NFC

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs created this revision.
jroelofs added reviewers: njames93, aaron.ballman.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.
jroelofs added a child revision: D74669: [clang-tidy] New check: 
bugprone-suspicious-include.
jroelofs marked an inline comment as done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:14
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"

this belongs in the other patch.


Splitting this out from https://reviews.llvm.org/D74669


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75489

Files:
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h

Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
-#include "../utils/HeaderFileExtensionsUtils.h"
+#include "../utils/FileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
@@ -29,8 +29,8 @@
   : ClangTidyCheck(Name, Context),
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
-utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
- HeaderFileExtensions, ',');
+utils::parseFileExtensions(RawStringHeaderFileExtensions,
+   HeaderFileExtensions, ',');
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
@@ -54,7 +54,7 @@
 
 private:
   std::string RawStringHeaderFileExtensions;
-  utils::HeaderFileExtensionsSet HeaderFileExtensions;
+  utils::FileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -273,13 +273,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
+  return utils::isFileExtension(FileName, HeaderFileExtensions);
 }
 
 bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
 
 bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
-  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
+  return utils::isFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
+++ /dev/null
@@ -1,70 +0,0 @@
-//===--- HeaderFileExtensionsUtils.cpp - clang-tidy--*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "HeaderFileExtensionsUtils.h"
-#include "clang/Basic/CharInfo.h"
-#include "llvm/Support/Path.h"
-
-namespace clang {
-namespace tidy {
-namespace utils {
-
-bool isExpansionLocInHeaderFile(
-SourceLocation Loc, const SourceManager &SM,
-const HeaderFileExtensionsSet &HeaderFileExtensions) {
-  SourceLocation ExpansionLoc = SM.getExpansionLoc(Loc);
-  return isHeaderFileExtension(SM.getFilename(ExpansionLoc),
-   HeaderFileExtensions);
-}
-
-bool isPresumedLocInHeaderFile(
-SourceLocation Loc, SourceMa

[PATCH] D75489: [clang-tidy] Generalize HeaderFileExtensions.{h, cpp}. NFC

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked an inline comment as done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:14
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"

this belongs in the other patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75489/new/

https://reviews.llvm.org/D75489



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

[PATCH] D75489: [clang-tidy] Generalize HeaderFileExtensions.{h, cpp}. NFC

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 247762.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75489/new/

https://reviews.llvm.org/D75489

Files:
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h

Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
-#include "../utils/HeaderFileExtensionsUtils.h"
+#include "../utils/FileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
@@ -29,8 +29,8 @@
   : ClangTidyCheck(Name, Context),
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
-utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
- HeaderFileExtensions, ',');
+utils::parseFileExtensions(RawStringHeaderFileExtensions,
+   HeaderFileExtensions, ',');
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
@@ -54,7 +54,7 @@
 
 private:
   std::string RawStringHeaderFileExtensions;
-  utils::HeaderFileExtensionsSet HeaderFileExtensions;
+  utils::FileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -273,13 +273,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
+  return utils::isFileExtension(FileName, HeaderFileExtensions);
 }
 
 bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
 
 bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
-  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
+  return utils::isFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
+++ /dev/null
@@ -1,70 +0,0 @@
-//===--- HeaderFileExtensionsUtils.cpp - clang-tidy--*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "HeaderFileExtensionsUtils.h"
-#include "clang/Basic/CharInfo.h"
-#include "llvm/Support/Path.h"
-
-namespace clang {
-namespace tidy {
-namespace utils {
-
-bool isExpansionLocInHeaderFile(
-SourceLocation Loc, const SourceManager &SM,
-const HeaderFileExtensionsSet &HeaderFileExtensions) {
-  SourceLocation ExpansionLoc = SM.getExpansionLoc(Loc);
-  return isHeaderFileExtension(SM.getFilename(ExpansionLoc),
-   HeaderFileExtensions);
-}
-
-bool isPresumedLocInHeaderFile(
-SourceLocation Loc, SourceManager &SM,
-const HeaderFileExtensionsSet &HeaderFileExtensions) {
-  PresumedLoc PresumedLocation = SM.getPresumedLoc(Loc);
-  return isHeaderFileExtension(PresumedLocation.getFilename(),
-   HeaderFileExtensions);
-}
-
-bool isSpellingLocInHeaderFile(
-SourceLocation Loc, SourceManager &SM,
-const HeaderFileExtensionsSet &HeaderFileExtensions) {
-  SourceLocation SpellingLoc = SM.getSpellingLoc(Loc);
-  return isHeaderFileExtension(SM.getFilename(SpellingLoc),
- 

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 247764.
jroelofs added a comment.

implement review feedback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #import of file with '.c' extension
+#import "c.c"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension
+#  include  
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -18,11 +18,12 @@
 
 /// Finds and fixes header guards.
 /// The check supports these options:
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
@@ -30,7 +31,7 @@
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
 utils::parseFileExtensions(RawStringHeaderFileExtensions,
-   HeaderFileExtensions, ',');
+   HeaderFileExtensions, ';');
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -11,6 +11,7 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -34,12 +35,23 @@
 
 /// Returns recommended default value for the lis

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked 2 inline comments as done.
jroelofs added a comment.

In D74669#1902107 , @njames93 wrote:

> Adding the parent revision means you don't need to have those changes in this 
> patch.


IIUC, that is what I've already done:

https://reviews.llvm.org/D74669 is `git show c4dd6f5903e -U999`
https://reviews.llvm.org/D75489 is `git show 0cda7e0b9ed -U999`

where `c4dd6f5903e` is the parent of `0cda7e0b9ed`.

The suggestion to switch over to `;`s as delimiters is what's causing this diff 
to still touch so many files.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h:24
+/// extensions of header files (The filename extensions should not contain
+/// "." prefix). "h;hh;hpp;hxx" by default.
+//

";h;hh;hpp;hxx"



Comment at: 
clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h:25
+/// extensions of header files (The filename extensions should not contain
+/// "." prefix). "h;hh;hpp;hxx" by default.
+///

";h;hh;hpp;hxx"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



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

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs created this revision.
jroelofs added reviewers: njames93, aaron.ballman.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

https://reviews.llvm.org/D74669#inline-685657


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75621

Files:
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h

Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -18,11 +18,12 @@
 
 /// Finds and fixes header guards.
 /// The check supports these options:
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
@@ -30,7 +31,7 @@
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
 utils::parseFileExtensions(RawStringHeaderFileExtensions,
-   HeaderFileExtensions, ',');
+   HeaderFileExtensions, ';');
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -34,7 +34,7 @@
 
 /// Returns recommended default value for the list of header file
 /// extensions.
-inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
 
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
===
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
+++ clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
@@ -22,11 +22,12 @@
 /// The check supports these options:
 ///   - `UseHeaderFileExtension`: Whether to use file extension to distinguish
 /// header files. True by default.
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/misc-definitions-in-headers.html
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -34,7 +34,7 @@
   RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
   "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
   if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
-  HeaderFileExtensions, ',')) {
+  

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 248203.
jroelofs added a comment.

Split out https://reviews.llvm.org/D75621


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #import of file with '.c' extension
+#import "c.c"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension
+#  include  
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -11,6 +11,7 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -36,10 +37,21 @@
 /// extensions.
 inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
 
+/// Returns recommended default value for the list of implementaiton file
+/// extensions.
+inline StringRef defaultImplementationFileExtensions() {
+  return "c;cc;cpp;cxx";
+}
+
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
  FileExtensionsSet &FileExtensions, char Delimiter);
 
+/// Decides whether a file has a header file extension.
+/// Returns the file extension, if included in the provided set.
+llvm::Optional
+getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions);
+
 /// Decides whether a file has one of the specified file extensions.
 bool isFileExtension(StringRef FileName,
  const FileExtensionsSet &FileExtensions);
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
@@ -46,13 +46,20 @@
   return true;
 }
 
-bool isFileExtension(StringRef FileName,
- const FileExtensionsSet &FileExtensions) {
+llvm::Optional
+getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions) {
   StringRef Extension = llvm::sys::path::extension(FileName);
   if (Extension.empty())
-return false;
+return llvm::None;
   // Skip "." prefix.
-  return FileExtensions.count(Extension.substr(1)) > 0;
+  if (!FileExtensions.count(Extension.substr(1)))
+return llvm::None;
+  return Extension;
+}
+
+bool isFileExtension(StringRef FileName,
+ const FileExtensionsSet &FileExtensions) {
+  return getFileExtension(FileName, FileExtensions).hasValue();
 }
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
@@ -0,0 +1,57 @@
+//===--- SuspiciousIncludeCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache Li

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 248206.
jroelofs added a comment.

Fix comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75621/new/

https://reviews.llvm.org/D75621

Files:
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h

Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -18,11 +18,12 @@
 
 /// Finds and fixes header guards.
 /// The check supports these options:
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
@@ -30,7 +31,7 @@
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
 utils::parseFileExtensions(RawStringHeaderFileExtensions,
-   HeaderFileExtensions, ',');
+   HeaderFileExtensions, ';');
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -34,7 +34,7 @@
 
 /// Returns recommended default value for the list of header file
 /// extensions.
-inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
 
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
===
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
+++ clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
@@ -22,11 +22,12 @@
 /// The check supports these options:
 ///   - `UseHeaderFileExtension`: Whether to use file extension to distinguish
 /// header files. True by default.
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/misc-definitions-in-headers.html
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -34,7 +34,7 @@
   RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
   "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
   if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
-  HeaderFileExtensions, ',')) {
+  HeaderFileExtensions, ';')) {

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 248208.
jroelofs marked an inline comment as not done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #import of file with '.c' extension
+#import "c.c"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension
+#  include  
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -11,6 +11,7 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -36,10 +37,21 @@
 /// extensions.
 inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
 
+/// Returns recommended default value for the list of implementaiton file
+/// extensions.
+inline StringRef defaultImplementationFileExtensions() {
+  return "c;cc;cpp;cxx";
+}
+
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
  FileExtensionsSet &FileExtensions, char Delimiter);
 
+/// Decides whether a file has a header file extension.
+/// Returns the file extension, if included in the provided set.
+llvm::Optional
+getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions);
+
 /// Decides whether a file has one of the specified file extensions.
 bool isFileExtension(StringRef FileName,
  const FileExtensionsSet &FileExtensions);
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
@@ -46,13 +46,20 @@
   return true;
 }
 
-bool isFileExtension(StringRef FileName,
- const FileExtensionsSet &FileExtensions) {
+llvm::Optional
+getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions) {
   StringRef Extension = llvm::sys::path::extension(FileName);
   if (Extension.empty())
-return false;
+return llvm::None;
   // Skip "." prefix.
-  return FileExtensions.count(Extension.substr(1)) > 0;
+  if (!FileExtensions.count(Extension.substr(1)))
+return llvm::None;
+  return Extension;
+}
+
+bool isFileExtension(StringRef FileName,
+ const FileExtensionsSet &FileExtensions) {
+  return getFileExtension(FileName, FileExtensions).hasValue();
 }
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
@@ -0,0 +1,57 @@
+//===--- SuspiciousIncludeCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Ex

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 248246.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// clang-format off
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #import of file with '.c' extension
+#import "c.c"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension
+#  include  
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -11,6 +11,7 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -36,6 +37,12 @@
 /// extensions.
 inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
 
+/// Returns recommended default value for the list of implementaiton file
+/// extensions.
+inline StringRef defaultImplementationFileExtensions() {
+  return "c;cc;cpp;cxx";
+}
+
 /// Returns recommended default value for the list of file extension
 /// delimiters.
 inline StringRef defaultFileExtensionDelimiters() { return ",;"; }
@@ -45,6 +52,11 @@
  FileExtensionsSet &FileExtensions,
  StringRef Delimiters);
 
+/// Decides whether a file has a header file extension.
+/// Returns the file extension, if included in the provided set.
+llvm::Optional
+getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions);
+
 /// Decides whether a file has one of the specified file extensions.
 bool isFileExtension(StringRef FileName,
  const FileExtensionsSet &FileExtensions);
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
@@ -59,13 +59,20 @@
   return true;
 }
 
-bool isFileExtension(StringRef FileName,
- const FileExtensionsSet &FileExtensions) {
+llvm::Optional
+getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions) {
   StringRef Extension = llvm::sys::path::extension(FileName);
   if (Extension.empty())
-return false;
+return llvm::None;
   // Skip "." prefix.
-  return FileExtensions.count(Extension.substr(1)) > 0;
+  if (!FileExtensions.count(Extension.substr(1)))
+return llvm::None;
+  return Extension;
+}
+
+bool isFileExtension(StringRef FileName,
+ const FileExtensionsSet &FileExtensions) {
+  return getFileExtension(FileName, FileExtensions).hasValue();
 }
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
@@ -0,0 +1,57 @@
+//===--- SuspiciousIncludeCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of th

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 248245.
jroelofs added a comment.

Preserve backwards compatibility of ',' as a delimiter (for now).

> The llvm::StringRef::split function can take multiple split characters,

AFAIU, that's for multi-character delimiters, not multiple delimiters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75621/new/

https://reviews.llvm.org/D75621

Files:
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h

Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -18,11 +18,12 @@
 
 /// Finds and fixes header guards.
 /// The check supports these options:
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
@@ -30,7 +31,8 @@
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
 utils::parseFileExtensions(RawStringHeaderFileExtensions,
-   HeaderFileExtensions, ',');
+   HeaderFileExtensions,
+   utils::defaultFileExtensionDelimiters());
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -34,11 +34,16 @@
 
 /// Returns recommended default value for the list of header file
 /// extensions.
-inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
+
+/// Returns recommended default value for the list of file extension
+/// delimiters.
+inline StringRef defaultFileExtensionDelimiters() { return ",;"; }
 
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter);
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters);
 
 /// Decides whether a file has one of the specified file extensions.
 bool isFileExtension(StringRef FileName,
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
@@ -9,6 +9,7 @@
 #include "FileExtensionsUtils.h"
 #include "clang/Basic/CharInfo.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace tidy {
@@ -33,9 +34,21 @@
 }
 
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter) {
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters) {
   SmallVector Suffixes;
-  AllFileExtensions.split(Suffixes, Delimiter);
+  for (const char Delimiter : Delimiters) {
+if (StringRef::npos != AllFileExtensions.find(Delimiter)) {
+  if (Delimiter == ',') {
+llvm::errs()
+<< "Using ',' as a file extension delimiter is deprecated. Please "
+ 

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 248357.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75621/new/

https://reviews.llvm.org/D75621

Files:
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h

Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -18,11 +18,12 @@
 
 /// Finds and fixes header guards.
 /// The check supports these options:
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
@@ -30,7 +31,8 @@
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
 utils::parseFileExtensions(RawStringHeaderFileExtensions,
-   HeaderFileExtensions, ',');
+   HeaderFileExtensions,
+   utils::defaultFileExtensionDelimiters());
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -34,11 +34,16 @@
 
 /// Returns recommended default value for the list of header file
 /// extensions.
-inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
+
+/// Returns recommended default value for the list of file extension
+/// delimiters.
+inline StringRef defaultFileExtensionDelimiters() { return ",;"; }
 
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter);
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters);
 
 /// Decides whether a file has one of the specified file extensions.
 bool isFileExtension(StringRef FileName,
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
@@ -9,6 +9,7 @@
 #include "FileExtensionsUtils.h"
 #include "clang/Basic/CharInfo.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace tidy {
@@ -33,9 +34,21 @@
 }
 
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter) {
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters) {
   SmallVector Suffixes;
-  AllFileExtensions.split(Suffixes, Delimiter);
+  for (const char Delimiter : Delimiters) {
+if (AllFileExtensions.contains(Delimiter)) {
+  if (Delimiter == ',') {
+llvm::errs()
+<< "Using ',' as a file extension delimiter is deprecated. Please "
+   "switch your configuration to use ';'.\n";
+  }
+  AllFileExtensions.split(Suffixes, Delimiter);
+  break;
+}
+  }
+
   FileExtensions.clear();
   for (StringRef Suffix : Suffixes) {
 StringRef Extension = Suffix.trim();
Index: cl

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 248755.
jroelofs added a comment.

- Don't spam the deprecation message. Move that to release notes.
- Drop unnecessary `const`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75621/new/

https://reviews.llvm.org/D75621

Files:
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -125,6 +125,11 @@
   check now detects in class initializers and constructor initializers which
   are deemed to be redundant.
 
+- Checks supporting the ``HeaderFileExtensions`` flag now support ``;`` as a
+  delimiter in addition to ``,``, with the latter being deprecated as of this
+  release. This simplifies how one specifies the options on the command line:
+  ``--config="{CheckOptions: [{ key: HeaderFileExtensions, value: h;;hpp;hxx }]}"``
+
 Renamed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -18,11 +18,12 @@
 
 /// Finds and fixes header guards.
 /// The check supports these options:
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
@@ -30,7 +31,8 @@
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
 utils::parseFileExtensions(RawStringHeaderFileExtensions,
-   HeaderFileExtensions, ',');
+   HeaderFileExtensions,
+   utils::defaultFileExtensionDelimiters());
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -34,11 +34,16 @@
 
 /// Returns recommended default value for the list of header file
 /// extensions.
-inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
+
+/// Returns recommended default value for the list of file extension
+/// delimiters.
+inline StringRef defaultFileExtensionDelimiters() { return ",;"; }
 
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter);
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters);
 
 /// Decides whether a file has one of the specified file extensions.
 bool isFileExtension(StringRef FileName,
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
@@ -9,6 +9,7 @@
 #include "FileExtensionsUtils.h"
 #include "clang/Basic/CharInfo.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 248757.
jroelofs added a comment.

- Drop dead include.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75621/new/

https://reviews.llvm.org/D75621

Files:
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -125,6 +125,11 @@
   check now detects in class initializers and constructor initializers which
   are deemed to be redundant.
 
+- Checks supporting the ``HeaderFileExtensions`` flag now support ``;`` as a
+  delimiter in addition to ``,``, with the latter being deprecated as of this
+  release. This simplifies how one specifies the options on the command line:
+  ``--config="{CheckOptions: [{ key: HeaderFileExtensions, value: h;;hpp;hxx }]}"``
+
 Renamed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -18,11 +18,12 @@
 
 /// Finds and fixes header guards.
 /// The check supports these options:
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
@@ -30,7 +31,8 @@
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
 utils::parseFileExtensions(RawStringHeaderFileExtensions,
-   HeaderFileExtensions, ',');
+   HeaderFileExtensions,
+   utils::defaultFileExtensionDelimiters());
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -34,11 +34,16 @@
 
 /// Returns recommended default value for the list of header file
 /// extensions.
-inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
+
+/// Returns recommended default value for the list of file extension
+/// delimiters.
+inline StringRef defaultFileExtensionDelimiters() { return ",;"; }
 
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter);
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters);
 
 /// Decides whether a file has one of the specified file extensions.
 bool isFileExtension(StringRef FileName,
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
@@ -33,9 +33,16 @@
 }
 
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter) {
+ FileExtensionsSet &FileExtensions,
+ Strin

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked an inline comment as done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:44-46
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter);
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters);

njames93 wrote:
> Doesn't belong in this patch, but this function should be changed in a follow 
> up to return an `Optional`.
Looking at the uses, I'm not convinced this would be better in this specific 
case.

Compare:
```
if (Optional HFE = parseFileExtensions(Extensions, 
Delimiters)) {
  HeaderFileExtensions = *HFE;
} else {
  errs() << "Invalid extensions: " << Extensions;
}
```

With the status quo:
```
if (!parseFileExtensions(Extensions, HeaderFileExtensions, Delimiters)) {
  errs() << "Invalid extensions: " << Extensions;
}
```

Optional's explicit operator bool is great for when you want to guard on the 
presence of the thing, but not so great when you want to check for it not being 
there. I feel like we'd need some new utility to go with Optional to make this 
nice:

```
if (not(HeaderFileExtensions) = parseFileExtensions(Extensions, Delimiters)) {
  errs() << "Invalid extensions: " << Extensions;
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75621/new/

https://reviews.llvm.org/D75621



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

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked an inline comment as done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:44-46
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter);
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters);

njames93 wrote:
> jroelofs wrote:
> > njames93 wrote:
> > > Doesn't belong in this patch, but this function should be changed in a 
> > > follow up to return an `Optional`.
> > Looking at the uses, I'm not convinced this would be better in this 
> > specific case.
> > 
> > Compare:
> > ```
> > if (Optional HFE = parseFileExtensions(Extensions, 
> > Delimiters)) {
> >   HeaderFileExtensions = *HFE;
> > } else {
> >   errs() << "Invalid extensions: " << Extensions;
> > }
> > ```
> > 
> > With the status quo:
> > ```
> > if (!parseFileExtensions(Extensions, HeaderFileExtensions, Delimiters)) {
> >   errs() << "Invalid extensions: " << Extensions;
> > }
> > ```
> > 
> > Optional's explicit operator bool is great for when you want to guard on 
> > the presence of the thing, but not so great when you want to check for it 
> > not being there. I feel like we'd need some new utility to go with Optional 
> > to make this nice:
> > 
> > ```
> > if (not(HeaderFileExtensions) = parseFileExtensions(Extensions, 
> > Delimiters)) {
> >   errs() << "Invalid extensions: " << Extensions;
> > }
> > ```
> To be honest it should probably be `Expected` rather than `Optional`, but 
> that still doesn't help solve the cleanliness issue.
Implementation of the `not` idea here:

https://gcc.godbolt.org/z/XZh39g


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75621/new/

https://reviews.llvm.org/D75621



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

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs closed this revision.
jroelofs added a comment.

https://github.com/llvm/llvm-project/commit/47caa69120e582bf1b795ec646f069c83b0e9456


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75621/new/

https://reviews.llvm.org/D75621



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

[PATCH] D75489: [clang-tidy] Generalize HeaderFileExtensions.{h, cpp}. NFC

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs closed this revision.
jroelofs added a comment.

https://github.com/llvm/llvm-project/commit/3486cc014b208df3897cf5656db0d0fdeae26d6b


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75489/new/

https://reviews.llvm.org/D75489



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

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 249164.
jroelofs added a comment.

Fix comment, add docs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// clang-format off
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #import of file with '.c' extension
+#import "c.c"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension
+#  include  
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - bugprone-suspicious-include
+
+bugprone-suspicious-include
+===
+
+The checker detects various cases when an include refers to what appears to be
+an implementation file, which often leads to hard-to-track-down ODR violations.
+
+Examples:
+
+.. code-block:: c++
+
+  #include "Dinosaur.hpp" // OK, .hpp files tend not to have definitions.
+  #include "Pterodactyl.h"// OK, .h files tend not to have definitions.
+  #include "Velociraptor.cpp" // Warning, filename is suspicious
+  #import "Stegosaurus.c" // Warning, fliename is suspicious
+  #include_next  // Warning, filename is suspicious
+
+Options
+---
+.. option:: HeaderFileExtensions
+
+   Default value: ";h;hh;hpp;hxx"
+   A semicolon-separated list of filename extensions of header files (The
+   filename extensions should not contain "." prefix). For extension-less
+   header files, using an empty string or leaving an empty string between ";"
+   if there are other filename extensions.
+
+.. option:: HeaderFileExtensions
+
+   Default value: "c;cc;cpp;cxx"
+   Likewise, a semicolon-separated list of filename extensions of
+   implementation files.
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -11,6 +11,7 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -36,6 +37,12 @@
 /// extensions.
 inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
 
+/// Returns recommended default value for the list of implementaiton file
+/// extensions.
+inline StringRef defaultImplementationFileExtensions() {
+  return "c;cc;cpp;cxx";
+}
+
 /// Returns recommended default value for the list of file extension
 /// delimiters.
 inline StringRef defaultFileExtensionDelimiters() { return ",;"; }
@@ -45,6 +52,11 @@
  FileExtensionsSet &FileExtensions,
  StringRef Delimiters);
 
+/// Decides whether a file has a header file extension.
+/// Returns the file extension, if included in the provided set.
+llvm::Optional
+getFileExtension(StringRef FileName, con

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked 3 inline comments as done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:9
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H

aaron.ballman wrote:
> Can drop the `HEADER` from these macros.
Missed this one before. Oops.
https://github.com/llvm/llvm-project/commit/c71ef7a85d2447903ff9240aff649a57d70b050d


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



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

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs closed this revision.
jroelofs marked 7 inline comments as done.
jroelofs added a comment.

Thanks for the review!

https://github.com/llvm/llvm-project/commit/1e0669bfe05f0f48ee88152c4a1d581f484f8d67




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp:10
+#include "a.cpp"
+
+#include "b.h"

aaron.ballman wrote:
> Can you add a test for:
> ```
> #include "i.cpp"
> ```
> to show that it suggests `i.h` (which already exists in-tree) but not `i` or 
> `i.hpp`?
happy to :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



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

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 249223.
jroelofs added a comment.

Implement review feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/i.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// clang-format off
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+// CHECK-MESSAGES: [[@LINE+2]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'i.h'?
+#include "i.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #import of file with '.c' extension
+#import "c.c"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension
+#  include  
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - bugprone-suspicious-include
+
+bugprone-suspicious-include
+===
+
+The checker detects various cases when an include refers to what appears to be
+an implementation file, which often leads to hard-to-track-down ODR violations.
+
+Examples:
+
+.. code-block:: c++
+
+  #include "Dinosaur.hpp" // OK, .hpp files tend not to have definitions.
+  #include "Pterodactyl.h"// OK, .h files tend not to have definitions.
+  #include "Velociraptor.cpp" // Warning, filename is suspicious.
+  #import "Stegosaurus.c" // Warning, fliename is suspicious.
+  #include_next  // Warning, filename is suspicious.
+
+Options
+---
+.. option:: HeaderFileExtensions
+
+   Default value: ";h;hh;hpp;hxx"
+   A semicolon-separated list of filename extensions of header files (the
+   filename extensions should not contain a "." prefix). For extension-less
+   header files, use an empty string or leave an empty string between ";"
+   if there are other filename extensions.
+
+.. option:: ImplementationFileExtensions
+
+   Default value: "c;cc;cpp;cxx"
+   Likewise, a semicolon-separated list of filename extensions of
+   implementation files.
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -11,6 +11,7 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -36,6 +37,12 @@
 /// extensions.
 inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
 
+/// Returns recommended default value for the list of implementaiton file
+/// extensions.
+inline StringRef defaultImplementationFileExtensions() {
+  return "c;cc;cpp;cxx";
+}
+
 /// Returns recommended default value for the list of file extension
 /// delimiters.
 inline StringRef defaultFileExtensionDelimiters() { return ",;"; }
@@ -45,6 +52,11 @@
 

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 249224.
jroelofs added a comment.

Add missing release notes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/i.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// clang-format off
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+// CHECK-MESSAGES: [[@LINE+2]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'i.h'?
+#include "i.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #import of file with '.c' extension
+#import "c.c"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension
+#  include  
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - bugprone-suspicious-include
+
+bugprone-suspicious-include
+===
+
+The checker detects various cases when an include refers to what appears to be
+an implementation file, which often leads to hard-to-track-down ODR violations.
+
+Examples:
+
+.. code-block:: c++
+
+  #include "Dinosaur.hpp" // OK, .hpp files tend not to have definitions.
+  #include "Pterodactyl.h"// OK, .h files tend not to have definitions.
+  #include "Velociraptor.cpp" // Warning, filename is suspicious.
+  #import "Stegosaurus.c" // Warning, fliename is suspicious.
+  #include_next  // Warning, filename is suspicious.
+
+Options
+---
+.. option:: HeaderFileExtensions
+
+   Default value: ";h;hh;hpp;hxx"
+   A semicolon-separated list of filename extensions of header files (the
+   filename extensions should not contain a "." prefix). For extension-less
+   header files, use an empty string or leave an empty string between ";"
+   if there are other filename extensions.
+
+.. option:: ImplementationFileExtensions
+
+   Default value: "c;cc;cpp;cxx"
+   Likewise, a semicolon-separated list of filename extensions of
+   implementation files.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -98,6 +98,12 @@
 
   Finds recursive functions and diagnoses them.
 
+- New :doc:`bugprone-suspicious-includei
+  ` check.
+
+  Finds includes that appear to be referring to implementation files (which
+  tends to cause ODR violations), and diagnoses them.
+
 New check aliases
 ^
 
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -11,6 +11,7 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Optional.h"
 #include "llv

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked 6 inline comments as done.
jroelofs added a comment.

https://github.com/llvm/llvm-project/commit/52bbdad7d63fd060d102b3591b433d116a982255


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



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

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 249377.
jroelofs added a comment.

Don't fire on `#import`s.

The error on Windows suggests it would normally do the "right" thing anyway:

http://45.33.8.238/win/10088/step_8.txt

  
C:\src\llvm-project\out\gn\obj\clang-tools-extra\test\clang-tidy\checkers\Output\bugprone-suspicious-include.cpp.tmp.cpp:18:2:
 error: #import of type library is an unsupported Microsoft feature 
[clang-diagnostic-error]
  #import "c.c"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/i.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// clang-format off
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+// CHECK-MESSAGES: [[@LINE+2]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'i.h'?
+#include "i.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension
+#  include  
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - bugprone-suspicious-include
+
+bugprone-suspicious-include
+===
+
+The check detects various cases when an include refers to what appears to be an
+implementation file, which often leads to hard-to-track-down ODR violations.
+
+Examples:
+
+.. code-block:: c++
+
+  #include "Dinosaur.hpp" // OK, .hpp files tend not to have definitions.
+  #include "Pterodactyl.h"// OK, .h files tend not to have definitions.
+  #include "Velociraptor.cpp" // Warning, filename is suspicious.
+  #include_next  // Warning, filename is suspicious.
+
+Options
+---
+.. option:: HeaderFileExtensions
+
+   Default value: `";h;hh;hpp;hxx"`
+   A semicolon-separated list of filename extensions of header files (the
+   filename extensions should not contain a "." prefix). For extension-less
+   header files, use an empty string or leave an empty string between ";"
+   if there are other filename extensions.
+
+.. option:: ImplementationFileExtensions
+
+   Default value: `"c;cc;cpp;cxx"`
+   Likewise, a semicolon-separated list of filename extensions of
+   implementation files.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -82,6 +82,13 @@
 
   Checks for usages of identifiers reserved for use by the implementation.
 
+- New :doc:`bugprone-suspicious-include
+  ` check.
+
+  Finds cases where an include refers to what appears to be an implementation
+  file, which often leads to hard-to-track-down ODR violations, and diagnoses
+  them.
+
 - New :doc:`cert-oop57-cpp
   ` check.
 
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

re-landed in 
https://github.com/llvm/llvm-project/commit/2c9cf9f4ddd01ae9eb47522266a6343104f9d0b5


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



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

[PATCH] D73904: [clang] stop baremetal driver to append .a to lib

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

+1, looks good with a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73904/new/

https://reviews.llvm.org/D73904



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

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs created this revision.
jroelofs added a reviewer: aaron.ballman.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai.
Herald added a project: clang.

Detects and fixes suspicious code like: `#include "foo.cpp"`.

  

Inspired by: https://twitter.com/lefticus/status/1228458240364687360?s=20


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp
  clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/misc-no-include-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-no-include-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-no-include-cpp.cpp
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy %s misc-no-include-cpp %t -- -- -isystem %S/Inputs/Headers
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: suspicious #include
+// CHECK-MESSAGES: [[@LINE+3]]:1: warning: suspicious #include
+#include "a.cpp"
+#include "b.h"
+#include 
+
+// CHECK-FIXES: #include "a.h"
+// CHECK-FIXES-NEXT: #include "b.h"
+// CHECK-FIXES-NEXT: #include 
Index: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.h
@@ -0,0 +1,39 @@
+//===--- NoIncludeCPPCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NOINCLUDECPPCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NOINCLUDECPPCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Warns on inclusion of files whose names suggest that they're implementation
+/// files, instead of headers. E.g:
+///
+/// #include "foo.cpp"  // warning
+/// #include "bar.c"// warning
+/// #include "baz.h"// no diagnostic
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-NoIncludeCPP.html
+class NoIncludeCPPCheck : public ClangTidyCheck {
+public:
+  NoIncludeCPPCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP);
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NOINCLUDECPPCHECK_H
Index: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp
@@ -0,0 +1,70 @@
+//===--- NoIncludeCPPCheck.cpp - clang-tidy ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NoIncludeCPPCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+class NoIncludeCCallbacks : public PPCallbacks {
+public:
+  explicit NoIncludeCCallbacks(ClangTidyCheck &Check,
+   const SourceManager &SM)
+  : Check(Check) {}
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+  StringRef FileName, bool IsAngled,
+  CharSourceRange FilenameRange, const FileEntry *File,
+  StringRef SearchPath, StringRef RelativePath,
+  const Module *Imported,
+  SrcMgr::CharacteristicKind FileType) override;
+
+private:
+  ClangTidyCheck &Check;
+};
+} // namespace
+
+void NoIncludeCPPCheck::registerPPCallbacks(const SourceManager &SM,
+Preprocessor *PP,
+Preprocessor *ModuleExpanderPP) {
+  PP->addPPCallbacks(::std::make_unique(*this, SM));
+}
+
+void NoIncludeCCallbacks::InclusionDirective(
+SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
+bool IsAngl

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked an inline comment as done.
jroelofs added a comment.

In D74669#1878168 , @njames93 wrote:

> I have a feeling this check should be called something along the lines of 
> bugprone-suspicous-include.


That's a much better name, I like it.




Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:62
+  Check.diag(HashLoc, "suspicious #include")
+  << FixItHint::CreateReplacement(FilenameRange,
+  ((IsAngled ? "<" : "\"") + FileName +

njames93 wrote:
> This replacement is dangerous, I have a feeling no fix-it should be provided 
> or at least do a search of the include directories to see if file you are 
> trying to include actually does exist. The correct file could be `*.hpp` like 
> what boost uses for all its header files
Yeah, perhaps the FixIt should only be added if there is a single candidate 
replacement that exists on the `-I` path.

Another option is to not add FixIts at all, and instead emit a list of `note:`s 
suggesting each of the candidates.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



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

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked 4 inline comments as done and an inline comment as not done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:62
+  Check.diag(HashLoc, "suspicious #include")
+  << FixItHint::CreateReplacement(FilenameRange,
+  ((IsAngled ? "<" : "\"") + FileName +

njames93 wrote:
> jroelofs wrote:
> > njames93 wrote:
> > > This replacement is dangerous, I have a feeling no fix-it should be 
> > > provided or at least do a search of the include directories to see if 
> > > file you are trying to include actually does exist. The correct file 
> > > could be `*.hpp` like what boost uses for all its header files
> > Yeah, perhaps the FixIt should only be added if there is a single candidate 
> > replacement that exists on the `-I` path.
> > 
> > Another option is to not add FixIts at all, and instead emit a list of 
> > `note:`s suggesting each of the candidates.
> How about the case if someone wants to (for whatever reason) include 
> something like this:
> 
> ```
> #include "example.h"
> #include "example.cpp"
> ```
> That looks intentional and a fix it shouldn't be emitted.
I'd imagine that people doing this intentionally (unity builds come to mind) 
would turn off this check. That said, is there something better this check 
could be doing to accommodate those people?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



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

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 244883.
jroelofs marked an inline comment as done.
jroelofs added a comment.

- Renamed to bugprone-suspicious-include
- Removed FixIts in favor of `note:` suggestions, iff the suggested file exists 
on the header search path.
- Removed unnecessary unused parameter casts-to-void.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers
+
+// CHECK-MESSAGES: [[@LINE+4]]:1: warning: suspicious #include of file with .cpp extension
+// CHECK-MESSAGES: [[@LINE+3]]:1: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:1: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:1: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .c extension
+#include 
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .cc extension
+#include 
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .cxx extension
+#include 
+
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
@@ -0,0 +1,39 @@
+//===--- SuspiciousIncludeCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Warns on inclusion of files whose names suggest that they're implementation
+/// files, instead of headers. E.g:
+///
+/// #include "foo.cpp"  // warning
+/// #include "bar.c"// warning
+/// #include "baz.h"// no diagnostic
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html
+class SuspiciousIncludeCheck : public ClangTidyCheck {
+public:
+  SuspiciousIncludeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
@@ -0,0 +1,74 @@
+//===--- SuspiciousIncludeCheck.cpp - clang-tidy --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SuspiciousIncludeCheck.h"
+#include "clang/AST/ASTContext.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+namespace {
+class SuspiciousIncludePPCallbacks : public PPCallbacks {
+public:
+  explicit SuspiciousIncludePPCallbacks(ClangTidyCheck &Check,
+const SourceManager &SM,
+Preprocessor *PP)
+  : Check(Check), PP(PP) {}
+
+  void InclusionDirecti

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked an inline comment as done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h:26
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html
+class SuspiciousIncludeCheck : public ClangTidyCheck {

FIXME: still need to write docs for this.



Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:50
+
+  (void)FilenameRange;
+  (void)File;

njames93 wrote:
> I have a feeling this is to suppress some unused parameter linting check. If 
> it is then it should be removed as unused parameters in an overridden 
> function shouldn't emit a warning.
> Same for below.
> 
> Side note: File a bug with whichever linter tool gave you that warning (if 
> there was one).
> 
Sorry, did this out of habit. Indeed it isn't actually needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



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

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 244884.
jroelofs added a comment.

git format-patch


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers
+
+// CHECK-MESSAGES: [[@LINE+4]]:1: warning: suspicious #include of file with .cpp extension
+// CHECK-MESSAGES: [[@LINE+3]]:1: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:1: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:1: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .c extension
+#include 
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .cc extension
+#include 
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .cxx extension
+#include 
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
@@ -0,0 +1,39 @@
+//===--- SuspiciousIncludeCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Warns on inclusion of files whose names suggest that they're implementation
+/// files, instead of headers. E.g:
+///
+/// #include "foo.cpp"  // warning
+/// #include "bar.c"// warning
+/// #include "baz.h"// no diagnostic
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html
+class SuspiciousIncludeCheck : public ClangTidyCheck {
+public:
+  SuspiciousIncludeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
@@ -0,0 +1,73 @@
+//===--- SuspiciousIncludeCheck.cpp - clang-tidy --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SuspiciousIncludeCheck.h"
+#include "clang/AST/ASTContext.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+namespace {
+class SuspiciousIncludePPCallbacks : public PPCallbacks {
+public:
+  explicit SuspiciousIncludePPCallbacks(ClangTidyCheck &Check,
+const SourceManager &SM,
+Preprocessor *PP)
+  : Check(Check), PP(PP) {}
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+  StringRef FileName, bool IsAngled,
+  CharSourceRange FilenameRange, const FileEntry *File,
+  StringRef SearchPath, StringRef RelativePath,
+ 

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked an inline comment as done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp:59
+Optional File = PP->LookupFile(
+HashLoc /* FIXME: lies */, (FileName + RE).str(), IsAngled, 
nullptr,
+nullptr, CurDir, nullptr, nullptr, nullptr, nullptr, nullptr);

FIXME: is this a reasonable `SourceLocation` to give to `LookupFile`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



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

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 244994.
jroelofs added a comment.

Implement review feedback re: DiagLoc


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with .cpp extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #include of file with .c extension
+#import "c.c"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include of file with .c extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with .cc extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with .cxx extension
+#  include  
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
@@ -0,0 +1,39 @@
+//===--- SuspiciousIncludeCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Warns on inclusion of files whose names suggest that they're implementation
+/// files, instead of headers. E.g:
+///
+/// #include "foo.cpp"  // warning
+/// #include "bar.c"// warning
+/// #include "baz.h"// no diagnostic
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html
+class SuspiciousIncludeCheck : public ClangTidyCheck {
+public:
+  SuspiciousIncludeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
@@ -0,0 +1,75 @@
+//===--- SuspiciousIncludeCheck.cpp - clang-tidy --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SuspiciousIncludeCheck.h"
+#include "clang/AST/ASTContext.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+namespace {
+class SuspiciousIncludePPCallbacks : public PPCallbacks {
+public:
+  explicit SuspiciousIncludePPCallbacks(ClangTidyCheck &Check,
+const SourceManager &SM,
+Preprocessor *PP)
+  : Check(Check), PP(PP) {}
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+  

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked an inline comment as done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp:59
+Optional File = PP->LookupFile(
+HashLoc /* FIXME: lies */, (FileName + RE).str(), IsAngled, 
nullptr,
+nullptr, CurDir, nullptr, nullptr, nullptr, nullptr, nullptr);

njames93 wrote:
> jroelofs wrote:
> > FIXME: is this a reasonable `SourceLocation` to give to `LookupFile`?
> Main thing is to make sure that the same location is used for the warnings 
> and the notes.
> I personally found this the most pleasing diagnostics
> 
> ```
> SourceLocation DiagLoc = FilenameRange.getBegin().getLocWithOffset(1);
> ```
> 
> ```
> /home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11:
>  warning: suspicious #include of file with .cpp extension 
> [bugprone-suspicious-include]
> #include "a.cpp"
>   ^
> /home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11:
>  note: did you mean to include 'a'?
> /home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11:
>  note: did you mean to include 'a.h'?
> /home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11:
>  note: did you mean to include 'a.hpp'?
> ```
I don't have much experience with modules to lean on for intuition on what this 
check should do with them. I've added a test to codify what the pass currently 
does in that case, but if there's something better it should be trying when 
searching for suggestions, let's make it do that instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



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

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 245031.
jroelofs added a comment.

Make the diagnostics more accurate.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with .cpp extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: suspicious #import of file with .c extension
+#import "c.c"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with .c extension
+#include_next 
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with .cc extension
+# include  
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with .cxx extension
+#  include  
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
@@ -0,0 +1,39 @@
+//===--- SuspiciousIncludeCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Warns on inclusion of files whose names suggest that they're implementation
+/// files, instead of headers. E.g:
+///
+/// #include "foo.cpp"  // warning
+/// #include "bar.c"// warning
+/// #include "baz.h"// no diagnostic
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html
+class SuspiciousIncludeCheck : public ClangTidyCheck {
+public:
+  SuspiciousIncludeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
@@ -0,0 +1,75 @@
+//===--- SuspiciousIncludeCheck.cpp - clang-tidy --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SuspiciousIncludeCheck.h"
+#include "clang/AST/ASTContext.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+namespace {
+class SuspiciousIncludePPCallbacks : public PPCallbacks {
+public:
+  explicit SuspiciousIncludePPCallbacks(ClangTidyCheck &Check,
+const SourceManager &SM,
+Preprocessor *PP)
+  : Check(Check), PP(PP) {}
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+  StringRef FileName, bool IsAngled

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a subscriber: efriedma.
jroelofs added a comment.

>   the targets where you would want to use this can't run the libcxx

testsuite anyway (because they don't have an operating system to run the
test programs under).

I used to run libcxx tests for an arm baremetal toolchain I was building
via semihosted QEMU. It was awkward and slow (especially for some of the
std::*_distribution tests), but it worked.

Jon


Repository:
  rCXX libc++

https://reviews.llvm.org/D41316



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

[PATCH] D39534: [libunwind] Add ifdefs around ELF specific parts of UnwindRegisters*.S for ARM

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D39534#915920, @mstorsjo wrote:

> In https://reviews.llvm.org/D39534#915916, @compnerd wrote:
>
> > Very well, if thats the current implementation in the AsmParser, thats 
> > reasonable.  I don't think that the directive has anything to do with the 
> > file format though.
>
>
> I can agree with that. In addition to making the assembler accept/reject 
> certain instructions though, it actually does another thing which actually is 
> file format specific - it sets the eabi attributes that indicates that the 
> object file contains such instructions.


Are they eabi/gnueabi things?


Repository:
  rL LLVM

https://reviews.llvm.org/D39534



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

[PATCH] D39534: [libunwind] Add ifdefs around ELF specific parts of UnwindRegisters*.S for ARM

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D39534#915937, @mstorsjo wrote:

> In https://reviews.llvm.org/D39534#915935, @jroelofs wrote:
>
> > In https://reviews.llvm.org/D39534#915920, @mstorsjo wrote:
> >
> > > In https://reviews.llvm.org/D39534#915916, @compnerd wrote:
> > >
> > > > Very well, if thats the current implementation in the AsmParser, thats 
> > > > reasonable.  I don't think that the directive has anything to do with 
> > > > the file format though.
> > >
> > >
> > > I can agree with that. In addition to making the assembler accept/reject 
> > > certain instructions though, it actually does another thing which 
> > > actually is file format specific - it sets the eabi attributes that 
> > > indicates that the object file contains such instructions.
> >
> >
> > Are they eabi/gnueabi things?
>
>
> I think so. You can read them with `readelf -A foo.o`, and override them with 
> manual `.eabi_attribute` attributes. (That's useful e.g. for indicating that 
> while a binary contains NEON instructions, it doesn't strict require them for 
> running. E.g. raspbian does check such tags for checking that all binaries 
> work on their baseline of armv6.)


i.e. should this be keyed off of `__ARM_EABI__` instead?


Repository:
  rL LLVM

https://reviews.llvm.org/D39534



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

[PATCH] D39793: [asan] Remove semicolon after do {} while (0)

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs accepted this revision.
jroelofs added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D39793



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

[PATCH] D40218: [Clang] Add __builtin_launder

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: include/clang/Basic/Builtins.def:480
 BUILTIN(__builtin_thread_pointer, "v*", "nc")
+BUILTIN(__builtin_launder, "v*v*", "nt")
 

GCC's is type-generic:

```
#include 

bool is_type_generic() {
  int v;
  return std::is_same::value;
}
```

However this `BUILTIN` line says that this one is not (yeah, I see it's being 
fixed up in Sema later in this patch). I think you need to use custom type 
checking here via `t`.


https://reviews.llvm.org/D40218



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

[PATCH] D40218: [Clang] Add __builtin_launder

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: include/clang/Basic/Builtins.def:480
 BUILTIN(__builtin_thread_pointer, "v*", "nc")
+BUILTIN(__builtin_launder, "v*v*", "nt")
 

EricWF wrote:
> jroelofs wrote:
> > GCC's is type-generic:
> > 
> > ```
> > #include 
> > 
> > bool is_type_generic() {
> >   int v;
> >   return std::is_same >   decltype(&v)>::value;
> > }
> > ```
> > 
> > However this `BUILTIN` line says that this one is not (yeah, I see it's 
> > being fixed up in Sema later in this patch). I think you need to use custom 
> > type checking here via `t`.
> The `t` is specified. It's in the third argument. 
ohhh. duh. sorry.


https://reviews.llvm.org/D40218



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

[PATCH] D28346: [AVR] Allow specifying the CPU on the command line

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

Testcase?


https://reviews.llvm.org/D28346



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

[PATCH] D30945: Fix mis-spelled enum

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs closed this revision.
jroelofs added a comment.

r297756


https://reviews.llvm.org/D30945



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

[PATCH] D29818: [libcxx] Threading support: Attempt to externalize system_clock::now() and steady_clock::now() implementations

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D29818#700949, @ed wrote:

> Worth mentioning: the latest version of macOS now supports `clock_gettime()`. 
> Maybe better to leave the code as is and simply axe the Mach time code at 
> some point in the future?


Supporting only the latest and greatest is unfriendly to folks who are stuck on 
particular versions...


https://reviews.llvm.org/D29818



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D30158#702760, @madsravn wrote:

> In https://reviews.llvm.org/D30158#699342, @jroelofs wrote:
>
> > In https://reviews.llvm.org/D30158#699132, @madsravn wrote:
> >
> > > In https://reviews.llvm.org/D30158#698871, @aaron.ballman wrote:
> > >
> > > > In https://reviews.llvm.org/D30158#696534, @madsravn wrote:
> > > >
> > > > > Any updates on this?
> > > >
> > > >
> > > > Have you run it over the test suite on the revision before 
> > > > random_shuffle was removed from libc++?
> > >
> > >
> > > I can't find any more tests for random_shuffle. I have looked in the SVN 
> > > log for from december 2014 until now. It works on the three tests 
> > > currently in trunk.
> >
> >
> > Try just before r294328.
>
>
> I can't see any random_shuffle tests in the libc++ repo before r294328 
> either. I don't know if they aren't there or if I just can't find them. Would 
> you be able to point them out for me with revision and path?


Did you look at the diff from the commit I mentioned? That's the one that 
removed them.


https://reviews.llvm.org/D30158



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

[PATCH] D31140: [LLVMbugs] [Bug 18710] Only generate .ARM.exidx and .ARM.extab when needed in EHABI

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

Can you clarify the logic here? It's my understanding that:

`-fno-exceptions` does *not* imply `-fno-unwind-tables`

however:

`-fno-unwind-tables` *does* imply that exceptions cannot be used on targets 
that require the tables to do unwinding.


https://reviews.llvm.org/D31140



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

[PATCH] D31375: Add docs for libunwind

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs created this revision.
Herald added a subscriber: mgorny.

I'm still iffy about the build goop for this. I started mostly cargo-culting 
the stuff from libcxx, but couldn't get that to work. What's in the patch seems 
to work for the standalone build, but does not work for in tree builds (and I 
have no idea why).

Comments / corrections / feedback would be much appreciated.


https://reviews.llvm.org/D31375

Files:
  CMakeLists.txt
  docs/BuildingLibunwind.rst
  docs/CMakeLists.txt
  docs/README.txt
  docs/conf.py
  docs/index.rst

Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -0,0 +1,100 @@
+.. _index:
+
+===
+libunwind LLVM Unwinder
+===
+
+Overview
+
+
+libunwind is an implementation of the interface defined by the HP libunwind
+project. It was contributed by Apple as a way to enable clang++ to port to
+platforms that do not have a system unwinder. It is intended to be a small and
+fast implementation of the ABI, leaving off some features of HP's libunwind
+that never materialized (e.g. remote unwinding).
+
+The unwinder has two levels of API. The high level APIs are the `_Unwind_*`
+functions which implement functionality required by `__cxa_*` exception
+funcionts. The low level APIs are the `unw_*` functions which are an interface
+defined by the old HP libunwind project.
+
+Getting Started with libunwind
+--
+
+.. toctree::
+   :maxdepth: 2
+
+   BuildingLibunwind
+
+Current Status
+--
+
+libunwind is a production-quality unwinder, with platform support for DWARF
+unwind info, SjLj, and ARM EHABI.
+
+The low level libunwind API was designed to work either in-process (aka local)
+or to operate on another process (aka remote), but only the local path has been
+implemented. Remote unwinding remains as future work.
+
+Platform and Compiler Support
+-
+
+libunwind is known to work on the following platforms:
+
+   
+OS   Arch CompilersUnwind Info
+   
+Mac OS X i386, x86_64 Clang, GCC   DWARF CFI
+iOS  ARM  ClangSjLj
+Linuxi386, x86_64 Clang, GCC   DWARF CFI
+LinuxARM  Clang, GCC   EHABI
+Bare Metal   ARM  Clang, GCC   EHABI
+   
+
+The following minimum compiler versions are strongly recommended.
+
+* Clang 3.5 and above
+* GCC 4.7 and above.
+
+Anything older *may* work.
+
+Notes and Known Issues
+--
+
+* TODO
+
+Getting Involved
+
+
+First please review our `Developer's Policy `__
+and `Getting started with LLVM `__.
+
+**Bug Reports**
+
+If you think you've found a bug in libunwind, please report it using
+the `LLVM Bugzilla`_. If you're not sure, you
+can post a message to the `cfe-dev mailing list`_ or on IRC.
+Please include "libunwind" in your subject.
+
+**Patches**
+
+If you want to contribute a patch to libc++, the best place for that is
+`Phabricator `_. Please include [libunwind] in the subject and
+add `cfe-commits` as a subscriber. Also make sure you are subscribed to the
+`cfe-commits mailing list `_.
+
+**Discussion and Questions**
+
+Send discussions and questions to the
+`cfe-dev mailing list `_.
+Please include [libunwind] in the subject.
+
+
+Quick Links
+===
+* `LLVM Homepage `_
+* `LLVM Bugzilla `_
+* `cfe-commits Mailing List`_
+* `cfe-dev Mailing List`_
+* `Browse libunwind -- SVN `_
+* `Browse libunwind -- ViewVC `_
Index: docs/conf.py
===
--- docs/conf.py
+++ docs/conf.py
@@ -0,0 +1,251 @@
+# -*- coding: utf-8 -*-
+#
+# libunwind documentation build configuration file.
+#
+# This file is execfile()d with the current directory set to its containing dir.
+#
+# Note that not all possible configuration values are present in this
+# autogenerated file.
+#
+# All configuration values have a default; values that are commented out
+# serve to show the default.
+
+import sys, os
+
+# If extensions (or modules to document with autodoc) are in another directory,
+# add these directories to sys.path here. If the directory is relative to the
+# documentation root, use os.path.abspath to make it absolute, like shown here.
+#sys.path.insert(0, os.path.abspath('.'))
+
+# -- General configurat

[PATCH] D31375: Add docs for libunwind

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 93074.

https://reviews.llvm.org/D31375

Files:
  CMakeLists.txt
  docs/BuildingLibunwind.rst
  docs/CMakeLists.txt
  docs/README.txt
  docs/conf.py
  docs/index.rst

Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -0,0 +1,101 @@
+.. _index:
+
+===
+libunwind LLVM Unwinder
+===
+
+Overview
+
+
+libunwind is an implementation of the interface defined by the HP libunwind
+project. It was contributed by Apple as a way to enable clang++ to port to
+platforms that do not have a system unwinder. It is intended to be a small and
+fast implementation of the ABI, leaving off some features of HP's libunwind
+that never materialized (e.g. remote unwinding).
+
+The unwinder has two levels of API. The high level APIs are the `_Unwind_*`
+functions which implement functionality required by `__cxa_*` exception
+funcionts. The low level APIs are the `unw_*` functions which are an interface
+defined by the old HP libunwind project.
+
+Getting Started with libunwind
+--
+
+.. toctree::
+   :maxdepth: 2
+
+   BuildingLibunwind
+
+Current Status
+--
+
+libunwind is a production-quality unwinder, with platform support for DWARF
+unwind info, SjLj, and ARM EHABI.
+
+The low level libunwind API was designed to work either in-process (aka local)
+or to operate on another process (aka remote), but only the local path has been
+implemented. Remote unwinding remains as future work.
+
+Platform and Compiler Support
+-
+
+libunwind is known to work on the following platforms:
+
+   
+OS   Arch CompilersUnwind Info
+   
+Mac OS X i386, x86_64 Clang, GCC   DWARF CFI
+iOS  ARM  ClangSjLj
+Linuxi386, x86_64 Clang, GCC   DWARF CFI
+LinuxARM  Clang, GCC   EHABI
+Bare Metal   ARM  Clang, GCC   EHABI
+NetBSD   x86_64   Clang, GCC   DWARF CFI
+   
+
+The following minimum compiler versions are strongly recommended.
+
+* Clang 3.5 and above
+* GCC 4.7 and above.
+
+Anything older *may* work.
+
+Notes and Known Issues
+--
+
+* TODO
+
+Getting Involved
+
+
+First please review our `Developer's Policy `__
+and `Getting started with LLVM `__.
+
+**Bug Reports**
+
+If you think you've found a bug in libunwind, please report it using
+the `LLVM Bugzilla`_. If you're not sure, you
+can post a message to the `cfe-dev mailing list`_ or on IRC.
+Please include "libunwind" in your subject.
+
+**Patches**
+
+If you want to contribute a patch to libc++, the best place for that is
+`Phabricator `_. Please include [libunwind] in the subject and
+add `cfe-commits` as a subscriber. Also make sure you are subscribed to the
+`cfe-commits mailing list `_.
+
+**Discussion and Questions**
+
+Send discussions and questions to the
+`cfe-dev mailing list `_.
+Please include [libunwind] in the subject.
+
+
+Quick Links
+===
+* `LLVM Homepage `_
+* `LLVM Bugzilla `_
+* `cfe-commits Mailing List`_
+* `cfe-dev Mailing List`_
+* `Browse libunwind -- SVN `_
+* `Browse libunwind -- ViewVC `_
Index: docs/conf.py
===
--- docs/conf.py
+++ docs/conf.py
@@ -0,0 +1,251 @@
+# -*- coding: utf-8 -*-
+#
+# libunwind documentation build configuration file.
+#
+# This file is execfile()d with the current directory set to its containing dir.
+#
+# Note that not all possible configuration values are present in this
+# autogenerated file.
+#
+# All configuration values have a default; values that are commented out
+# serve to show the default.
+
+import sys, os
+
+# If extensions (or modules to document with autodoc) are in another directory,
+# add these directories to sys.path here. If the directory is relative to the
+# documentation root, use os.path.abspath to make it absolute, like shown here.
+#sys.path.insert(0, os.path.abspath('.'))
+
+# -- General configuration -
+
+# If your documentation needs a minimal Sphinx version, state it here.
+#needs_sphinx = '1.0'
+
+# Add any Sphinx extension module names here, as strings. They can be extensions
+# coming with Sphinx (named 'sphinx.ext.*') 

[PATCH] D31375: Add docs for libunwind

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D31375#710891, @compnerd wrote:

> What happens when you try building it in tree?


The docs-libunwind-html target is missing, and the docs don't get built.




Comment at: docs/index.rst:82
+
+If you want to contribute a patch to libc++, the best place for that is
+`Phabricator `_. Please include 
[libunwind] in the subject and

compnerd wrote:
> ITYM libunwind :)
doh


https://reviews.llvm.org/D31375



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

[PATCH] D31375: Add docs for libunwind

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 93075.

https://reviews.llvm.org/D31375

Files:
  CMakeLists.txt
  docs/BuildingLibunwind.rst
  docs/CMakeLists.txt
  docs/README.txt
  docs/conf.py
  docs/index.rst

Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -0,0 +1,101 @@
+.. _index:
+
+===
+libunwind LLVM Unwinder
+===
+
+Overview
+
+
+libunwind is an implementation of the interface defined by the HP libunwind
+project. It was contributed by Apple as a way to enable clang++ to port to
+platforms that do not have a system unwinder. It is intended to be a small and
+fast implementation of the ABI, leaving off some features of HP's libunwind
+that never materialized (e.g. remote unwinding).
+
+The unwinder has two levels of API. The high level APIs are the `_Unwind_*`
+functions which implement functionality required by `__cxa_*` exception
+funcionts. The low level APIs are the `unw_*` functions which are an interface
+defined by the old HP libunwind project.
+
+Getting Started with libunwind
+--
+
+.. toctree::
+   :maxdepth: 2
+
+   BuildingLibunwind
+
+Current Status
+--
+
+libunwind is a production-quality unwinder, with platform support for DWARF
+unwind info, SjLj, and ARM EHABI.
+
+The low level libunwind API was designed to work either in-process (aka local)
+or to operate on another process (aka remote), but only the local path has been
+implemented. Remote unwinding remains as future work.
+
+Platform and Compiler Support
+-
+
+libunwind is known to work on the following platforms:
+
+   
+OS   Arch CompilersUnwind Info
+   
+Mac OS X i386, x86_64 Clang, GCC   DWARF CFI
+iOS  ARM  ClangSjLj
+Linuxi386, x86_64 Clang, GCC   DWARF CFI
+LinuxARM  Clang, GCC   EHABI
+Bare Metal   ARM  Clang, GCC   EHABI
+NetBSD   x86_64   Clang, GCC   DWARF CFI
+   
+
+The following minimum compiler versions are strongly recommended.
+
+* Clang 3.5 and above
+* GCC 4.7 and above.
+
+Anything older *may* work.
+
+Notes and Known Issues
+--
+
+* TODO
+
+Getting Involved
+
+
+First please review our `Developer's Policy `__
+and `Getting started with LLVM `__.
+
+**Bug Reports**
+
+If you think you've found a bug in libunwind, please report it using
+the `LLVM Bugzilla`_. If you're not sure, you
+can post a message to the `cfe-dev mailing list`_ or on IRC.
+Please include "libunwind" in your subject.
+
+**Patches**
+
+If you want to contribute a patch to libunwind, the best place for that is
+`Phabricator `_. Please include [libunwind] in the subject and
+add `cfe-commits` as a subscriber. Also make sure you are subscribed to the
+`cfe-commits mailing list `_.
+
+**Discussion and Questions**
+
+Send discussions and questions to the
+`cfe-dev mailing list `_.
+Please include [libunwind] in the subject.
+
+
+Quick Links
+===
+* `LLVM Homepage `_
+* `LLVM Bugzilla `_
+* `cfe-commits Mailing List`_
+* `cfe-dev Mailing List`_
+* `Browse libunwind -- SVN `_
+* `Browse libunwind -- ViewVC `_
Index: docs/conf.py
===
--- docs/conf.py
+++ docs/conf.py
@@ -0,0 +1,251 @@
+# -*- coding: utf-8 -*-
+#
+# libunwind documentation build configuration file.
+#
+# This file is execfile()d with the current directory set to its containing dir.
+#
+# Note that not all possible configuration values are present in this
+# autogenerated file.
+#
+# All configuration values have a default; values that are commented out
+# serve to show the default.
+
+import sys, os
+
+# If extensions (or modules to document with autodoc) are in another directory,
+# add these directories to sys.path here. If the directory is relative to the
+# documentation root, use os.path.abspath to make it absolute, like shown here.
+#sys.path.insert(0, os.path.abspath('.'))
+
+# -- General configuration -
+
+# If your documentation needs a minimal Sphinx version, state it here.
+#needs_sphinx = '1.0'
+
+# Add any Sphinx extension module names here, as strings. They can be extensions
+# coming with Sphinx (named 'sphinx.ext.*

[PATCH] D31375: Add docs for libunwind

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D31375#711839, @EricWF wrote:

> In https://reviews.llvm.org/D31375#710897, @jroelofs wrote:
>
> > In https://reviews.llvm.org/D31375#710891, @compnerd wrote:
> >
> > > What happens when you try building it in tree?
> >
> >
> > The docs-libunwind-html target is missing, and the docs don't get built.
>
>
> Shouldn't that be what happens when libunwind is built 
> "out-of-tree"/standalone, not the other way around?


"is" != "ought". That's what /is/ happening, not necessarily what we want to 
happen.


https://reviews.llvm.org/D31375



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

[PATCH] D31375: Add docs for libunwind

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D31375#712152, @jroelofs wrote:

> In https://reviews.llvm.org/D31375#711839, @EricWF wrote:
>
> > In https://reviews.llvm.org/D31375#710897, @jroelofs wrote:
> >
> > > In https://reviews.llvm.org/D31375#710891, @compnerd wrote:
> > >
> > > > What happens when you try building it in tree?
> > >
> > >
> > > The docs-libunwind-html target is missing, and the docs don't get built.
> >
> >
> > Shouldn't that be what happens when libunwind is built 
> > "out-of-tree"/standalone, not the other way around?
>
>
> "is" != "ought". That's what /is/ happening, not necessarily what we want to 
> happen.


And FWIW, it looks like libcxx's docs behave the same way... So this might be a 
problem with the way the `runtimes` dir works.


https://reviews.llvm.org/D31375



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

[PATCH] D31375: Add docs for libunwind

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs closed this revision.
jroelofs added a comment.

r298922


https://reviews.llvm.org/D31375



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

[PATCH] D31422: Add builder for libunwind docs

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs created this revision.

Dimitri, do you mind hosting another docs builder on your machine?


https://reviews.llvm.org/D31422

Files:
  buildbot/osuosl/master/config/builders.py
  zorg/buildbot/builders/SphinxDocsBuilder.py


Index: zorg/buildbot/builders/SphinxDocsBuilder.py
===
--- zorg/buildbot/builders/SphinxDocsBuilder.py
+++ zorg/buildbot/builders/SphinxDocsBuilder.py
@@ -11,7 +11,8 @@
 clang_html= False, # Build Clang HTML documentation
 clang_tools_html  = False, # Build Clang Extra Tools HTML documentation
 lld_html  = False, # Build LLD HTML documentation
-libcxx_html   = False  # Build Libc++ HTML documentation
+libcxx_html   = False, # Build Libc++ HTML documentation
+libunwind_html= False  # Build libunwind HTML documentation
 ):
 
 f = buildbot.process.factory.BuildFactory()
@@ -23,6 +24,7 @@
 lld_srcdir = llvm_srcdir + '/tools/lld'
 libcxx_srcdir = llvm_srcdir + '/projects/libcxx'
 libcxxabi_srcdir = llvm_srcdir + '/projects/libcxxabi'
+libunwind_srcdir = llvm_srcdir + '/projects/libunwind'
 
 # Get LLVM. This is essential for all builds
 # because we build all subprojects in tree
@@ -65,6 +67,13 @@
   defaultBranch='trunk',
   workdir=libcxxabi_srcdir))
 
+if libunwind_html:
+f.addStep(SVN(name='svn-libunwind',
+  mode='update',
+  baseURL='http://llvm.org/svn/llvm-project/libunwind/',
+  defaultBranch='trunk',
+  workdir=libunwind_srcdir))
+
 f.addStep(ShellCommand(name="create-build-dir",
command=["mkdir", "-p", llvm_objdir],
haltOnFailure=False, # We might of already 
created the directory in a previous build
@@ -132,4 +141,12 @@
targets=['docs-libcxx-html']
   ))
 
+if libunwind_html:
+f.addStep(NinjaCommand(name="docs-libunwind-html",
+   haltOnFailure=True,
+   description=["Build libunwind Sphinx HTML 
documentation"],
+   workdir=llvm_objdir,
+   targets=['docs-libunwind-html']
+  ))
+
 return f
Index: buildbot/osuosl/master/config/builders.py
===
--- buildbot/osuosl/master/config/builders.py
+++ buildbot/osuosl/master/config/builders.py
@@ -1353,6 +1353,13 @@
'builddir':"libcxx-sphinx-docs",
'factory': 
SphinxDocsBuilder.getSphinxDocsBuildFactory(libcxx_html=True),
'category' : 'libcxx'
+ },
+ {
+   'name':"libunwind-sphinx-docs",
+   'slavenames':["gribozavr3"],
+   'builddir':"libunwind-sphinx-docs",
+   'factory': 
SphinxDocsBuilder.getSphinxDocsBuildFactory(libunwind_html=True),
+   'category' : 'libunwind'
  }
]
 


Index: zorg/buildbot/builders/SphinxDocsBuilder.py
===
--- zorg/buildbot/builders/SphinxDocsBuilder.py
+++ zorg/buildbot/builders/SphinxDocsBuilder.py
@@ -11,7 +11,8 @@
 clang_html= False, # Build Clang HTML documentation
 clang_tools_html  = False, # Build Clang Extra Tools HTML documentation
 lld_html  = False, # Build LLD HTML documentation
-libcxx_html   = False  # Build Libc++ HTML documentation
+libcxx_html   = False, # Build Libc++ HTML documentation
+libunwind_html= False  # Build libunwind HTML documentation
 ):
 
 f = buildbot.process.factory.BuildFactory()
@@ -23,6 +24,7 @@
 lld_srcdir = llvm_srcdir + '/tools/lld'
 libcxx_srcdir = llvm_srcdir + '/projects/libcxx'
 libcxxabi_srcdir = llvm_srcdir + '/projects/libcxxabi'
+libunwind_srcdir = llvm_srcdir + '/projects/libunwind'
 
 # Get LLVM. This is essential for all builds
 # because we build all subprojects in tree
@@ -65,6 +67,13 @@
   defaultBranch='trunk',
   workdir=libcxxabi_srcdir))
 
+if libunwind_html:
+f.addStep(SVN(name='svn-libunwind',
+  mode='update',
+  baseURL='http://llvm.org/svn/llvm-project/libunwind/',
+  defaultBranch='trunk',
+  workdir=libunwind_srcdir))
+
 f.addStep(ShellCommand(name="create-build-dir",
command=["mkdir", "-p", llvm_objdir],
haltOnFailure=False, # We might of already created the directory in a previous build
@@ -132,4 +141,12 @@
targets=['docs-libcxx-html']
  

[PATCH] D31375: Add docs for libunwind

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

Review for adding a builder in: https://reviews.llvm.org/D31422


https://reviews.llvm.org/D31375



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

[PATCH] D25157: [compiler-rt] [cmake] Respect COMPILER_RT_BUILD_* for libs, headers and tests

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: test/sanitizer_common/CMakeLists.txt:7
 set(SUPPORTED_TOOLS)
-if(CMAKE_SYSTEM_NAME MATCHES "Darwin|Linux|FreeBSD" AND NOT ANDROID)
+if(CMAKE_SYSTEM_NAME MATCHES "Darwin|Linux|FreeBSD" AND NOT ANDROID AND
+   COMPILER_RT_HAS_ASAN)

Are the `CMAKE_SYSTEM_NAME_MATCHES` guards here necessary any more, now that 
the checks are happening in config-ix.cmake?


https://reviews.llvm.org/D25157



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