[PATCH] D80425: Fix LLVM/Clang builds with mingw toolchain

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
Herald added subscribers: llvm-commits, cfe-commits, mstorsjo, dexonsmith, 
mgorny.
Herald added projects: clang, LLVM.

These are a collection of small fixes to make LLVM/Clang build with a 
clang+mingw toolchain to target Windows.

The three commits address the following problems:

- When using LTO we pass --lto-cache-directory to lld - but this option is not 
supported by the lld MingW driver so it fails with unknown argument.
- Don't symlink the tools - a MingW build version of clang should be assumed to 
be used on Windows - which doesn't support symlinks correctly - so instead use 
the copy path of the code for MingW as well.
- The logic for linking libclang with libdl was a bit flawed - use the similar 
logic as to other places in the CMake build system.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80425

Files:
  clang/tools/libclang/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/HandleLLVMOptions.cmake


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -66,9 +66,8 @@
   endif ()
 endif ()
 
-find_library(DL_LIBRARY_PATH dl)
-if (DL_LIBRARY_PATH)
-  list(APPEND LIBS dl)
+if (HAVE_LIBDL)
+  list(APPEND LIBS ${CMAKE_DL_LIBS})
 endif()
 
 option(LIBCLANG_BUILD_STATIC
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -953,6 +953,8 @@
   # time a little since we re-link a lot of the same objects, and significantly
   # improves incremental build time.
   # FIXME: We should move all this logic into the clang driver.
+  #
+  # FIXME; mingw lld driver doesn't support any of the options below
   if(APPLE)
 append("-Wl,-cache_path_lto,${PROJECT_BINARY_DIR}/lto.cache"
CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
@@ -962,7 +964,7 @@
   elseif(LLVM_USE_LINKER STREQUAL "gold")
 append("-Wl,--plugin-opt,cache-dir=${PROJECT_BINARY_DIR}/lto.cache"
CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
-  elseif(LINKER_IS_LLD_LINK)
+  elseif(LINKER_IS_LLD_LINK AND NOT MINGW)
 append("/lldltocache:${PROJECT_BINARY_DIR}/lto.cache"
CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
   endif()
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1868,7 +1868,7 @@
   if(NOT ARG_OUTPUT_DIR)
 # If you're not overriding the OUTPUT_DIR, we can make the link relative in
 # the same directory.
-if(CMAKE_HOST_UNIX)
+if(CMAKE_HOST_UNIX AND NOT MINGW)
   set(dest_binary "$")
 endif()
 if(CMAKE_CONFIGURATION_TYPES)
@@ -1894,7 +1894,7 @@
 endif()
   endif()
 
-  if(CMAKE_HOST_UNIX)
+  if(CMAKE_HOST_UNIX AND NOT MINGW)
 set(LLVM_LINK_OR_COPY create_symlink)
   else()
 set(LLVM_LINK_OR_COPY copy)


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -66,9 +66,8 @@
   endif ()
 endif ()
 
-find_library(DL_LIBRARY_PATH dl)
-if (DL_LIBRARY_PATH)
-  list(APPEND LIBS dl)
+if (HAVE_LIBDL)
+  list(APPEND LIBS ${CMAKE_DL_LIBS})
 endif()
 
 option(LIBCLANG_BUILD_STATIC
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -953,6 +953,8 @@
   # time a little since we re-link a lot of the same objects, and significantly
   # improves incremental build time.
   # FIXME: We should move all this logic into the clang driver.
+  #
+  # FIXME; mingw lld driver doesn't support any of the options below
   if(APPLE)
 append("-Wl,-cache_path_lto,${PROJECT_BINARY_DIR}/lto.cache"
CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
@@ -962,7 +964,7 @@
   elseif(LLVM_USE_LINKER STREQUAL "gold")
 append("-Wl,--plugin-opt,cache-dir=${PROJECT_BINARY_DIR}/lto.cache"
CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
-  elseif(LINKER_IS_LLD_LINK)
+  elseif(LINKER_IS_LLD_LINK AND NOT MINGW)
 append("/lldltocache:${PROJECT_BINARY_DIR}/lto.cache"
CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
   endif()
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1868,7 +1868,7 @@
   if(NOT ARG_OUTPUT_DIR)
 # If you're not overriding the OUTPUT_DIR, we can make the link relative in
 # the same directory.
-if(CMAKE_HOST_UNIX)
+if(CMAKE_HOST_UNIX AND NOT MINGW)
   set(dest_binary "$")
  

[PATCH] D80425: Fix LLVM/Clang builds with mingw toolchain

[Tobias Hieta via Phabricator via cfe-commits]

thieta marked 5 inline comments as done.
thieta added inline comments.



Comment at: clang/tools/libclang/CMakeLists.txt:71
+  list(APPEND LIBS ${CMAKE_DL_LIBS})
 endif()
 

mstorsjo wrote:
> If you say this is the same way it's done elsewhere, then sure - although I 
> have no idea about what the issue is, why I haven't run into it, etc. 
> Normally you wouldn't have a `libdl` on mingw right? What's the concrete 
> issue you're running into, and in which conditions would one run into it?
The problem here is that on my system `find_library()` picks up libdl in 
`/usr/lib` and then tries to link to it and gives me an error about it. 
`HAVE_LIBDL` comes from `config-ix.cmake` where it's checked if we are on 
windows or not: 
https://github.com/llvm/llvm-project/blob/216833b32befd14079130a3b857906f4e301179c/llvm/cmake/config-ix.cmake#L101

So this is how other places uses `CMAKE_DL_LIBS` like here: 
https://github.com/llvm/llvm-project/blob/7aaff8fd2da2812a2b3cbc8a41af29774b10a7d6/llvm/lib/Support/CMakeLists.txt#L13



Comment at: llvm/cmake/modules/AddLLVM.cmake:1872
+if(CMAKE_HOST_UNIX AND NOT MINGW)
   set(dest_binary "$")
 endif()

mstorsjo wrote:
> Not entirely sure what this one does - is it just a condition that needs to 
> match the one for `create_symlink` below?
As the comment above:

```
# If you're not overriding the OUTPUT_DIR, we can make the link relative in
# the same directory.
```

so that means that it creates the symlink like `clang-10.exe -> clang.exe`

So this breaks when using the copy path below.



Comment at: llvm/cmake/modules/AddLLVM.cmake:1898
+  if(CMAKE_HOST_UNIX AND NOT MINGW)
 set(LLVM_LINK_OR_COPY create_symlink)
   else()

mstorsjo wrote:
> What's the practical issue you're trying to fix with this one here? If 
> `CMAKE_HOST_UNIX`, i.e. when cross compiling, it does work fine to create 
> symlinks (saving a bit of disk space and bandwidth). Then when transferring 
> the built products to an actual windows system, they're converted into copies 
> at some point (e.g. when zipping up the results).
The problem I tried to fix here was to avoid creating symlinks since the 
binaries under mingw is always going to be executed on windows - so yeah I 
could remove the symlinks and redo them as copies later - but I thought it 
might be better to just do it directly from the build system so that you can 
build on wsl and use it from the same dir from the windows host.

But yeah I am not married to this change - I think it's not to vital.



Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:957
+  #
+  # FIXME; mingw lld driver doesn't support any of the options below
   if(APPLE)

mstorsjo wrote:
> We could certainly add support for it in the mingw driver - ideally with the 
> same name as for ELF, which then would be remapped to the corresponding 
> lld-link option. This takes just a couple lines in lld/MinGW/Options.td, 
> lld/MinGW/Driver.cpp and lld/test/MinGW/driver.test.
TBH I was trying to avoid that bit of extra work :)

But yeah it would be nice to have it supported there as well - I could maintain 
this change as a patch locally if you don't think I should have this in there 
while waiting for doing the change in the lld driver.



Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:967
CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
-  elseif(LINKER_IS_LLD_LINK)
+  elseif(LINKER_IS_LLD_LINK AND NOT MINGW)
 append("/lldltocache:${PROJECT_BINARY_DIR}/lto.cache"

mstorsjo wrote:
> Do you happen to know why `LINKER_IS_LLD_LINK` gets set in this case? 
> `ld.lld` (the ELF linker interface, which then the MinGW driver remaps onto 
> the COFF backend with the `lld-link` interface) certainly doesn't take 
> `lld-link` style options. I believe (without diving further into it) that we 
> shouldn't be setting this flag in this combination, but with the option 
> implemented, we should fit it into the case further above, `elseif((UNIX OR 
> MINGW) AND LLVM_USE_LINKER STREQUAL "lld")`
Yeah I bet that variable is set because I pass `LLVM_USE_LINKER=lld` but I 
haven't digged to deeply. I can rework the if statement here when we have the 
lld option in there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80425



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

[PATCH] D80425: Fix LLVM/Clang builds with mingw toolchain

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80425



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

[PATCH] D80425: Fix LLVM/Clang builds with mingw toolchain

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80425



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

[PATCH] D80425: Fix LLVM/Clang builds with mingw toolchain

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

I am planning to revise this one now that we have thinlto-cache-dir option 
landed here are my plans:

- Keep the libdl patch as is (seems like there are no more comments on this).
- Remove the symlink patch for now and potentially move that to another patch
- Rework the cache-dir option so that it passes the same option as we pass to 
ELF lld.

Is that what everyone would expect?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80425



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

[PATCH] D80425: Fix LLVM/Clang builds with mingw toolchain

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Sounds good - I'll close this one and open three new ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80425



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

[PATCH] D80492: Avoid linking libdl unless needed

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
thieta added a reviewer: mstorsjo.
thieta added a project: clang.
Herald added subscribers: cfe-commits, mgorny.

This fixes cross-compilation when targeting a platform not using libdl and 
mirrors libdl linking in other parts of the code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80492

Files:
  clang/tools/libclang/CMakeLists.txt


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -66,9 +66,8 @@
   endif ()
 endif ()
 
-find_library(DL_LIBRARY_PATH dl)
-if (DL_LIBRARY_PATH)
-  list(APPEND LIBS dl)
+if (HAVE_LIBDL)
+  list(APPEND LIBS ${CMAKE_DL_LIBS})
 endif()
 
 option(LIBCLANG_BUILD_STATIC


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -66,9 +66,8 @@
   endif ()
 endif ()
 
-find_library(DL_LIBRARY_PATH dl)
-if (DL_LIBRARY_PATH)
-  list(APPEND LIBS dl)
+if (HAVE_LIBDL)
+  list(APPEND LIBS ${CMAKE_DL_LIBS})
 endif()
 
 option(LIBCLANG_BUILD_STATIC
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80492: Avoid linking libdl unless needed

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

@mstorsjo mind landing this for me as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80492



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

[PATCH] D77746: [Driver] Default arm-linux-androideabi to -z max-page-size=4096

[Tobias Hieta via Phabricator via cfe-commits]

thieta accepted this revision.
thieta added a comment.

Thanks for handling this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77746



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

[PATCH] D80492: Avoid linking libdl unless needed

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Thanks - I'll look into a fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80492



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

[PATCH] D81385: Fix libdl linking for libclang in standalone mode

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
thieta added reviewers: mstorsjo, beanz, lebedev.ri.
thieta added a project: clang.
Herald added subscribers: cfe-commits, mgorny.

As noted in https://reviews.llvm.org/D80492 that change broke standalone builds 
of clang. This reverts the old behavior of libdl linking when in standalone 
mode.

As a note I think it would be safe to just do:

`list(APPEND LIBS ${CMAKE_DL_LIBS})`

But I opted for the pragmatic fix this time. Let me know if you think we should 
try that instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81385

Files:
  clang/tools/libclang/CMakeLists.txt


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -68,7 +68,12 @@
 
 if (HAVE_LIBDL)
   list(APPEND LIBS ${CMAKE_DL_LIBS})
-endif()
+elseif (CLANG_BUILT_STANDALONE)
+  find_library(DL_LIBRARY_PATH dl)
+  if (DL_LIBRARY_PATH)
+list(APPEND LIBS dl)
+  endif ()
+endif ()
 
 option(LIBCLANG_BUILD_STATIC
   "Build libclang as a static library (in addition to a shared one)" OFF)


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -68,7 +68,12 @@
 
 if (HAVE_LIBDL)
   list(APPEND LIBS ${CMAKE_DL_LIBS})
-endif()
+elseif (CLANG_BUILT_STANDALONE)
+  find_library(DL_LIBRARY_PATH dl)
+  if (DL_LIBRARY_PATH)
+list(APPEND LIBS dl)
+  endif ()
+endif ()
 
 option(LIBCLANG_BUILD_STATIC
   "Build libclang as a static library (in addition to a shared one)" OFF)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80492: Avoid linking libdl unless needed

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

@lebedev.ri fix for your issue posted here: https://reviews.llvm.org/D81385


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80492



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

[PATCH] D132791: Fix formatting in release notes

[Tobias Hieta via Phabricator via cfe-commits]

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

Thanks for doing this pass! Much appreciated. As long as you tried to build the 
docs after you changes and it passes without any warnings or errors I am happy 
for you to commit this directly to `release/15.x` - just let me know if you do 
it so I can sync the repos.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132791

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

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Hmm. 15.0.0 is just a week away - I am not planning any more RCs unless we hit 
something critical. What's the risk of taking this specific change at this 
point? Would it make more sense to wait for 15.0.1? (I am guessing it's better 
if it goes into 15.0.0 or not in 15.x at all).

I am just pushing back because I don't want to introduce to much risk this late 
in the cycle - but I will defer to you guys since you are the domain experts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132877

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

[PATCH] D133044: [Frontend] Restore Preprocessor::getPredefines()

[Tobias Hieta via Phabricator via cfe-commits]

thieta accepted this revision.
thieta added a comment.

LGTM and seems pretty safe so I am not opposed to merge it before 15 final.

I would maybe add a link to the GitHub issue in the comment, but that's a nit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133044

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

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

I think the easiest way to handle this is that when you have it approved here - 
push it to fork on GitHub based on the release branch and create a GitHub issue 
and write /branch aballman/llvm-project/my_branch in a comment and it will 
queue up the cherry pick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133800

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

[PATCH] D128409: [clang-cl] Add -emit-ast to clang-cl driver

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 440178.
thieta added a comment.

Added tests and moved the check for TY_Plist and TY_AST to it's own if 
statement block


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128409

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/ast.c
  clang/test/Driver/cl-outputs.c


Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -37,6 +37,12 @@
 // CHECK-MULTIPLESOURCEOK2: "-o" "cl-outputs.obj"
 // CHECK-MULTIPLESOURCEOK2: "-o" "cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /Fotest.ast -###  -- %s  2>&1 | FileCheck 
-check-prefix=FoASTNAME %s
+// FoASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /Fotest.plist -### -- %s 2>&1 | FileCheck 
-check-prefix=FoPLIST %s
+// FoPLIST:  "-o" "test.plist"
+
 // RUN: %clang_cl /c /oa -### -- %s 2>&1 | FileCheck -check-prefix=oNAME1 %s
 // oNAME1:  "-o" "a.obj"
 
@@ -82,6 +88,11 @@
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck 
-check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /otest.ast -###  -- %s  2>&1 | FileCheck 
-check-prefix=oASTNAME %s
+// oASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /otest.plist -###  -- %s  2>&1 | FileCheck 
-check-prefix=oPLIST %s
+// oPLIST:  "-o" "test.plist"
 
 // RUN: %clang_cl /c /obar /Fofoo -### -- %s 2>&1 | FileCheck 
-check-prefix=FooRACE1 %s
 // FooRACE1: "-o" "foo.obj"
Index: clang/test/Driver/ast.c
===
--- clang/test/Driver/ast.c
+++ clang/test/Driver/ast.c
@@ -25,3 +25,15 @@
 // FIXME: There is a problem with compiling AST's in that the input language is
 // not available for use by other tools (for example, to automatically add
 // -lstdc++). We may need -x [objective-]c++-ast and all that goodness. :(
+
+// Also check clang-cl since the driver is slightly different
+// RUN: %clang_cl -ccc-print-phases -emit-ast %s 2> %t
+// RUN: echo 'END' >> %t
+// RUN: FileCheck -check-prefix EMIT-AST-PHASES-CLANGCL -input-file %t %s
+
+// EMIT-AST-PHASES-CLANGCL: 0: input,
+// EMIT-AST-PHASES-CLANGCL: , c
+// EMIT-AST-PHASES-CLANGCL: 1: preprocessor, {0}, cpp-output
+// EMIT-AST-PHASES-CLANGCL: 2: compiler, {1}, ast
+// EMIT-AST-PHASES-CLANGCL-NOT: 3:
+// EMIT-AST-PHASES-CLANGCL: END
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5672,6 +5672,14 @@
 }
   } else if (JA.getType() == types::TY_PCH && IsCLMode()) {
 NamedOutput = C.getArgs().MakeArgString(GetClPchPath(C, BaseName));
+  } else if ((JA.getType() == types::TY_Plist || JA.getType() == 
types::TY_AST) &&
+ C.getArgs().hasArg(options::OPT__SLASH_Fo, 
options::OPT__SLASH_o)) {
+StringRef Val =
+C.getArgs()
+.getLastArg(options::OPT__SLASH_Fo, options::OPT__SLASH_o)
+->getValue();
+NamedOutput =
+MakeCLOutputFilename(C.getArgs(), Val, BaseName, types::TY_Object);
   } else {
 const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
 assert(Suffix && "All types used for output should have a suffix.");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1091,7 +1091,7 @@
 def dynamic : Flag<["-"], "dynamic">, Flags<[NoArgumentUnused]>;
 def d_Flag : Flag<["-"], "d">, Group;
 def d_Joined : Joined<["-"], "d">, Group;
-def emit_ast : Flag<["-"], "emit-ast">,
+def emit_ast : Flag<["-"], "emit-ast">, Flags<[CoreOption]>,
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option, FC1Option, 
FlangOption]>, Group,
   HelpText<"Use the LLVM representation for assembler and object files">;


Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -37,6 +37,12 @@
 // CHECK-MULTIPLESOURCEOK2: "-o" "cl-outputs.obj"
 // CHECK-MULTIPLESOURCEOK2: "-o" "cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /Fotest.ast -###  -- %s  2>&1 | FileCheck -check-prefix=FoASTNAME %s
+// FoASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /Fotest.plist -### -- %s 2>&1 | FileCheck -check-prefix=FoPLIST %s
+// FoPLIST:  "-o" "test.plist"
+
 // RUN: %clang_cl /c /oa -### -- %s 2>&1 | FileCheck -check-prefix=oNAME1 %s
 // oNAME1:  "-o" "a.obj"
 
@@ -82,6 +88,11 @@
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck -check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/

[PATCH] D128409: [clang-cl] Add -emit-ast to clang-cl driver

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 440227.
thieta added a comment.

Fixed missing -- in test file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128409

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/ast.c
  clang/test/Driver/cl-outputs.c


Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -37,6 +37,12 @@
 // CHECK-MULTIPLESOURCEOK2: "-o" "cl-outputs.obj"
 // CHECK-MULTIPLESOURCEOK2: "-o" "cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /Fotest.ast -###  -- %s  2>&1 | FileCheck 
-check-prefix=FoASTNAME %s
+// FoASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /Fotest.plist -### -- %s 2>&1 | FileCheck 
-check-prefix=FoPLIST %s
+// FoPLIST:  "-o" "test.plist"
+
 // RUN: %clang_cl /c /oa -### -- %s 2>&1 | FileCheck -check-prefix=oNAME1 %s
 // oNAME1:  "-o" "a.obj"
 
@@ -82,6 +88,11 @@
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck 
-check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /otest.ast -###  -- %s  2>&1 | FileCheck 
-check-prefix=oASTNAME %s
+// oASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /otest.plist -###  -- %s  2>&1 | FileCheck 
-check-prefix=oPLIST %s
+// oPLIST:  "-o" "test.plist"
 
 // RUN: %clang_cl /c /obar /Fofoo -### -- %s 2>&1 | FileCheck 
-check-prefix=FooRACE1 %s
 // FooRACE1: "-o" "foo.obj"
Index: clang/test/Driver/ast.c
===
--- clang/test/Driver/ast.c
+++ clang/test/Driver/ast.c
@@ -25,3 +25,15 @@
 // FIXME: There is a problem with compiling AST's in that the input language is
 // not available for use by other tools (for example, to automatically add
 // -lstdc++). We may need -x [objective-]c++-ast and all that goodness. :(
+
+// Also check clang-cl since the driver is slightly different
+// RUN: %clang_cl -ccc-print-phases -emit-ast -- %s 2> %t
+// RUN: echo 'END' >> %t
+// RUN: FileCheck -check-prefix EMIT-AST-PHASES-CLANGCL -input-file %t %s
+
+// EMIT-AST-PHASES-CLANGCL: 0: input,
+// EMIT-AST-PHASES-CLANGCL: , c
+// EMIT-AST-PHASES-CLANGCL: 1: preprocessor, {0}, cpp-output
+// EMIT-AST-PHASES-CLANGCL: 2: compiler, {1}, ast
+// EMIT-AST-PHASES-CLANGCL-NOT: 3:
+// EMIT-AST-PHASES-CLANGCL: END
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5672,6 +5672,14 @@
 }
   } else if (JA.getType() == types::TY_PCH && IsCLMode()) {
 NamedOutput = C.getArgs().MakeArgString(GetClPchPath(C, BaseName));
+  } else if ((JA.getType() == types::TY_Plist || JA.getType() == 
types::TY_AST) &&
+ C.getArgs().hasArg(options::OPT__SLASH_Fo, 
options::OPT__SLASH_o)) {
+StringRef Val =
+C.getArgs()
+.getLastArg(options::OPT__SLASH_Fo, options::OPT__SLASH_o)
+->getValue();
+NamedOutput =
+MakeCLOutputFilename(C.getArgs(), Val, BaseName, types::TY_Object);
   } else {
 const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
 assert(Suffix && "All types used for output should have a suffix.");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1091,7 +1091,7 @@
 def dynamic : Flag<["-"], "dynamic">, Flags<[NoArgumentUnused]>;
 def d_Flag : Flag<["-"], "d">, Group;
 def d_Joined : Joined<["-"], "d">, Group;
-def emit_ast : Flag<["-"], "emit-ast">,
+def emit_ast : Flag<["-"], "emit-ast">, Flags<[CoreOption]>,
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option, FC1Option, 
FlangOption]>, Group,
   HelpText<"Use the LLVM representation for assembler and object files">;


Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -37,6 +37,12 @@
 // CHECK-MULTIPLESOURCEOK2: "-o" "cl-outputs.obj"
 // CHECK-MULTIPLESOURCEOK2: "-o" "cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /Fotest.ast -###  -- %s  2>&1 | FileCheck -check-prefix=FoASTNAME %s
+// FoASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /Fotest.plist -### -- %s 2>&1 | FileCheck -check-prefix=FoPLIST %s
+// FoPLIST:  "-o" "test.plist"
+
 // RUN: %clang_cl /c /oa -### -- %s 2>&1 | FileCheck -check-prefix=oNAME1 %s
 // oNAME1:  "-o" "a.obj"
 
@@ -82,6 +88,11 @@
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck -check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /o

[PATCH] D128409: [clang-cl] Add -emit-ast to clang-cl driver

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Fixed the test issue. Regarding `/Fo` bit - if you want I can change it to only 
handle `/o` in this if statement. I don't mind either way.




Comment at: clang/test/Driver/ast.c:30
+// Also check clang-cl since the driver is slightly different
+// RUN: %clang_cl -ccc-print-phases -emit-ast %s 2> %t
+// RUN: echo 'END' >> %t

hans wrote:
> This needs a `--` before `%s` otherwise clang-cl might interpret the filename 
> as a flag, e.g. if it starts with /Users which is common on Mac.
Fixed - will also fix the other tests in the same file as a NFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128409

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

[PATCH] D128409: [clang-cl] Add -emit-ast to clang-cl driver

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 440242.
thieta added a comment.

Just handle /o for AST and plist files


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128409

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/ast.c
  clang/test/Driver/cl-outputs.c


Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -82,6 +82,11 @@
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck 
-check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /otest.ast -###  -- %s  2>&1 | FileCheck 
-check-prefix=oASTNAME %s
+// oASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /otest.plist -###  -- %s  2>&1 | FileCheck 
-check-prefix=oPLIST %s
+// oPLIST:  "-o" "test.plist"
 
 // RUN: %clang_cl /c /obar /Fofoo -### -- %s 2>&1 | FileCheck 
-check-prefix=FooRACE1 %s
 // FooRACE1: "-o" "foo.obj"
Index: clang/test/Driver/ast.c
===
--- clang/test/Driver/ast.c
+++ clang/test/Driver/ast.c
@@ -25,3 +25,15 @@
 // FIXME: There is a problem with compiling AST's in that the input language is
 // not available for use by other tools (for example, to automatically add
 // -lstdc++). We may need -x [objective-]c++-ast and all that goodness. :(
+
+// Also check clang-cl since the driver is slightly different
+// RUN: %clang_cl -ccc-print-phases -emit-ast -- %s 2> %t
+// RUN: echo 'END' >> %t
+// RUN: FileCheck -check-prefix EMIT-AST-PHASES-CLANGCL -input-file %t %s
+
+// EMIT-AST-PHASES-CLANGCL: 0: input,
+// EMIT-AST-PHASES-CLANGCL: , c
+// EMIT-AST-PHASES-CLANGCL: 1: preprocessor, {0}, cpp-output
+// EMIT-AST-PHASES-CLANGCL: 2: compiler, {1}, ast
+// EMIT-AST-PHASES-CLANGCL-NOT: 3:
+// EMIT-AST-PHASES-CLANGCL: END
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5661,6 +5661,14 @@
 }
   } else if (JA.getType() == types::TY_PCH && IsCLMode()) {
 NamedOutput = C.getArgs().MakeArgString(GetClPchPath(C, BaseName));
+  } else if ((JA.getType() == types::TY_Plist || JA.getType() == 
types::TY_AST) &&
+ C.getArgs().hasArg(options::OPT__SLASH_o)) {
+StringRef Val =
+C.getArgs()
+.getLastArg(options::OPT__SLASH_o)
+->getValue();
+NamedOutput =
+MakeCLOutputFilename(C.getArgs(), Val, BaseName, types::TY_Object);
   } else {
 const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
 assert(Suffix && "All types used for output should have a suffix.");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1097,7 +1097,7 @@
 def dynamic : Flag<["-"], "dynamic">, Flags<[NoArgumentUnused]>;
 def d_Flag : Flag<["-"], "d">, Group;
 def d_Joined : Joined<["-"], "d">, Group;
-def emit_ast : Flag<["-"], "emit-ast">,
+def emit_ast : Flag<["-"], "emit-ast">, Flags<[CoreOption]>,
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option, FC1Option, 
FlangOption]>, Group,
   HelpText<"Use the LLVM representation for assembler and object files">;


Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -82,6 +82,11 @@
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck -check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /otest.ast -###  -- %s  2>&1 | FileCheck -check-prefix=oASTNAME %s
+// oASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /otest.plist -###  -- %s  2>&1 | FileCheck -check-prefix=oPLIST %s
+// oPLIST:  "-o" "test.plist"
 
 // RUN: %clang_cl /c /obar /Fofoo -### -- %s 2>&1 | FileCheck -check-prefix=FooRACE1 %s
 // FooRACE1: "-o" "foo.obj"
Index: clang/test/Driver/ast.c
===
--- clang/test/Driver/ast.c
+++ clang/test/Driver/ast.c
@@ -25,3 +25,15 @@
 // FIXME: There is a problem with compiling AST's in that the input language is
 // not available for use by other tools (for example, to automatically add
 // -lstdc++). We may need -x [objective-]c++-ast and all that goodness. :(
+
+// Also check clang-cl since the driver is slightly different
+// RUN: %clang_cl -ccc-print-phases -emit-ast -- %s 2> %t
+// RUN: echo 'END' >> %t
+// RUN: FileCheck -check-prefix EMIT-AST-PHASES-CLANGCL -input-file %t %s
+
+// EMIT-AST-PHASES-CLANGCL: 0: input,
+// EM

[PATCH] D128409: [clang-cl] Add -emit-ast to clang-cl driver

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 440243.
thieta added a comment.

Updated commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128409

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/ast.c
  clang/test/Driver/cl-outputs.c


Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -82,6 +82,11 @@
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck 
-check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /otest.ast -###  -- %s  2>&1 | FileCheck 
-check-prefix=oASTNAME %s
+// oASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /otest.plist -###  -- %s  2>&1 | FileCheck 
-check-prefix=oPLIST %s
+// oPLIST:  "-o" "test.plist"
 
 // RUN: %clang_cl /c /obar /Fofoo -### -- %s 2>&1 | FileCheck 
-check-prefix=FooRACE1 %s
 // FooRACE1: "-o" "foo.obj"
Index: clang/test/Driver/ast.c
===
--- clang/test/Driver/ast.c
+++ clang/test/Driver/ast.c
@@ -25,3 +25,15 @@
 // FIXME: There is a problem with compiling AST's in that the input language is
 // not available for use by other tools (for example, to automatically add
 // -lstdc++). We may need -x [objective-]c++-ast and all that goodness. :(
+
+// Also check clang-cl since the driver is slightly different
+// RUN: %clang_cl -ccc-print-phases -emit-ast -- %s 2> %t
+// RUN: echo 'END' >> %t
+// RUN: FileCheck -check-prefix EMIT-AST-PHASES-CLANGCL -input-file %t %s
+
+// EMIT-AST-PHASES-CLANGCL: 0: input,
+// EMIT-AST-PHASES-CLANGCL: , c
+// EMIT-AST-PHASES-CLANGCL: 1: preprocessor, {0}, cpp-output
+// EMIT-AST-PHASES-CLANGCL: 2: compiler, {1}, ast
+// EMIT-AST-PHASES-CLANGCL-NOT: 3:
+// EMIT-AST-PHASES-CLANGCL: END
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5661,6 +5661,14 @@
 }
   } else if (JA.getType() == types::TY_PCH && IsCLMode()) {
 NamedOutput = C.getArgs().MakeArgString(GetClPchPath(C, BaseName));
+  } else if ((JA.getType() == types::TY_Plist || JA.getType() == 
types::TY_AST) &&
+ C.getArgs().hasArg(options::OPT__SLASH_o)) {
+StringRef Val =
+C.getArgs()
+.getLastArg(options::OPT__SLASH_o)
+->getValue();
+NamedOutput =
+MakeCLOutputFilename(C.getArgs(), Val, BaseName, types::TY_Object);
   } else {
 const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
 assert(Suffix && "All types used for output should have a suffix.");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1097,7 +1097,7 @@
 def dynamic : Flag<["-"], "dynamic">, Flags<[NoArgumentUnused]>;
 def d_Flag : Flag<["-"], "d">, Group;
 def d_Joined : Joined<["-"], "d">, Group;
-def emit_ast : Flag<["-"], "emit-ast">,
+def emit_ast : Flag<["-"], "emit-ast">, Flags<[CoreOption]>,
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option, FC1Option, 
FlangOption]>, Group,
   HelpText<"Use the LLVM representation for assembler and object files">;


Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -82,6 +82,11 @@
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck -check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /otest.ast -###  -- %s  2>&1 | FileCheck -check-prefix=oASTNAME %s
+// oASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /otest.plist -###  -- %s  2>&1 | FileCheck -check-prefix=oPLIST %s
+// oPLIST:  "-o" "test.plist"
 
 // RUN: %clang_cl /c /obar /Fofoo -### -- %s 2>&1 | FileCheck -check-prefix=FooRACE1 %s
 // FooRACE1: "-o" "foo.obj"
Index: clang/test/Driver/ast.c
===
--- clang/test/Driver/ast.c
+++ clang/test/Driver/ast.c
@@ -25,3 +25,15 @@
 // FIXME: There is a problem with compiling AST's in that the input language is
 // not available for use by other tools (for example, to automatically add
 // -lstdc++). We may need -x [objective-]c++-ast and all that goodness. :(
+
+// Also check clang-cl since the driver is slightly different
+// RUN: %clang_cl -ccc-print-phases -emit-ast -- %s 2> %t
+// RUN: echo 'END' >> %t
+// RUN: FileCheck -check-prefix EMIT-AST-PHASES-CLANGCL -input-file %t %s
+
+// EMIT-AST-PHASES-CLANGCL: 0: input,
+// EMIT-AST-PHASES-CL

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
thieta added reviewers: steakhal, martong, NoQ.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
thieta requested review of this revision.
Herald added a project: clang.

When doing CTU analysis setup you pre-compile .cpp to .ast and then
you run clang-extdef-mapping on the .cpp file as well. This is a
pretty slow process since we have to recompile the file each time.

With this patch you can now run clang-extdef-mapping directly on
the .ast file. That saves a lot of time.

I tried this on llvm/lib/AsmParser/Parser.cpp and running
extdef-mapping on the .cpp file took 5.4s on my machine.

While running it on the .ast file it took 2s.

This can save a lot of time for the setup phase of CTU analysis.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128704

Files:
  clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -1,4 +1,4 @@
-//===- ClangExtDefMapGen.cpp ---===//
+//===- ClangExtDefMapGen.cpp -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,10 +13,12 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
@@ -29,12 +31,16 @@
 using namespace clang::cross_tu;
 using namespace clang::tooling;
 
-static cl::OptionCategory ClangExtDefMapGenCategory("clang-extdefmapgen options");
+static cl::OptionCategory
+ClangExtDefMapGenCategory("clang-extdefmapgen options");
 
 class MapExtDefNamesConsumer : public ASTConsumer {
 public:
-  MapExtDefNamesConsumer(ASTContext &Context)
-  : Ctx(Context), SM(Context.getSourceManager()) {}
+  MapExtDefNamesConsumer(ASTContext &Context,
+ StringRef astFilePath = StringRef())
+  : Ctx(Context), SM(Context.getSourceManager()) {
+CurrentFileName = astFilePath.str();
+  }
 
   ~MapExtDefNamesConsumer() {
 // Flush results to standard output.
@@ -111,6 +117,46 @@
 
 static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 
+void HandleAST(StringRef astPath) {
+
+  CompilerInstance CI;
+
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  TextDiagnosticPrinter *DiagClient =
+  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  IntrusiveRefCntPtr Diags(
+  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
+
+  std::unique_ptr unit = ASTUnit::LoadFromASTFile(
+  astPath.str(), CI.getPCHContainerOperations()->getRawReader(),
+  ASTUnit::LoadASTOnly, Diags, CI.getFileSystemOpts());
+
+  FileManager fm(CI.getFileSystemOpts());
+  SmallString<128> absPath(astPath);
+  fm.makeAbsolutePath(absPath);
+
+  std::unique_ptr consumer =
+  std::make_unique(unit->getASTContext(), absPath);
+  consumer->HandleTranslationUnit(unit->getASTContext());
+}
+
+void HandleFiles(ArrayRef sourceFiles,
+ CompilationDatabase &compilations) {
+  std::vector sourceToBeParsed;
+  for (StringRef src : sourceFiles) {
+if (src.endswith(".ast"))
+  HandleAST(src);
+else
+  sourceToBeParsed.push_back(src.str());
+  }
+
+  if (!sourceToBeParsed.empty()) {
+ClangTool Tool(compilations, sourceToBeParsed);
+Tool.run(newFrontendActionFactory().get());
+  }
+}
+
 int main(int argc, const char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0], false);
@@ -127,8 +173,6 @@
   }
   CommonOptionsParser &OptionsParser = ExpectedParser.get();
 
-  ClangTool Tool(OptionsParser.getCompilations(),
- OptionsParser.getSourcePathList());
-
-  return Tool.run(newFrontendActionFactory().get());
+  HandleFiles(OptionsParser.getSourcePathList(),
+  OptionsParser.getCompilations());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

This still needs tests - but I wanted to get your early input on the approach 
here and what you all think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

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

[PATCH] D128409: [clang-cl] Add -emit-ast to clang-cl driver

[Tobias Hieta via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3f0578dd87ee: [clang-cl] Add -emit-ast to clang-cl driver 
(authored by thieta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128409

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/ast.c
  clang/test/Driver/cl-outputs.c


Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -82,6 +82,11 @@
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck 
-check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /otest.ast -###  -- %s  2>&1 | FileCheck 
-check-prefix=oASTNAME %s
+// oASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /otest.plist -###  -- %s  2>&1 | FileCheck 
-check-prefix=oPLIST %s
+// oPLIST:  "-o" "test.plist"
 
 // RUN: %clang_cl /c /obar /Fofoo -### -- %s 2>&1 | FileCheck 
-check-prefix=FooRACE1 %s
 // FooRACE1: "-o" "foo.obj"
Index: clang/test/Driver/ast.c
===
--- clang/test/Driver/ast.c
+++ clang/test/Driver/ast.c
@@ -25,3 +25,15 @@
 // FIXME: There is a problem with compiling AST's in that the input language is
 // not available for use by other tools (for example, to automatically add
 // -lstdc++). We may need -x [objective-]c++-ast and all that goodness. :(
+
+// Also check clang-cl since the driver is slightly different
+// RUN: %clang_cl -ccc-print-phases -emit-ast -- %s 2> %t
+// RUN: echo 'END' >> %t
+// RUN: FileCheck -check-prefix EMIT-AST-PHASES-CLANGCL -input-file %t %s
+
+// EMIT-AST-PHASES-CLANGCL: 0: input,
+// EMIT-AST-PHASES-CLANGCL: , c
+// EMIT-AST-PHASES-CLANGCL: 1: preprocessor, {0}, cpp-output
+// EMIT-AST-PHASES-CLANGCL: 2: compiler, {1}, ast
+// EMIT-AST-PHASES-CLANGCL-NOT: 3:
+// EMIT-AST-PHASES-CLANGCL: END
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5661,6 +5661,14 @@
 }
   } else if (JA.getType() == types::TY_PCH && IsCLMode()) {
 NamedOutput = C.getArgs().MakeArgString(GetClPchPath(C, BaseName));
+  } else if ((JA.getType() == types::TY_Plist || JA.getType() == 
types::TY_AST) &&
+ C.getArgs().hasArg(options::OPT__SLASH_o)) {
+StringRef Val =
+C.getArgs()
+.getLastArg(options::OPT__SLASH_o)
+->getValue();
+NamedOutput =
+MakeCLOutputFilename(C.getArgs(), Val, BaseName, types::TY_Object);
   } else {
 const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
 assert(Suffix && "All types used for output should have a suffix.");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1097,7 +1097,7 @@
 def dynamic : Flag<["-"], "dynamic">, Flags<[NoArgumentUnused]>;
 def d_Flag : Flag<["-"], "d">, Group;
 def d_Joined : Joined<["-"], "d">, Group;
-def emit_ast : Flag<["-"], "emit-ast">,
+def emit_ast : Flag<["-"], "emit-ast">, Flags<[CoreOption]>,
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option, FC1Option, 
FlangOption]>, Group,
   HelpText<"Use the LLVM representation for assembler and object files">;


Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -82,6 +82,11 @@
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck -check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /otest.ast -###  -- %s  2>&1 | FileCheck -check-prefix=oASTNAME %s
+// oASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /otest.plist -###  -- %s  2>&1 | FileCheck -check-prefix=oPLIST %s
+// oPLIST:  "-o" "test.plist"
 
 // RUN: %clang_cl /c /obar /Fofoo -### -- %s 2>&1 | FileCheck -check-prefix=FooRACE1 %s
 // FooRACE1: "-o" "foo.obj"
Index: clang/test/Driver/ast.c
===
--- clang/test/Driver/ast.c
+++ clang/test/Driver/ast.c
@@ -25,3 +25,15 @@
 // FIXME: There is a problem with compiling AST's in that the input language is
 // not available for use by other tools (for example, to automatically add
 // -lstdc++). We may need -x [objective-]c++-ast and all that goodness. :(
+
+// Also check clang-cl since the driver is slightly different
+// RUN: %clang_cl -ccc-print-phases -emit-ast -- %s 2> %t
+// RUN: echo 'END' >> %t
+/

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 440558.
thieta added a comment.

Add test. Just repurpose the same test we already have but add a step to
generate ast first and then pushing that through extdef-mapping. It should
always produce the same result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

Files:
  clang/test/Analysis/func-mapping-test.cpp
  clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -1,4 +1,4 @@
-//===- ClangExtDefMapGen.cpp ---===//
+//===- ClangExtDefMapGen.cpp -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,10 +13,12 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
@@ -29,12 +31,16 @@
 using namespace clang::cross_tu;
 using namespace clang::tooling;
 
-static cl::OptionCategory ClangExtDefMapGenCategory("clang-extdefmapgen options");
+static cl::OptionCategory
+ClangExtDefMapGenCategory("clang-extdefmapgen options");
 
 class MapExtDefNamesConsumer : public ASTConsumer {
 public:
-  MapExtDefNamesConsumer(ASTContext &Context)
-  : Ctx(Context), SM(Context.getSourceManager()) {}
+  MapExtDefNamesConsumer(ASTContext &Context,
+ StringRef astFilePath = StringRef())
+  : Ctx(Context), SM(Context.getSourceManager()) {
+CurrentFileName = astFilePath.str();
+  }
 
   ~MapExtDefNamesConsumer() {
 // Flush results to standard output.
@@ -111,6 +117,46 @@
 
 static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 
+void HandleAST(StringRef astPath) {
+
+  CompilerInstance CI;
+
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  TextDiagnosticPrinter *DiagClient =
+  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  IntrusiveRefCntPtr Diags(
+  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
+
+  std::unique_ptr unit = ASTUnit::LoadFromASTFile(
+  astPath.str(), CI.getPCHContainerOperations()->getRawReader(),
+  ASTUnit::LoadASTOnly, Diags, CI.getFileSystemOpts());
+
+  FileManager fm(CI.getFileSystemOpts());
+  SmallString<128> absPath(astPath);
+  fm.makeAbsolutePath(absPath);
+
+  std::unique_ptr consumer =
+  std::make_unique(unit->getASTContext(), absPath);
+  consumer->HandleTranslationUnit(unit->getASTContext());
+}
+
+void HandleFiles(ArrayRef sourceFiles,
+ CompilationDatabase &compilations) {
+  std::vector sourceToBeParsed;
+  for (StringRef src : sourceFiles) {
+if (src.endswith(".ast"))
+  HandleAST(src);
+else
+  sourceToBeParsed.push_back(src.str());
+  }
+
+  if (!sourceToBeParsed.empty()) {
+ClangTool Tool(compilations, sourceToBeParsed);
+Tool.run(newFrontendActionFactory().get());
+  }
+}
+
 int main(int argc, const char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0], false);
@@ -127,8 +173,6 @@
   }
   CommonOptionsParser &OptionsParser = ExpectedParser.get();
 
-  ClangTool Tool(OptionsParser.getCompilations(),
- OptionsParser.getSourcePathList());
-
-  return Tool.run(newFrontendActionFactory().get());
+  HandleFiles(OptionsParser.getSourcePathList(),
+  OptionsParser.getCompilations());
 }
Index: clang/test/Analysis/func-mapping-test.cpp
===
--- clang/test/Analysis/func-mapping-test.cpp
+++ clang/test/Analysis/func-mapping-test.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_extdef_map %s -- | FileCheck --implicit-check-not "c:@y" --implicit-check-not "c:@z" %s
+// RUN: %clang -emit-ast %s -o %t.ast
+// RUN: %clang_extdef_map %t.ast -- | FileCheck --implicit-check-not "c:@y" --implicit-check-not "c:@z" %s
 
 int f(int) {
   return 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:122
+
+  CompilerInstance CI;
+

Not sure if I can just create a compilerinstance here and if I should feed it 
anymore data then I already do.



Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:135
+
+  FileManager fm(CI.getFileSystemOpts());
+  SmallString<128> absPath(astPath);

For some reason CI.getFileManager() often returns no filemanager - but 
sometimes it works? I just ended up creating my own to make the path absolute.



Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:136
+  FileManager fm(CI.getFileSystemOpts());
+  SmallString<128> absPath(astPath);
+  fm.makeAbsolutePath(absPath);

Pretty sure 128 is wrong here - but I searched the codebase and that seems to 
be the most common size used? I couldn't find anything using PATH_MAX or 
something like that.



Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:141
+  std::make_unique(unit->getASTContext(), absPath);
+  consumer->HandleTranslationUnit(unit->getASTContext());
+}

I tried to figure out how to "run" the consumer on a already loaded ASTUnit - 
but I didn't find anything in the api for that and this way works since it's 
more or less what will happen when the consumer is executed anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

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

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:136
+  FileManager fm(CI.getFileSystemOpts());
+  SmallString<128> absPath(astPath);
+  fm.makeAbsolutePath(absPath);

thieta wrote:
> Pretty sure 128 is wrong here - but I searched the codebase and that seems to 
> be the most common size used? I couldn't find anything using PATH_MAX or 
> something like that.
Never mind 128 is just the default size not the absolute size. Got it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

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

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 441279.
thieta added a comment.

Clean-ups and error handling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

Files:
  clang/test/Analysis/func-mapping-test.cpp
  clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -1,4 +1,4 @@
-//===- ClangExtDefMapGen.cpp ---===//
+//===- ClangExtDefMapGen.cpp -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,10 +13,12 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
@@ -29,12 +31,16 @@
 using namespace clang::cross_tu;
 using namespace clang::tooling;
 
-static cl::OptionCategory ClangExtDefMapGenCategory("clang-extdefmapgen options");
+static cl::OptionCategory
+ClangExtDefMapGenCategory("clang-extdefmapgen options");
 
 class MapExtDefNamesConsumer : public ASTConsumer {
 public:
-  MapExtDefNamesConsumer(ASTContext &Context)
-  : Ctx(Context), SM(Context.getSourceManager()) {}
+  MapExtDefNamesConsumer(ASTContext &Context,
+ StringRef astFilePath = StringRef())
+  : Ctx(Context), SM(Context.getSourceManager()) {
+CurrentFileName = astFilePath.str();
+  }
 
   ~MapExtDefNamesConsumer() {
 // Flush results to standard output.
@@ -111,6 +117,56 @@
 
 static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 
+static bool HandleAST(StringRef astPath) {
+
+  CompilerInstance CI;
+
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  TextDiagnosticPrinter *DiagClient =
+  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  IntrusiveRefCntPtr Diags(
+  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
+
+  std::unique_ptr unit = ASTUnit::LoadFromASTFile(
+  astPath.str(), CI.getPCHContainerOperations()->getRawReader(),
+  ASTUnit::LoadASTOnly, Diags, CI.getFileSystemOpts());
+
+  if (!unit)
+return false;
+
+  FileManager fm(CI.getFileSystemOpts());
+  SmallString<128> absPath(astPath);
+  fm.makeAbsolutePath(absPath);
+
+  std::unique_ptr consumer =
+  std::make_unique(unit->getASTContext(), absPath);
+  consumer->HandleTranslationUnit(unit->getASTContext());
+
+  return true;
+}
+
+static int HandleFiles(ArrayRef sourceFiles,
+   CompilationDatabase &compilations) {
+  std::vector sourceToBeParsed;
+  for (StringRef src : sourceFiles) {
+if (src.endswith(".ast")) {
+  if (!HandleAST(src)) {
+return 1;
+  }
+} else {
+  sourceToBeParsed.push_back(src.str());
+}
+  }
+
+  if (!sourceToBeParsed.empty()) {
+ClangTool Tool(compilations, sourceToBeParsed);
+return Tool.run(newFrontendActionFactory().get());
+  }
+
+  return 0;
+}
+
 int main(int argc, const char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0], false);
@@ -127,8 +183,6 @@
   }
   CommonOptionsParser &OptionsParser = ExpectedParser.get();
 
-  ClangTool Tool(OptionsParser.getCompilations(),
- OptionsParser.getSourcePathList());
-
-  return Tool.run(newFrontendActionFactory().get());
+  return HandleFiles(OptionsParser.getSourcePathList(),
+ OptionsParser.getCompilations());
 }
Index: clang/test/Analysis/func-mapping-test.cpp
===
--- clang/test/Analysis/func-mapping-test.cpp
+++ clang/test/Analysis/func-mapping-test.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_extdef_map %s -- | FileCheck --implicit-check-not "c:@y" --implicit-check-not "c:@z" %s
+// RUN: %clang -emit-ast %s -o %t.ast
+// RUN: %clang_extdef_map %t.ast -- | FileCheck --implicit-check-not "c:@y" --implicit-check-not "c:@z" %s
 
 int f(int) {
   return 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 441930.
thieta marked 6 inline comments as done.
thieta added a comment.

Addressed review feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/Analysis/func-mapping-test.cpp
  clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -1,4 +1,4 @@
-//===- ClangExtDefMapGen.cpp ---===//
+//===- ClangExtDefMapGen.cpp -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,10 +13,12 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
@@ -29,12 +31,16 @@
 using namespace clang::cross_tu;
 using namespace clang::tooling;
 
-static cl::OptionCategory ClangExtDefMapGenCategory("clang-extdefmapgen options");
+static cl::OptionCategory
+ClangExtDefMapGenCategory("clang-extdefmapgen options");
 
 class MapExtDefNamesConsumer : public ASTConsumer {
 public:
-  MapExtDefNamesConsumer(ASTContext &Context)
-  : Ctx(Context), SM(Context.getSourceManager()) {}
+  MapExtDefNamesConsumer(ASTContext &Context,
+ StringRef astFilePath = StringRef())
+  : Ctx(Context), SM(Context.getSourceManager()) {
+CurrentFileName = astFilePath.str();
+  }
 
   ~MapExtDefNamesConsumer() {
 // Flush results to standard output.
@@ -111,6 +117,82 @@
 
 static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 
+static IntrusiveRefCntPtr Diags;
+
+IntrusiveRefCntPtr GetDiagnosticsEngine() {
+  if (Diags) {
+// Call reset to make sure we don't mix errors
+Diags->Reset(false);
+return Diags;
+  }
+
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  TextDiagnosticPrinter *DiagClient =
+  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  DiagClient->setPrefix("clang-extdef-mappping");
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+
+  IntrusiveRefCntPtr DiagEngine(
+  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
+  Diags.swap(DiagEngine);
+
+  // Retain this one time so it's not destroyed by ASTUnit::LoadFromASTFile
+  Diags->Retain();
+  return Diags;
+}
+
+static CompilerInstance *CI = nullptr;
+
+static bool HandleAST(StringRef astPath) {
+
+  if (!CI)
+CI = new CompilerInstance();
+
+  IntrusiveRefCntPtr DiagEngine = GetDiagnosticsEngine();
+
+  std::unique_ptr Unit = ASTUnit::LoadFromASTFile(
+  astPath.str(), CI->getPCHContainerOperations()->getRawReader(),
+  ASTUnit::LoadASTOnly, DiagEngine, CI->getFileSystemOpts());
+
+  if (!Unit)
+return false;
+
+  FileManager fm(CI->getFileSystemOpts());
+  SmallString<128> absPath(astPath);
+  fm.makeAbsolutePath(absPath);
+
+  MapExtDefNamesConsumer Consumer =
+  MapExtDefNamesConsumer(Unit->getASTContext(), absPath);
+  Consumer.HandleTranslationUnit(Unit->getASTContext());
+
+  return true;
+}
+
+static int HandleFiles(ArrayRef sourceFiles,
+   CompilationDatabase &compilations) {
+  std::vector SourcesToBeParsed;
+
+  // Loop over all input files, if they are pre-compiled AST
+  // process them directly in HandleAST, otherwise put them
+  // on a list for ClangTool to handle.
+  for (StringRef Src : sourceFiles) {
+if (Src.endswith(".ast")) {
+  if (!HandleAST(Src)) {
+return 1;
+  }
+} else {
+  SourcesToBeParsed.push_back(Src.str());
+}
+  }
+
+  if (!SourcesToBeParsed.empty()) {
+ClangTool Tool(compilations, SourcesToBeParsed);
+return Tool.run(newFrontendActionFactory().get());
+  }
+
+  return 0;
+}
+
 int main(int argc, const char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0], false);
@@ -118,7 +200,10 @@
 
   const char *Overview = "\nThis tool collects the USR name and location "
  "of external definitions in the source files "
- "(excluding headers).\n";
+ "(excluding headers).\n"
+ "Input can be either source files that are compiled "
+ "with compile database or .ast files that are "
+

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

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

Thanks for the review! I uploaded a new version addressing all (I think) of 
your feedback and added a release note.

The help text is a bit weird since it's using a commonoptionsparser we can't 
really change the text to add `` as a specific input and we would have to 
create some kind of extension point in the parser or something. Not sure what 
you feel about this issue.

I will try to do some extensive testing - but I need to figure out a way to do 
all that ast building and extdef dumping in a automated way from the 
compile_commands database. I might run it on some internal code instead since I 
have a setup for that. Meanwhile let's make sure the code is in good shape.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

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

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Ran some internal benchmarks on 1570 files (C and C++ mixed but much more C++ 
than C):

Running clang-extdef-mapping on the source files took: 268s 
Running clang-extdef-mapping on the AST: 102s

That's quite a large speed up if you already need to generate the AST files.

I'll fix the variable names and then merge this one unless someone complains.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

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

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 442211.
thieta marked 3 inline comments as done.
thieta added a comment.

Uppercase all variables


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/Analysis/func-mapping-test.cpp
  clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -1,4 +1,4 @@
-//===- ClangExtDefMapGen.cpp ---===//
+//===- ClangExtDefMapGen.cpp -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,10 +13,12 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
@@ -29,12 +31,16 @@
 using namespace clang::cross_tu;
 using namespace clang::tooling;
 
-static cl::OptionCategory ClangExtDefMapGenCategory("clang-extdefmapgen options");
+static cl::OptionCategory
+ClangExtDefMapGenCategory("clang-extdefmapgen options");
 
 class MapExtDefNamesConsumer : public ASTConsumer {
 public:
-  MapExtDefNamesConsumer(ASTContext &Context)
-  : Ctx(Context), SM(Context.getSourceManager()) {}
+  MapExtDefNamesConsumer(ASTContext &Context,
+ StringRef astFilePath = StringRef())
+  : Ctx(Context), SM(Context.getSourceManager()) {
+CurrentFileName = astFilePath.str();
+  }
 
   ~MapExtDefNamesConsumer() {
 // Flush results to standard output.
@@ -111,6 +117,82 @@
 
 static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 
+static IntrusiveRefCntPtr Diags;
+
+IntrusiveRefCntPtr GetDiagnosticsEngine() {
+  if (Diags) {
+// Call reset to make sure we don't mix errors
+Diags->Reset(false);
+return Diags;
+  }
+
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  TextDiagnosticPrinter *DiagClient =
+  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  DiagClient->setPrefix("clang-extdef-mappping");
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+
+  IntrusiveRefCntPtr DiagEngine(
+  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
+  Diags.swap(DiagEngine);
+
+  // Retain this one time so it's not destroyed by ASTUnit::LoadFromASTFile
+  Diags->Retain();
+  return Diags;
+}
+
+static CompilerInstance *CI = nullptr;
+
+static bool HandleAST(StringRef AstPath) {
+
+  if (!CI)
+CI = new CompilerInstance();
+
+  IntrusiveRefCntPtr DiagEngine = GetDiagnosticsEngine();
+
+  std::unique_ptr Unit = ASTUnit::LoadFromASTFile(
+  AstPath.str(), CI->getPCHContainerOperations()->getRawReader(),
+  ASTUnit::LoadASTOnly, DiagEngine, CI->getFileSystemOpts());
+
+  if (!Unit)
+return false;
+
+  FileManager FM(CI->getFileSystemOpts());
+  SmallString<128> AbsPath(AstPath);
+  FM.makeAbsolutePath(AbsPath);
+
+  MapExtDefNamesConsumer Consumer =
+  MapExtDefNamesConsumer(Unit->getASTContext(), AbsPath);
+  Consumer.HandleTranslationUnit(Unit->getASTContext());
+
+  return true;
+}
+
+static int HandleFiles(ArrayRef SourceFiles,
+   CompilationDatabase &compilations) {
+  std::vector SourcesToBeParsed;
+
+  // Loop over all input files, if they are pre-compiled AST
+  // process them directly in HandleAST, otherwise put them
+  // on a list for ClangTool to handle.
+  for (StringRef Src : SourceFiles) {
+if (Src.endswith(".ast")) {
+  if (!HandleAST(Src)) {
+return 1;
+  }
+} else {
+  SourcesToBeParsed.push_back(Src.str());
+}
+  }
+
+  if (!SourcesToBeParsed.empty()) {
+ClangTool Tool(compilations, SourcesToBeParsed);
+return Tool.run(newFrontendActionFactory().get());
+  }
+
+  return 0;
+}
+
 int main(int argc, const char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0], false);
@@ -118,7 +200,10 @@
 
   const char *Overview = "\nThis tool collects the USR name and location "
  "of external definitions in the source files "
- "(excluding headers).\n";
+ "(excluding headers).\n"
+ "Input can be either source files that are compiled "
+ "with compile database or .ast files that are "
+   

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe6ff553979e8: [clang-extdef-mapping] Directly process .ast 
files (authored by thieta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/Analysis/func-mapping-test.cpp
  clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -1,4 +1,4 @@
-//===- ClangExtDefMapGen.cpp ---===//
+//===- ClangExtDefMapGen.cpp -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,10 +13,12 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
@@ -29,12 +31,16 @@
 using namespace clang::cross_tu;
 using namespace clang::tooling;
 
-static cl::OptionCategory ClangExtDefMapGenCategory("clang-extdefmapgen options");
+static cl::OptionCategory
+ClangExtDefMapGenCategory("clang-extdefmapgen options");
 
 class MapExtDefNamesConsumer : public ASTConsumer {
 public:
-  MapExtDefNamesConsumer(ASTContext &Context)
-  : Ctx(Context), SM(Context.getSourceManager()) {}
+  MapExtDefNamesConsumer(ASTContext &Context,
+ StringRef astFilePath = StringRef())
+  : Ctx(Context), SM(Context.getSourceManager()) {
+CurrentFileName = astFilePath.str();
+  }
 
   ~MapExtDefNamesConsumer() {
 // Flush results to standard output.
@@ -111,6 +117,82 @@
 
 static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 
+static IntrusiveRefCntPtr Diags;
+
+IntrusiveRefCntPtr GetDiagnosticsEngine() {
+  if (Diags) {
+// Call reset to make sure we don't mix errors
+Diags->Reset(false);
+return Diags;
+  }
+
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  TextDiagnosticPrinter *DiagClient =
+  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  DiagClient->setPrefix("clang-extdef-mappping");
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+
+  IntrusiveRefCntPtr DiagEngine(
+  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
+  Diags.swap(DiagEngine);
+
+  // Retain this one time so it's not destroyed by ASTUnit::LoadFromASTFile
+  Diags->Retain();
+  return Diags;
+}
+
+static CompilerInstance *CI = nullptr;
+
+static bool HandleAST(StringRef AstPath) {
+
+  if (!CI)
+CI = new CompilerInstance();
+
+  IntrusiveRefCntPtr DiagEngine = GetDiagnosticsEngine();
+
+  std::unique_ptr Unit = ASTUnit::LoadFromASTFile(
+  AstPath.str(), CI->getPCHContainerOperations()->getRawReader(),
+  ASTUnit::LoadASTOnly, DiagEngine, CI->getFileSystemOpts());
+
+  if (!Unit)
+return false;
+
+  FileManager FM(CI->getFileSystemOpts());
+  SmallString<128> AbsPath(AstPath);
+  FM.makeAbsolutePath(AbsPath);
+
+  MapExtDefNamesConsumer Consumer =
+  MapExtDefNamesConsumer(Unit->getASTContext(), AbsPath);
+  Consumer.HandleTranslationUnit(Unit->getASTContext());
+
+  return true;
+}
+
+static int HandleFiles(ArrayRef SourceFiles,
+   CompilationDatabase &compilations) {
+  std::vector SourcesToBeParsed;
+
+  // Loop over all input files, if they are pre-compiled AST
+  // process them directly in HandleAST, otherwise put them
+  // on a list for ClangTool to handle.
+  for (StringRef Src : SourceFiles) {
+if (Src.endswith(".ast")) {
+  if (!HandleAST(Src)) {
+return 1;
+  }
+} else {
+  SourcesToBeParsed.push_back(Src.str());
+}
+  }
+
+  if (!SourcesToBeParsed.empty()) {
+ClangTool Tool(compilations, SourcesToBeParsed);
+return Tool.run(newFrontendActionFactory().get());
+  }
+
+  return 0;
+}
+
 int main(int argc, const char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0], false);
@@ -118,7 +200,10 @@
 
   const char *Overview = "\nThis tool collects the USR name and location "
  "of external definitions in the source files "
- "(excluding headers).\n";
+ "(excluding headers).\n"
+ "Input can be either source files that are compiled

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Isn't it better to silence this warning with a pragma instead of disabling it 
for the whole file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129152

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

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

[Tobias Hieta via Phabricator via cfe-commits]

thieta accepted this revision.
thieta added a comment.

Thanks for switching to pragma!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129152

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

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:42
+  : Ctx(Context), SM(Context.getSourceManager()) {
+CurrentFileName = astFilePath.str();
+  }

steakhal wrote:
> Why is this not initialized in the //initialized-list// like the rest of the 
> members?
Ah no reason - I think I moved this around a few times so it just happened to 
end up here. I will push a NFC with that.



Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:149
+  if (!CI)
+CI = new CompilerInstance();
+

steakhal wrote:
> What takes the ownership of `CI`? When is it deleted?
I don't think anyone takes ownership and it's never properly deleted. I don't 
think we really need to since this application just runs and exits and never 
really keep any state. Do you see a problem with it never being deleted?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

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

[PATCH] D122298: [clang-cl] Ignore /Wv and /Wv:17 flags

[Tobias Hieta via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG67d9276b1602: [clang-cl] Ignore /Wv and /Wv:17 flags 
(authored by thieta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122298

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -396,6 +396,8 @@
 // RUN:/volatile:iso \
 // RUN:/w12345 \
 // RUN:/wd1234 \
+// RUN:/Wv \
+// RUN:/Wv:17 \
 // RUN:/Zc:__cplusplus \
 // RUN:/Zc:auto \
 // RUN:/Zc:forScope \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6530,6 +6530,7 @@
 def _SLASH_utf8 : CLIgnoredFlag<"utf-8">,
   HelpText<"Set source and runtime encoding to UTF-8 (default)">;
 def _SLASH_w : CLIgnoredJoined<"w">;
+def _SLASH_Wv_ : CLIgnoredJoined<"Wv">;
 def _SLASH_Zc___cplusplus : CLIgnoredFlag<"Zc:__cplusplus">;
 def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
 def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -396,6 +396,8 @@
 // RUN:/volatile:iso \
 // RUN:/w12345 \
 // RUN:/wd1234 \
+// RUN:/Wv \
+// RUN:/Wv:17 \
 // RUN:/Zc:__cplusplus \
 // RUN:/Zc:auto \
 // RUN:/Zc:forScope \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6530,6 +6530,7 @@
 def _SLASH_utf8 : CLIgnoredFlag<"utf-8">,
   HelpText<"Set source and runtime encoding to UTF-8 (default)">;
 def _SLASH_w : CLIgnoredJoined<"w">;
+def _SLASH_Wv_ : CLIgnoredJoined<"Wv">;
 def _SLASH_Zc___cplusplus : CLIgnoredFlag<"Zc:__cplusplus">;
 def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
 def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
thieta added reviewers: mehdi_amini, jyknight, jhenderson, hans, tstellar, 
cor3ntin, MaskRay.
Herald added subscribers: ayermolo, StephenFan, mgorny.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
thieta requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, yota9.
Herald added projects: clang, LLDB, LLVM.

Also make the soft toolchain requirements hard. This allows
us to use C++17 features in LLVM now. Remember that if we want
to adopt some new feature in a bigger way it should be discussed
and added to the CodingStandard document.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,7 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,21 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
 # _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
 # Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -79,20 +79,14 @@
 # Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
 # Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
-#include 
-#if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
-#error Unsupported libstdc++ version
-#endif
-#endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
-"
+  #include 
+  #if defined(__GLIBCXX__)
+  #if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
+  #error Unsupported libstdc++ version
+  #endif
+  #endif
+  int main() { return 0; }
+  "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
   message(FATAL_ERROR "libstdc++ version must be at least ${GCC_MIN}.")
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists

[PATCH] D129446: [clang][driver] Find Apple default SDK path

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Ping - any thoughts on this one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129446

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3684330 , @mehdi_amini 
wrote:

> What does it mean exactly? We can't use **anything** C++17 without writing it 
> in the coding standards?
> I'm not sure it'll be manageable: how do you see this playing out?

Probably poorly worded - what I was trying to convey is that if we want to use 
a C++17 feature that's really impactful on the syntax/readability we should 
probably consider recommending ways to use it in the coding standards, similar 
to our guidelines on using for() loops 
(https://llvm.org/docs/CodingStandards.html#use-range-based-for-loops-wherever-possible)
 or the auto keyword 
(https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 448261.
thieta added a comment.

Address some old comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,7 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,19 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
-# _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
-# Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -76,23 +74,15 @@
 set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
-# Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
-# Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
-#include 
-#if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
-#error Unsupported libstdc++ version
-#endif
-#endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
-"
+  #include 
+  #if defined(__GLIBCXX__)
+  #if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
+  #error Unsupported libstdc++ version
+  #endif
+  #endif
+  int main() { return 0; }
+  "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
   message(FATAL_ERROR "libstdc++ version must be at least ${GCC_MIN}.")
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -20,7 +20,7 @@
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
-  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
   set(CMAKE_CXX_STANDARD_REQUIRED YES)
   set(CMAKE_CXX_EXTENSIONS NO)
 endif()

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3684360 , @barannikov88 
wrote:

> There are a few places (primarily in ADT and Support) that check __cplusplus 
> > 201402. Do they need to be cleaned up?

Sounds good - but maybe that can be addressed in a separate diff unless they 
make the build fail after this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3684334 , @ChuanqiXu wrote:

> So it is free that developers want to use some C++17 features in a small 
> amount of code, right?

As soon as this land C++ 17 should be free to use everywhere yeah!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 448262.
thieta added a comment.

Fixed unintended indentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,7 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,19 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
-# _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
-# Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -76,22 +74,14 @@
 set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
-# Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
-# Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
 #include 
 #if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
+#if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
 #error Unsupported libstdc++ version
 #endif
 #endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
+int main() { return 0; }
 "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -20,7 +20,7 @@
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
-  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
   set(CMAKE_CXX_STANDARD_REQUIRED YES)
   set(CMAKE_CXX_EXTENSIONS NO)
 endif()
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -11,7 +11,7 @@
 include(GNUInstallDirs)

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3686397 , @thakis wrote:

> It'd be nice if this was landed opt-in behind some cmake variable at first, 
> so that folks could try it out on their bots and see how well things work.

You can already test this with `-DCMAKE_CXX_STANDARD=17` afaik. I wonder how 
many bot owners would actually test this if we made another flag available.

Since we can already test this with a current flag maybe we should just post to 
discourse that bot-owners can test it and a date when this will be merged. But 
I don't expect this to be a big problem, when we merged the soft error earlier 
this year - it only broke one bot which was out of date.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3684399 , @h-vetinari 
wrote:

> It may be worth calling out that this is about C++17 core language and not 
> the standard library?
>
> libstdcxx only finished C++17 support in GCC 12, and libcxx is still missing 
> various pieces even today (much less for Clang 5).

I will add a small line about this in the coding standards document.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3686718 , @thieta wrote:

> In D130689#3684399 , @h-vetinari 
> wrote:
>
>> It may be worth calling out that this is about C++17 core language and not 
>> the standard library?
>>
>> libstdcxx only finished C++17 support in GCC 12, and libcxx is still missing 
>> various pieces even today (much less for Clang 5).
>
> I will add a small line about this in the coding standards document.

Actually - never mind this is already well documented in the coding standards 
document:

  Unless otherwise documented, LLVM subprojects are written using standard C++17
  code and avoid unnecessary vendor-specific extensions.
  
  Nevertheless, we restrict ourselves to features which are available in the
  major toolchains supported as host compilers (see :doc:`GettingStarted` page,
  section `Software`).
  
  Each toolchain provides a good reference for what it accepts:
  
  * Clang: https://clang.llvm.org/cxx_status.html
  * GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
  * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx

I feel that's good enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3686723 , @h-vetinari 
wrote:

> I don't think the standard library can be called a vendor-specific extension, 
> and so I think this still could/should be made clearer (even though the 
> status links below would show upon inspection that there's no full support 
> yet, but that's a bit too tucked away I feel).

For libstdc++ and libc++ there are also pages with status in a specific 
version. Maybe we should link those matrixes as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3689157 , @thakis wrote:

> Is it expected and intentional that this increases the mac deployment target 
> to 10.12?

I wasn't aware of that - but I think it's expected since the check in RWMutex 
checks for the C++ standard and doesn't care about the deployment target for 
macOS. It seems pretty easy to change, but I wonder if that matters? 10.12 was 
released in 2016 so it's pretty old and this wouldn't affect most users of LLVM.

My gut feeling say that we should be fine with requiring 10.12 and if that 
becomes a big problem during the development phase someone could propose a 
patch to improve the check in RWMutex.

But in that case we should probably have a check for the deployment target as 
part of the cmake config and error if CXX_STANDARD > 17 and 
OSX_DEPLOYMENT_TARGET < 10.12.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3690274 , @nikic wrote:

> Given 
> https://github.com/llvm/llvm-project/blob/2bb7c54621f31a957302a4deb3d25b752acb07bd/llvm/include/llvm/Support/RWMutex.h#L22-L27,
>  it seems like this is supposed to be supported. This is probably just a 
> matter of moving the shared_mutex use behind the LLVM_USE_RW_MUTEX_IMPL guard?

Yeah - I just realized that when I checked this. Just building the rest of the 
tree now to confirm this is the only change we need and I will publish this a 
different diff first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

@nikic @thakis I fixed this issue in https://reviews.llvm.org/D131063 and it 
can be built with CXX_STANDARD=17 and OSX_DEPLOYMENT_TARGET=10.11.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

All right! Last call - I am going fix the spelling error and merge this 
tomorrow EU time unless someone objects. Thanks for all reviews so far!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 450364.
thieta added a comment.

Fixed spelling error and added links to C++ standard libraries


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to suppress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,13 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+
+  * libc++: https://libcxx.llvm.org/Status/Cxx17.html
+
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
+
+  * libstdc++: https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.2017
+
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,19 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
-# _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
-# Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -76,22 +74,14 @@
 set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
-# Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
-# Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
 #include 
 #if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
+#if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
 #error Unsupported libstdc++ version
 #endif
 #endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
+int main() { return 0; }
 "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -20,7 +20,7 @@
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
-  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
   set(CMAKE_CXX_STANDARD_REQUIRED YES)
   set(CMAKE_CXX_EXTENSIONS NO)
 

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb1356504e63a: [LLVM] Update C++ standard to 17 (authored by 
thieta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to suppress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,13 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+
+  * libc++: https://libcxx.llvm.org/Status/Cxx17.html
+
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
+
+  * libstdc++: https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.2017
+
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,19 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
-# _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
-# Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -76,22 +74,14 @@
 set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
-# Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
-# Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
 #include 
 #if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
+#if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
 #error Unsupported libstdc++ version
 #endif
 #endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
+int main() { return 0; }
 "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -20,7 +20,7 @@
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
-  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to c

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3705131 , @royjacobson 
wrote:

> This seems to have been more disruptive than expected, since an existing 
> CMakeCache.txt can make LLVM compile in previous C++14 configuration. This 
> seems to make some of the bots fail in a way that makes the patches making 
> use of C++17 features seem at fault.
>
> See:
> https://github.com/llvm/llvm-project/commit/ede96de751224487aea122af8bfb4e82bc54840b#commitcomment-80507826
> https://reviews.llvm.org/rG32fd0b7fd5ab
>
> How would you feel about adding something like
>
>   #if defined(__cplusplus) && __cplusplus < 201703L
>   #error "LLVM requires at least C++17"
>   #endif
>
> to some central header, to make this switch more visible?

I am not opposed to that directly. But this seems a bit dangerous where bots 
retain the cmakecache - there must be other cases where we can't really protect 
in this way.

Another approach would be to unset CMAKE_CXX_STANDARD if it's below 17 in cmake 
directly.

But in general - I am not a huge fan of CI / bots trying to keep the cache 
around - many weird issues can arise from this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3705236 , @royjacobson 
wrote:

> This affects people on their work branches as well, and it's not obvious that 
> it's a configuration error and not a broken master.
>
> The CMake approach sounds cleaner to me, but I don't know CMake well enough 
> to do it - if you could post a follow up patch I think it would be quite 
> helpful.

Good point - I tried a few things and came up with an approach that works here: 
https://reviews.llvm.org/D131367 - have a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3705474 , @dyung wrote:

> We are seeing an additional failure on an internal linux bot due to the 
> change to using C++17 by default when using GNU ld:
> ...
> Switching between BFD ld and gold still fails (although gold fails for a 
> slightly different reason). Superficially it seems that switching to C++17 
> for some reason might be causing the compiler to emit debug info that these 
> older non-lld linkers cannot understand for some reason?

Filed this issue here: https://github.com/llvm/llvm-project/issues/56994


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3705579 , @Jake-Egan wrote:

> There is a failure on the AIX bot also:
> ...
> https://lab.llvm.org/buildbot/#/builders/214/builds/2707/steps/5/logs/stdio

Filed an issue here: https://github.com/llvm/llvm-project/issues/56995


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3706199 , @cor3ntin wrote:

> Trying to read the logs,, notably 
> `C:\PROGRA~2\MIB055~1\2019\PROFES~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe
>  `, it would seem that this particular bot is running a version much older 
> than the current requirements  (Visual Studio 2019 16.7)
> Either I'm reading that wrong or the CMake script does not check the msvc 
> version?

The compilers are definitely version checked here: 
https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/CheckCompilerVersion.cmake#L50

But reading the cmake log it seems like it's using a cache already and it's 
hard to say if it's using the same version of MSVC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3706263 , @aaron.ballman 
wrote:

> 



> Something odd is going on here and we might want to consider a revert of this 
> patch until we resolve it. When I do a git pull and cmake files change, 
> Visual Studio's built-in CMake support automatically re-runs the configure 
> step. This should be all that's necessary to switch the toolchain, but it 
> isn't for some reason (today's build is also failing for me with C++17 
> related issues after I did another pulldown this morning). I deleted my cache 
> explicitly and regenerated CMake from scratch and am still getting the same 
> build errors. The failures I am getting are the same as what's shown by the 
> sanitizer bot for Windows: 
> https://lab.llvm.org/buildbot/#/builders/127/builds/33980/steps/4/logs/stdio 
> (I'm using VS 2019 16.11.17 FWIW).
>
> I hope we can resolve this quickly as basically no MSVC builds are green 
> right now in the build lab.

While we can revert this one - we also need to revert all changes that add 
C++17 features at this point as well. That will be a lot of churn. Let's see if 
we can figure out what's wrong first.

Can you try to locally rebuild with this patch https://reviews.llvm.org/D131382 
?

I think all the runtime errors is because of that one above - basically we 
don't set std=c++17 for any of the compiler-rt projects.

I also think we should merge https://reviews.llvm.org/D131367 for now - we can 
revert that later on if we think it adds to much complexity, since it will 
delete the bad cache values automatcially.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3706336 , @aaron.ballman 
wrote:

> That's the only reason this hasn't been reverted already. Landing sweeping 
> changes on a weekend is a good way to reduce the pain, but we really need to 
> be sure someone watches the build lab and reacts when subsequent changes 
> break everything like this.

Agreed, I think we need to update the protocol for changing the C++ standard in 
the future to account for more testing beforehand. I might push some changes to 
the policy document when all this has settled down to see if we can make sure 
it will be smoother the time we move to C++20. It's unfortunate that some stuff 
broke considering we where running some bots before it was merged and it didn't 
show any errors. And local windows builds for me have been clean as well.

>> Can you try to locally rebuild with this patch 
>> https://reviews.llvm.org/D131382 ?
>
> That improves things but the build still isn't clean:
> ...
> (FWIW, I don't know if any of the Windows builders in the lab are building 
> with /WX)

I am not sure either - but at least this removed the problem with the std=c++17 
not being passed around correctly.

>> I think all the runtime errors is because of that one above - basically we 
>> don't set std=c++17 for any of the compiler-rt projects.
>
> I wasn't building compiler-rt, so no idea why this improved things for me. 
> FWIW, he's my CMake config: `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON 
> -DLLVM_TARGETS_TO_BUILD="X86" -DLLVM_ENABLE_IDE=ON 
> -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" 
> -DLLVM_PARALLEL_COMPILE_JOBS=112 -DLLVM_PARALLEL_LINK_JOBS=16`

That is very odd. I would suspect that the problem was the compiler-rt not 
getting the right flag. But it seems to influence something else as well?

>> I also think we should merge https://reviews.llvm.org/D131367 for now - we 
>> can revert that later on if we think it adds to much complexity, since it 
>> will delete the bad cache values automatcially.
>
> Seems reasonable to me.

I landed this now - hopefully that fixes some of the issues seen in the bots - 
but we might need https://reviews.llvm.org/D131382 as well before they go 
green. I am not sure what the protocol is here, since @MaskRay suggested the 
change maybe we should wait for him to land it, or should we get it in ASAP to 
see if that fixes the bots?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3706424 , @aaron.ballman 
wrote:

> +1, thank you for thinking about how we can improve this process in the 
> future! Given that C++17 adoption across compilers has been far better than 
> C++20, I suspect the next time we bump the language version will be even more 
> of a challenge with these sort of weird issues.

I'll happily ignore that for another 3 years 😅

> Yeah, this is rather deeply concerning to be honest. I triple-checked and the 
> only changes in my tree were yours to compiler-rt and when I looked at the 
> configure logs, they clearly showed `compiler-rt project is disabled` in the 
> logs. It is really weird that a change to compiler-rt's cmake would have any 
> impact on Clang's ability to build when compiler-rt is disabled. I think 
> that's worth some investigative work.

Hmmm - something is really funny, with my cache nuking change it seems like 
this bot is back to working again: 
https://lab.llvm.org/buildbot/#/builders/123/builds/12164

> It seems like we might need that, but it also seems like it would be good to 
> understand why compiler-rt is impacting the rest of Clang when it's disabled. 
> That said, the goal is to get the build lab back to green as quickly as 
> possible, so I think it may make sense to land sooner rather than later.

I'll hold off a bit on this - it seems like my commit above have fixed at least 
some of the issues. There are still some msvc bots failing - but they seem to 
be failing because they have a too old version of MSVC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)

glandium wrote:
> You didn't update llvm/cmake/platforms/WinMsvc.cmake accordingly
AHA! That explains the fms-compatibility issue. Will you push a NFC commit or 
do you want me to do that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)

glandium wrote:
> thieta wrote:
> > glandium wrote:
> > > You didn't update llvm/cmake/platforms/WinMsvc.cmake accordingly
> > AHA! That explains the fms-compatibility issue. Will you push a NFC commit 
> > or do you want me to do that?
> I don't have push access.
fixed it here: 
https://github.com/llvm/llvm-project/commit/15eaefa5fe3608b03f1abefc31129efaf9eab88e


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3709742 , @aaron.ballman 
wrote:

> One thing I think would be a definite improvement is to have done an RFC on 
> Discourse for these changes so that downstreams have a chance to weigh in on 
> the impact. The patch was put up on Jul 28 and landed about a week later 
> without any notification to the rest of the community who might not be 
> watching cfe-commits -- that's a very fast turnaround and very little 
> notification for such a significant change.

Yeah this is on me. Honestly I didn't expect it to be that much of a problem 
but rather the toolchain requirement we posted as part of it would be the big 
hurdle where bot owners would have to upgrade to get the right versions. But 
lesson learned  and we should add some more delays in the policy here: 
https://llvm.org/docs/DeveloperPolicy.html#id23 and cover the C++ standards 
upgrade.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

I think a week or two of moratorium would be good for sure. I think we can 
table this discussion for now and I will write a RFC post in discourse when I 
have time and we can discuss the details there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Hi - I tried to incorporate all the feedback here and added a post to discourse 
suggesting tweaks to the developer policy - please have a look and review it: 
https://discourse.llvm.org/t/rfc-updates-to-developer-policy-around-c-standards-bump/64383


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D122298: [clang-cl] Ignore /Wv and /Wv:17 flags

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
thieta added reviewers: hans, mstorsjo, rnk.
Herald added a project: All.
thieta requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.
Herald added a project: clang.

MSVC supports passing /Wv and /Wv:17 to ignore warnings added
since that version. Clang doesn't have a option like this - but
we can ignore this flag instead of error.

MSVC documentation: 
https://docs.microsoft.com/en-us/cpp/build/reference/compiler-option-warning-level


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122298

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -396,6 +396,8 @@
 // RUN:/volatile:iso \
 // RUN:/w12345 \
 // RUN:/wd1234 \
+// RUN:/Wv \
+// RUN:/Wv:17 \
 // RUN:/Zc:__cplusplus \
 // RUN:/Zc:auto \
 // RUN:/Zc:forScope \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2493,12 +2493,12 @@
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], 
"fno-openmp-assume-threads-oversubscription">,
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
-def fopenmp_assume_no_thread_state : Flag<["-"], 
"fopenmp-assume-no-thread-state">, Group, 
-  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>, 
+def fopenmp_assume_no_thread_state : Flag<["-"], 
"fopenmp-assume-no-thread-state">, Group,
+  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>,
   HelpText<"Assert no thread in a parallel region modifies an ICV">,
   MarshallingInfoFlag>;
-def fopenmp_offload_mandatory : Flag<["-"], "fopenmp-offload-mandatory">, 
Group, 
-  Flags<[CC1Option, NoArgumentUnused]>, 
+def fopenmp_offload_mandatory : Flag<["-"], "fopenmp-offload-mandatory">, 
Group,
+  Flags<[CC1Option, NoArgumentUnused]>,
   HelpText<"Do not create a host fallback if offloading to the device fails.">,
   MarshallingInfoFlag>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
@@ -6528,6 +6528,7 @@
 def _SLASH_utf8 : CLIgnoredFlag<"utf-8">,
   HelpText<"Set source and runtime encoding to UTF-8 (default)">;
 def _SLASH_w : CLIgnoredJoined<"w">;
+def _SLASH_Wv_ : CLIgnoredJoined<"Wv">;
 def _SLASH_Zc___cplusplus : CLIgnoredFlag<"Zc:__cplusplus">;
 def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
 def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;
@@ -6542,7 +6543,6 @@
 def _SLASH_Zo : CLIgnoredFlag<"Zo">;
 def _SLASH_Zo_ : CLIgnoredFlag<"Zo-">;
 
-
 // Unsupported:
 
 def _SLASH_await : CLFlag<"await">;


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -396,6 +396,8 @@
 // RUN:/volatile:iso \
 // RUN:/w12345 \
 // RUN:/wd1234 \
+// RUN:/Wv \
+// RUN:/Wv:17 \
 // RUN:/Zc:__cplusplus \
 // RUN:/Zc:auto \
 // RUN:/Zc:forScope \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2493,12 +2493,12 @@
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], "fno-openmp-assume-threads-oversubscription">,
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
-def fopenmp_assume_no_thread_state : Flag<["-"], "fopenmp-assume-no-thread-state">, Group, 
-  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>, 
+def fopenmp_assume_no_thread_state : Flag<["-"], "fopenmp-assume-no-thread-state">, Group,
+  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>,
   HelpText<"Assert no thread in a parallel region modifies an ICV">,
   MarshallingInfoFlag>;
-def fopenmp_offload_mandatory : Flag<["-"], "fopenmp-offload-mandatory">, Group, 
-  Flags<[CC1Option, NoArgumentUnused]>, 
+def fopenmp_offload_mandatory : Flag<["-"], "fopenmp-offload-mandatory">, Group,
+  Flags<[CC1Option, NoArgumentUnused]>,
   HelpText<"Do not create a host fallback if offloading to the device fails.">,
   MarshallingInfoFlag>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
@@ -6528,6 +6528,7 @@
 def _SLASH_utf8 : CLIgnoredFlag<"utf-8">,
   HelpText<"Set source and runtime encoding to UTF-8 (default)">;
 def _SLASH_w : CLIgnoredJoined<"w">;
+def _SLASH_Wv_ : CLIgnoredJoined<"Wv">;
 def _SLASH_Zc___cplusplus : CLIgnoredFlag<"Zc:__cplusplus">;
 def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
 def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;
@@ -6542,7 +6543,6 @@
 def _SLASH_Zo : CLIgnoredFlag<"Zo">;
 def _SLASH_Zo_ : CLIgnoredFlag<"

[PATCH] D122298: [clang-cl] Ignore /Wv and /Wv:17 flags

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 417533.
thieta added a comment.

Added back removed blank line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122298

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -396,6 +396,8 @@
 // RUN:/volatile:iso \
 // RUN:/w12345 \
 // RUN:/wd1234 \
+// RUN:/Wv \
+// RUN:/Wv:17 \
 // RUN:/Zc:__cplusplus \
 // RUN:/Zc:auto \
 // RUN:/Zc:forScope \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2493,12 +2493,12 @@
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], 
"fno-openmp-assume-threads-oversubscription">,
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
-def fopenmp_assume_no_thread_state : Flag<["-"], 
"fopenmp-assume-no-thread-state">, Group, 
-  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>, 
+def fopenmp_assume_no_thread_state : Flag<["-"], 
"fopenmp-assume-no-thread-state">, Group,
+  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>,
   HelpText<"Assert no thread in a parallel region modifies an ICV">,
   MarshallingInfoFlag>;
-def fopenmp_offload_mandatory : Flag<["-"], "fopenmp-offload-mandatory">, 
Group, 
-  Flags<[CC1Option, NoArgumentUnused]>, 
+def fopenmp_offload_mandatory : Flag<["-"], "fopenmp-offload-mandatory">, 
Group,
+  Flags<[CC1Option, NoArgumentUnused]>,
   HelpText<"Do not create a host fallback if offloading to the device fails.">,
   MarshallingInfoFlag>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
@@ -6528,6 +6528,7 @@
 def _SLASH_utf8 : CLIgnoredFlag<"utf-8">,
   HelpText<"Set source and runtime encoding to UTF-8 (default)">;
 def _SLASH_w : CLIgnoredJoined<"w">;
+def _SLASH_Wv_ : CLIgnoredJoined<"Wv">;
 def _SLASH_Zc___cplusplus : CLIgnoredFlag<"Zc:__cplusplus">;
 def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
 def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -396,6 +396,8 @@
 // RUN:/volatile:iso \
 // RUN:/w12345 \
 // RUN:/wd1234 \
+// RUN:/Wv \
+// RUN:/Wv:17 \
 // RUN:/Zc:__cplusplus \
 // RUN:/Zc:auto \
 // RUN:/Zc:forScope \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2493,12 +2493,12 @@
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], "fno-openmp-assume-threads-oversubscription">,
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
-def fopenmp_assume_no_thread_state : Flag<["-"], "fopenmp-assume-no-thread-state">, Group, 
-  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>, 
+def fopenmp_assume_no_thread_state : Flag<["-"], "fopenmp-assume-no-thread-state">, Group,
+  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>,
   HelpText<"Assert no thread in a parallel region modifies an ICV">,
   MarshallingInfoFlag>;
-def fopenmp_offload_mandatory : Flag<["-"], "fopenmp-offload-mandatory">, Group, 
-  Flags<[CC1Option, NoArgumentUnused]>, 
+def fopenmp_offload_mandatory : Flag<["-"], "fopenmp-offload-mandatory">, Group,
+  Flags<[CC1Option, NoArgumentUnused]>,
   HelpText<"Do not create a host fallback if offloading to the device fails.">,
   MarshallingInfoFlag>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
@@ -6528,6 +6528,7 @@
 def _SLASH_utf8 : CLIgnoredFlag<"utf-8">,
   HelpText<"Set source and runtime encoding to UTF-8 (default)">;
 def _SLASH_w : CLIgnoredJoined<"w">;
+def _SLASH_Wv_ : CLIgnoredJoined<"Wv">;
 def _SLASH_Zc___cplusplus : CLIgnoredFlag<"Zc:__cplusplus">;
 def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
 def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122298: [clang-cl] Ignore /Wv and /Wv:17 flags

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2496
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
-def fopenmp_assume_no_thread_state : Flag<["-"], 
"fopenmp-assume-no-thread-state">, Group, 
-  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>, 
+def fopenmp_assume_no_thread_state : Flag<["-"], 
"fopenmp-assume-no-thread-state">, Group,
+  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>,

Do we mind removing these trailing whitespaces in the same commit? I can fix 
that - but they shouldn't be there anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122298

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

[PATCH] D122298: [clang-cl] Ignore /Wv and /Wv:17 flags

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 417537.
thieta added a comment.

Split whitespace to it's own commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122298

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -396,6 +396,8 @@
 // RUN:/volatile:iso \
 // RUN:/w12345 \
 // RUN:/wd1234 \
+// RUN:/Wv \
+// RUN:/Wv:17 \
 // RUN:/Zc:__cplusplus \
 // RUN:/Zc:auto \
 // RUN:/Zc:forScope \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6528,6 +6528,7 @@
 def _SLASH_utf8 : CLIgnoredFlag<"utf-8">,
   HelpText<"Set source and runtime encoding to UTF-8 (default)">;
 def _SLASH_w : CLIgnoredJoined<"w">;
+def _SLASH_Wv_ : CLIgnoredJoined<"Wv">;
 def _SLASH_Zc___cplusplus : CLIgnoredFlag<"Zc:__cplusplus">;
 def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
 def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -396,6 +396,8 @@
 // RUN:/volatile:iso \
 // RUN:/w12345 \
 // RUN:/wd1234 \
+// RUN:/Wv \
+// RUN:/Wv:17 \
 // RUN:/Zc:__cplusplus \
 // RUN:/Zc:auto \
 // RUN:/Zc:forScope \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6528,6 +6528,7 @@
 def _SLASH_utf8 : CLIgnoredFlag<"utf-8">,
   HelpText<"Set source and runtime encoding to UTF-8 (default)">;
 def _SLASH_w : CLIgnoredJoined<"w">;
+def _SLASH_Wv_ : CLIgnoredJoined<"Wv">;
 def _SLASH_Zc___cplusplus : CLIgnoredFlag<"Zc:__cplusplus">;
 def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
 def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122298: [clang-cl] Ignore /Wv and /Wv:17 flags

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Pushed the whitespace fix as a NFC here: 
https://github.com/llvm/llvm-project/commit/09c0685a043dd4028545c134b562c2605e294855


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122298

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

[PATCH] D129446: [clang][driver] Find Apple default SDK path

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
thieta added reviewers: egorzhdan, t.p.northover, dexonsmith, ldionne.
Herald added a project: All.
thieta requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: clang.

Currently if you download clang and install it on macOS it will
not be able to compile C++ applications out of the box. The
driver can't find the SDK path without help of SDKROOT or -isysroot.

For the Xcode toolchain this is always supplied with xcrun or
the wrapper binary that sets the correct SDKROOT.

But for new users this might be very confusing and since the
path for the SDKs is stable unless the user decide to install
Xcode to an alternative path we can try a naive search for it.

Currently this patch fails a bunch of tests that seems to assume
that no SDK is found and then the macosx-min stuff is set to
something very low. This changes when you have a real SDK.

I also haven't added any new tests to test this since I am didn't
want to assume that Xcode is installed on the system running the
tests. Does anyone have a good idea for testing this?

Tagged a bunch of apple people that have touched the driver
recently - but feel free to add more in here if needed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129446

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,7 +18,9 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Path.h"
@@ -2039,6 +2041,30 @@
 
 } // namespace
 
+static const std::string guessSDKPath(const llvm::Triple::OSType OSType) {
+  std::string Platform;
+
+  if (OSType == llvm::Triple::OSType::MacOSX || OSType == 
llvm::Triple::OSType::Darwin) {
+// Assume macOS when nothing specific is specified.
+Platform = "MacOSX";
+  } else if (OSType == llvm::Triple::OSType::IOS) {
+Platform = "IPhoneOS";
+  } else if (OSType == llvm::Triple::OSType::TvOS) {
+Platform = "AppleTVOS";
+  } else if (OSType == llvm::Triple::OSType::WatchOS) {
+Platform = "WatchOS";
+  }
+
+  std::string GuessPath = (
+"/Applications/Xcode.app/Contents/Developer/Platforms/"
++ Platform
++ ".platform/Developer/SDKs/"
++ Platform
++ ".sdk");
+
+  return GuessPath;
+}
+
 void Darwin::AddDeploymentTarget(DerivedArgList &Args) const {
   const OptTable &Opts = getDriver().getOpts();
 
@@ -2058,6 +2084,14 @@
 Args.append(Args.MakeSeparateArg(
 nullptr, Opts.getOption(options::OPT_isysroot), env));
   }
+} else {
+  // If the user doesn't pass -isysroot nor SDKROOT we will try to guess
+  // the standard path of the SDK (in /Applications/Xcode.app ...)
+  const std::string GuessPath = guessSDKPath(getTriple().getOS());
+  if (getVFS().exists(GuessPath)) {
+Args.append(Args.MakeSeparateArg(
+  nullptr, Opts.getOption(options::OPT_isysroot), GuessPath));
+  }
 }
   }
 
@@ -2251,6 +2285,7 @@
 return DriverArgs.getLastArgValue(options::OPT_isysroot);
   if (!getDriver().SysRoot.empty())
 return getDriver().SysRoot;
+
   return "/";
 }
 


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,7 +18,9 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Path.h"
@@ -2039,6 +2041,30 @@
 
 } // namespace
 
+static const std::string guessSDKPath(const llvm::Triple::OSType OSType) {
+  std::string Platform;
+
+  if (OSType == llvm::Triple::OSType::MacOSX || OSType == llvm::Triple::OSType::Darwin) {
+// Assume macOS when nothing specific is specified.
+Platform = "MacOSX";
+  } else if (OSType == llvm::Triple::OSType::IOS) {
+Platform = "IPhoneOS";
+  } else if (OSType == llvm::Triple::OSType::TvOS) {
+Platform = "AppleTVOS";
+  } else if (OSType == llvm::Triple::OSType::WatchOS) {
+Platform = "WatchOS";
+  }
+
+  std::string GuessPath = (
+"/Applications/Xcode.app/Contents/Developer/Platforms/"
++ Platform
++ ".platform/Developer/SDKs/"
++ Platform
++ ".sdk");
+
+  return GuessPath;
+}
+
 void Darwin::AddDeploymentTarget(DerivedArgList &Args) const {
   const OptTable &Opts = getDriver().getOpts();
 
@@ -2058,6 +2084,14 @@
 Args.append(

[PATCH] D125961: [clang-format] Don't break lines after pragma region

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
Herald added a project: All.
thieta requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We have autogenerated pragma regions in our code
which where awkwardly broken up like this:

#pragma region foo(bar : hello)

#pragma region foo(bar \

  : hello)

This fixes the problem by adding region as a keyword
and handling it the same way as pragma mark


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125961

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/test/Format/pragma-region.cpp


Index: clang/test/Format/pragma-region.cpp
===
--- /dev/null
+++ clang/test/Format/pragma-region.cpp
@@ -0,0 +1,5 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style="{ColumnLimit: 0}" \
+// RUN:   | FileCheck -strict-whitespace %s
+
+// CHECK: {{^\#pragma\ region\ TEST\(FOO\ \:\ BAR\)}}
+#pragma region TEST(FOO : BAR)
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1223,7 +1223,7 @@
   void parsePragma() {
 next(); // Consume "pragma".
 if (CurrentToken &&
-CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, 
Keywords.kw_region)) {
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
   next(); // Consume "mark".
   next(); // Consume first token (so we fix leading whitespace).
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -928,6 +928,7 @@
 kw___has_include_next = &IdentTable.get("__has_include_next");
 
 kw_mark = &IdentTable.get("mark");
+kw_region = &IdentTable.get("region");
 
 kw_extend = &IdentTable.get("extend");
 kw_option = &IdentTable.get("option");
@@ -1046,6 +1047,7 @@
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;


Index: clang/test/Format/pragma-region.cpp
===
--- /dev/null
+++ clang/test/Format/pragma-region.cpp
@@ -0,0 +1,5 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style="{ColumnLimit: 0}" \
+// RUN:   | FileCheck -strict-whitespace %s
+
+// CHECK: {{^\#pragma\ region\ TEST\(FOO\ \:\ BAR\)}}
+#pragma region TEST(FOO : BAR)
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1223,7 +1223,7 @@
   void parsePragma() {
 next(); // Consume "pragma".
 if (CurrentToken &&
-CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, Keywords.kw_region)) {
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
   next(); // Consume "mark".
   next(); // Consume first token (so we fix leading whitespace).
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -928,6 +928,7 @@
 kw___has_include_next = &IdentTable.get("__has_include_next");
 
 kw_mark = &IdentTable.get("mark");
+kw_region = &IdentTable.get("region");
 
 kw_extend = &IdentTable.get("extend");
 kw_option = &IdentTable.get("option");
@@ -1046,6 +1047,7 @@
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125961: [clang-format] Don't break lines after pragma region

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 430882.
thieta added a comment.

Added additional tests to check other pragma statements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125961

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/test/Format/pragma.cpp


Index: clang/test/Format/pragma.cpp
===
--- /dev/null
+++ clang/test/Format/pragma.cpp
@@ -0,0 +1,8 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style="{ColumnLimit: 0}" \
+// RUN:   | FileCheck -strict-whitespace %s
+
+// CHECK: {{^\#pragma\ warning\(disable\ \:\ 342;\ once\ \: 1232\)$}}
+#pragma warning (disable : 342; once : 1232)
+
+// CHECK: {{^\#pragma\ region\ TEST\(FOO\ \:\ BAR\)$}}
+#pragma region TEST(FOO : BAR)
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1223,7 +1223,7 @@
   void parsePragma() {
 next(); // Consume "pragma".
 if (CurrentToken &&
-CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, 
Keywords.kw_region)) {
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
   next(); // Consume "mark".
   next(); // Consume first token (so we fix leading whitespace).
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -928,6 +928,7 @@
 kw___has_include_next = &IdentTable.get("__has_include_next");
 
 kw_mark = &IdentTable.get("mark");
+kw_region = &IdentTable.get("region");
 
 kw_extend = &IdentTable.get("extend");
 kw_option = &IdentTable.get("option");
@@ -1046,6 +1047,7 @@
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;


Index: clang/test/Format/pragma.cpp
===
--- /dev/null
+++ clang/test/Format/pragma.cpp
@@ -0,0 +1,8 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style="{ColumnLimit: 0}" \
+// RUN:   | FileCheck -strict-whitespace %s
+
+// CHECK: {{^\#pragma\ warning\(disable\ \:\ 342;\ once\ \: 1232\)$}}
+#pragma warning (disable : 342; once : 1232)
+
+// CHECK: {{^\#pragma\ region\ TEST\(FOO\ \:\ BAR\)$}}
+#pragma region TEST(FOO : BAR)
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1223,7 +1223,7 @@
   void parsePragma() {
 next(); // Consume "pragma".
 if (CurrentToken &&
-CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, Keywords.kw_region)) {
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
   next(); // Consume "mark".
   next(); // Consume first token (so we fix leading whitespace).
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -928,6 +928,7 @@
 kw___has_include_next = &IdentTable.get("__has_include_next");
 
 kw_mark = &IdentTable.get("mark");
+kw_region = &IdentTable.get("region");
 
 kw_extend = &IdentTable.get("extend");
 kw_option = &IdentTable.get("option");
@@ -1046,6 +1047,7 @@
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125961: [clang-format] Don't break lines after pragma region

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D125961#3526347 , @wanders wrote:

> There are other pragmas which include colon. How do they fare?
>
> Example 
> https://docs.microsoft.com/en-us/cpp/preprocessor/warning?view=msvc-170
>
>   #pragma warning( disable : 4507 34; once : 4385; error : 164 )

I checked this and added an additional test to test for this. It works fine 
without any changes - I think the difference is when there is a additional 
token between pragma and the "text".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125961

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

[PATCH] D125961: [clang-format] Don't break lines after pragma region

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D125961#3527447 , @curdeius wrote:

> Thanks for the patch!
> Could you please add a unit test in unittest/Format/FormatTest.cpp instead of 
> in lit-based test/?
> Is there a bug report that your patch fixes?

Sure - I didn't know clang-format used unittests instead. Maybe there should be 
some documentation around this for new people used to adding lit tests?

I never filed a bug for it - I fixed it instead.

Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125961

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

[PATCH] D125961: [clang-format] Don't break lines after pragma region

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 430953.
thieta added a comment.

Switched to unit test over lit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125961

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18825,6 +18825,13 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, UnderstandPragmaRegion) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("#pragma region TEST(FOO : BAR)", Style);
+
+  EXPECT_EQ("#pragma region TEST(FOO : BAR)", format("#pragma region TEST(FOO 
: BAR)", Style));
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1223,7 +1223,7 @@
   void parsePragma() {
 next(); // Consume "pragma".
 if (CurrentToken &&
-CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, 
Keywords.kw_region)) {
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
   next(); // Consume "mark".
   next(); // Consume first token (so we fix leading whitespace).
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -928,6 +928,7 @@
 kw___has_include_next = &IdentTable.get("__has_include_next");
 
 kw_mark = &IdentTable.get("mark");
+kw_region = &IdentTable.get("region");
 
 kw_extend = &IdentTable.get("extend");
 kw_option = &IdentTable.get("option");
@@ -1046,6 +1047,7 @@
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18825,6 +18825,13 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, UnderstandPragmaRegion) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("#pragma region TEST(FOO : BAR)", Style);
+
+  EXPECT_EQ("#pragma region TEST(FOO : BAR)", format("#pragma region TEST(FOO : BAR)", Style));
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1223,7 +1223,7 @@
   void parsePragma() {
 next(); // Consume "pragma".
 if (CurrentToken &&
-CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, Keywords.kw_region)) {
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
   next(); // Consume "mark".
   next(); // Consume first token (so we fix leading whitespace).
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -928,6 +928,7 @@
 kw___has_include_next = &IdentTable.get("__has_include_next");
 
 kw_mark = &IdentTable.get("mark");
+kw_region = &IdentTable.get("region");
 
 kw_extend = &IdentTable.get("extend");
 kw_option = &IdentTable.get("option");
@@ -1046,6 +1047,7 @@
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125961: [clang-format] Don't break lines after pragma region

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 430954.
thieta added a comment.

Removed comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125961

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18825,6 +18825,13 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, UnderstandPragmaRegion) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("#pragma region TEST(FOO : BAR)", Style);
+
+  EXPECT_EQ("#pragma region TEST(FOO : BAR)", format("#pragma region TEST(FOO 
: BAR)", Style));
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1223,9 +1223,9 @@
   void parsePragma() {
 next(); // Consume "pragma".
 if (CurrentToken &&
-CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, 
Keywords.kw_region)) {
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
-  next(); // Consume "mark".
+  next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
 if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -928,6 +928,7 @@
 kw___has_include_next = &IdentTable.get("__has_include_next");
 
 kw_mark = &IdentTable.get("mark");
+kw_region = &IdentTable.get("region");
 
 kw_extend = &IdentTable.get("extend");
 kw_option = &IdentTable.get("option");
@@ -1046,6 +1047,7 @@
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18825,6 +18825,13 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, UnderstandPragmaRegion) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("#pragma region TEST(FOO : BAR)", Style);
+
+  EXPECT_EQ("#pragma region TEST(FOO : BAR)", format("#pragma region TEST(FOO : BAR)", Style));
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1223,9 +1223,9 @@
   void parsePragma() {
 next(); // Consume "pragma".
 if (CurrentToken &&
-CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, Keywords.kw_region)) {
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
-  next(); // Consume "mark".
+  next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
 if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -928,6 +928,7 @@
 kw___has_include_next = &IdentTable.get("__has_include_next");
 
 kw_mark = &IdentTable.get("mark");
+kw_region = &IdentTable.get("region");
 
 kw_extend = &IdentTable.get("extend");
 kw_option = &IdentTable.get("option");
@@ -1046,6 +1047,7 @@
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125961: [clang-format] Don't break lines after pragma region

[Tobias Hieta via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG749fb33e82ff: [clang-format] Don't break lines after 
pragma region (authored by thieta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125961

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19597,6 +19597,13 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, UnderstandPragmaRegion) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("#pragma region TEST(FOO : BAR)", Style);
+
+  EXPECT_EQ("#pragma region TEST(FOO : BAR)", format("#pragma region TEST(FOO 
: BAR)", Style));
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1251,9 +1251,9 @@
   void parsePragma() {
 next(); // Consume "pragma".
 if (CurrentToken &&
-CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, 
Keywords.kw_region)) {
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
-  next(); // Consume "mark".
+  next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
 if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -930,6 +930,7 @@
 kw___has_include_next = &IdentTable.get("__has_include_next");
 
 kw_mark = &IdentTable.get("mark");
+kw_region = &IdentTable.get("region");
 
 kw_extend = &IdentTable.get("extend");
 kw_option = &IdentTable.get("option");
@@ -1053,6 +1054,7 @@
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19597,6 +19597,13 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, UnderstandPragmaRegion) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("#pragma region TEST(FOO : BAR)", Style);
+
+  EXPECT_EQ("#pragma region TEST(FOO : BAR)", format("#pragma region TEST(FOO : BAR)", Style));
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1251,9 +1251,9 @@
   void parsePragma() {
 next(); // Consume "pragma".
 if (CurrentToken &&
-CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, Keywords.kw_region)) {
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
-  next(); // Consume "mark".
+  next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
 if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -930,6 +930,7 @@
 kw___has_include_next = &IdentTable.get("__has_include_next");
 
 kw_mark = &IdentTable.get("mark");
+kw_region = &IdentTable.get("region");
 
 kw_extend = &IdentTable.get("extend");
 kw_option = &IdentTable.get("option");
@@ -1053,6 +1054,7 @@
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
thieta added reviewers: MyDeveloperDay, curdeius, owenpan.
Herald added a project: All.
thieta requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In our code-base we auto-generate pragma regions the regions
look like method signatures like:

`#pragma region MYREGION(Foo: bar)`

The problem here was that the rest of the line after region
was not marked as stringliteral as in the case of pragma mark
so clang-format tried to change the formatting based on the
method signature.

Added test and mark it similar as pragma mark.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136336

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19968,6 +19968,11 @@
 
   EXPECT_EQ("#pragma region TEST(FOO : BAR)",
 format("#pragma region TEST(FOO : BAR)", Style));
+
+  verifyFormat("#pragma region TEST(FOO: NOSPACE)", Style);
+  EXPECT_EQ("#pragma region TEST(FOO: NOSPACE)",
+format("#pragma region TEST(FOO: NOSPACE)", Style));
+
 }
 
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1337,11 +1337,11 @@
 if (CurrentToken &&
 CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option,
   Keywords.kw_region)) {
-  bool IsMark = CurrentToken->is(Keywords.kw_mark);
+  bool IsMarkOrRegion = CurrentToken->isOneOf(Keywords.kw_mark, 
Keywords.kw_region);
   next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
-if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
+if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator))
   CurrentToken->setType(TT_ImplicitStringLiteral);
 next();
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19968,6 +19968,11 @@
 
   EXPECT_EQ("#pragma region TEST(FOO : BAR)",
 format("#pragma region TEST(FOO : BAR)", Style));
+
+  verifyFormat("#pragma region TEST(FOO: NOSPACE)", Style);
+  EXPECT_EQ("#pragma region TEST(FOO: NOSPACE)",
+format("#pragma region TEST(FOO: NOSPACE)", Style));
+
 }
 
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1337,11 +1337,11 @@
 if (CurrentToken &&
 CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option,
   Keywords.kw_region)) {
-  bool IsMark = CurrentToken->is(Keywords.kw_mark);
+  bool IsMarkOrRegion = CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_region);
   next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
-if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
+if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator))
   CurrentToken->setType(TT_ImplicitStringLiteral);
 next();
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136337: [clang-format] Discard pre-processor statements in parseBracedList()

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
thieta added reviewers: MyDeveloperDay, curdeius, owenpan.
Herald added a project: All.
thieta requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We had some code that looked like this:

  int foo()
  {
#pragma region hello(foo)
#pragma endregion
  }
  
  foo* bar() {}

This was confusingly indented like:

  int foo()
{
  #pragma region...
}

After some investigation I noticed that this is because
`parseBracedList()` thought that this was a initializer list.

The check here: 
https://github.com/tru/llvm-project/blob/main/clang/lib/Format/UnwrappedLineParser.cpp#L709
will mark the code above as ProbablyBracedList and then it
will format it totally wrong depending on your style settings.

My initial fix was to change the check above, but it became
really complicated to keep both initializer lists and my code
working.

My approach here instead is to discard any line that starts with #
since that is a pre-processor statement we shouldn't really care
about it in this case.

This fix passes all the unittests and our internal code-base, so
I am fairly confident in it, but I am no clang-format expert.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136337

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19970,6 +19970,14 @@
 format("#pragma region TEST(FOO : BAR)", Style));
 }
 
+TEST_F(FormatTest, PragmaRegionShouldNotBeBraceInitializers) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("CxxClass::CxxClass() {\n#pragma region test(hello)\n}", Style);
+  EXPECT_EQ("CxxClass::CxxClass()\n#pragma region TEST(bar: foo)\n{\n#pragma 
region TEST(foo: bar)\n#pragma endregion\n}\nCxxClass *Hello() {}",
+format("CxxClass::CxxClass()\n#pragma region TEST(bar: foo)\n{\n  
#pragma region TEST(foo: bar)\n  #pragma endregion\n}\nCxxClass* Hello()\n{}", 
Style));
+
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -751,6 +751,11 @@
   if (!LBraceStack.empty() && LBraceStack.back()->is(BK_Unknown))
 LBraceStack.back()->setBlockKind(BK_Block);
   break;
+case tok::hash:
+  do {
+NextTok = Tokens->getNextToken();
+  } while (NextTok->isNot(tok::eof));
+  break;
 default:
   break;
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19970,6 +19970,14 @@
 format("#pragma region TEST(FOO : BAR)", Style));
 }
 
+TEST_F(FormatTest, PragmaRegionShouldNotBeBraceInitializers) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("CxxClass::CxxClass() {\n#pragma region test(hello)\n}", Style);
+  EXPECT_EQ("CxxClass::CxxClass()\n#pragma region TEST(bar: foo)\n{\n#pragma region TEST(foo: bar)\n#pragma endregion\n}\nCxxClass *Hello() {}",
+format("CxxClass::CxxClass()\n#pragma region TEST(bar: foo)\n{\n  #pragma region TEST(foo: bar)\n  #pragma endregion\n}\nCxxClass* Hello()\n{}", Style));
+
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -751,6 +751,11 @@
   if (!LBraceStack.empty() && LBraceStack.back()->is(BK_Unknown))
 LBraceStack.back()->setBlockKind(BK_Block);
   break;
+case tok::hash:
+  do {
+NextTok = Tokens->getNextToken();
+  } while (NextTok->isNot(tok::eof));
+  break;
 default:
   break;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 470092.
thieta added a comment.

Added TokenAnnotatorTest and removed reduntant FormatTests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136336

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -6,6 +6,7 @@
 //
 
//===--===//
 
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Format/Format.h"
 
 #include "FormatTestUtils.h"
@@ -649,6 +650,23 @@
   EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsPragmaRegion) {
+  // Everything after #pragma region should be ImplicitStringLiteral
+  auto Tokens = annotate("#pragma region Foo(Bar: Hello)");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[6], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_ImplicitStringLiteral);
+
+  // Make sure it's annotated correctly inside a function as well
+  Tokens = annotate("void test(){\n#pragma region Foo(Bar: Hello)\n}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[11], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[12], tok::identifier, TT_ImplicitStringLiteral);
+
+}
+
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
   const char *BaseCode = nullptr;
   const char *ConstrainedCode = nullptr;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19965,9 +19965,7 @@
 TEST_F(FormatTest, UnderstandPragmaRegion) {
   auto Style = getLLVMStyleWithColumns(0);
   verifyFormat("#pragma region TEST(FOO : BAR)", Style);
-
-  EXPECT_EQ("#pragma region TEST(FOO : BAR)",
-format("#pragma region TEST(FOO : BAR)", Style));
+  verifyFormat("#pragma region TEST(FOO: NOSPACE)", Style);
 }
 
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1337,11 +1337,11 @@
 if (CurrentToken &&
 CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option,
   Keywords.kw_region)) {
-  bool IsMark = CurrentToken->is(Keywords.kw_mark);
+  bool IsMarkOrRegion = CurrentToken->isOneOf(Keywords.kw_mark, 
Keywords.kw_region);
   next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
-if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
+if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator))
   CurrentToken->setType(TT_ImplicitStringLiteral);
 next();
   }


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Format/Format.h"
 
 #include "FormatTestUtils.h"
@@ -649,6 +650,23 @@
   EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsPragmaRegion) {
+  // Everything after #pragma region should be ImplicitStringLiteral
+  auto Tokens = annotate("#pragma region Foo(Bar: Hello)");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[6], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_ImplicitStringLiteral);
+
+  // Make sure it's annotated correctly inside a function as well
+  Tokens = annotate("void test(){\n#pragma region Foo(Bar: Hello)\n}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[11], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[12], tok::identifier, TT_ImplicitStringLiteral);
+
+}
+
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
   const char *BaseCode = nullptr;
   const char *ConstrainedCode = nullptr;
Index: clang/unittests/Format/FormatTest.cpp
==

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D136336#3877238 , @owenpan wrote:

> Can you add a test to `TokenAnnotatorTest.cpp`?

Thanks for the review. Fixed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136336

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

[PATCH] D136337: [clang-format] Discard pre-processor statements in parseBracedList()

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136337

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

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 470380.
thieta added a comment.

git clang-format
removed accidental include


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136336

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -649,6 +649,22 @@
   EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsPragmaRegion) {
+  // Everything after #pragma region should be ImplicitStringLiteral
+  auto Tokens = annotate("#pragma region Foo(Bar: Hello)");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[6], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_ImplicitStringLiteral);
+
+  // Make sure it's annotated correctly inside a function as well
+  Tokens = annotate("void test(){\n#pragma region Foo(Bar: Hello)\n}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[11], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[12], tok::identifier, TT_ImplicitStringLiteral);
+}
+
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
   const char *BaseCode = nullptr;
   const char *ConstrainedCode = nullptr;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19965,9 +19965,7 @@
 TEST_F(FormatTest, UnderstandPragmaRegion) {
   auto Style = getLLVMStyleWithColumns(0);
   verifyFormat("#pragma region TEST(FOO : BAR)", Style);
-
-  EXPECT_EQ("#pragma region TEST(FOO : BAR)",
-format("#pragma region TEST(FOO : BAR)", Style));
+  verifyFormat("#pragma region TEST(FOO: NOSPACE)", Style);
 }
 
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1337,14 +1337,15 @@
 if (CurrentToken &&
 CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option,
   Keywords.kw_region)) {
-  bool IsMark = CurrentToken->is(Keywords.kw_mark);
+  bool IsMarkOrRegion =
+  CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_region);
   next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
-if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
+if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator)) {
   CurrentToken->setType(TT_ImplicitStringLiteral);
 next();
-  }
+}
 }
   }
 


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -649,6 +649,22 @@
   EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsPragmaRegion) {
+  // Everything after #pragma region should be ImplicitStringLiteral
+  auto Tokens = annotate("#pragma region Foo(Bar: Hello)");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[6], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_ImplicitStringLiteral);
+
+  // Make sure it's annotated correctly inside a function as well
+  Tokens = annotate("void test(){\n#pragma region Foo(Bar: Hello)\n}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[11], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[12], tok::identifier, TT_ImplicitStringLiteral);
+}
+
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
   const char *BaseCode = nullptr;
   const char *ConstrainedCode = nullptr;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19965,9 +19965,7 @@
 TEST_F(FormatTest, UnderstandPragmaRegion) {
   auto Style = getLLVMStyleWithColumns(0);
   verifyFormat("#pragma region TEST(FOO : BAR)", Style);
-
-  EXPECT_EQ("#pragma region TEST(FOO : BAR)",
-format("#pragma region TEST(FOO : BAR)", Style));
+  verifyFormat("#p

[PATCH] D136337: [clang-format] Discard pre-processor statements in parseBracedList()

[Tobias Hieta via Phabricator via cfe-commits]

thieta abandoned this revision.
thieta added a comment.

In D136337#3881476 , @owenpan wrote:

> Can you create an issue on GitHub and include the details on how to reproduce 
> the problem using the latest clang-format?

Ah I re-tested this with main and it seems like there has been a lot of changes 
to parseBracedList() since I started to investigate this and it seems to work 
out of the box now. I will close this diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136337

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

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

[Tobias Hieta via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd1d93db7106: [clang-format] Mark pragma region lines as 
StringLiterals (authored by thieta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136336

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -649,6 +649,22 @@
   EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsPragmaRegion) {
+  // Everything after #pragma region should be ImplicitStringLiteral
+  auto Tokens = annotate("#pragma region Foo(Bar: Hello)");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[6], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_ImplicitStringLiteral);
+
+  // Make sure it's annotated correctly inside a function as well
+  Tokens = annotate("void test(){\n#pragma region Foo(Bar: Hello)\n}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[11], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[12], tok::identifier, TT_ImplicitStringLiteral);
+}
+
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
   const char *BaseCode = nullptr;
   const char *ConstrainedCode = nullptr;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19965,9 +19965,7 @@
 TEST_F(FormatTest, UnderstandPragmaRegion) {
   auto Style = getLLVMStyleWithColumns(0);
   verifyFormat("#pragma region TEST(FOO : BAR)", Style);
-
-  EXPECT_EQ("#pragma region TEST(FOO : BAR)",
-format("#pragma region TEST(FOO : BAR)", Style));
+  verifyFormat("#pragma region TEST(FOO: NOSPACE)", Style);
 }
 
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1337,14 +1337,15 @@
 if (CurrentToken &&
 CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option,
   Keywords.kw_region)) {
-  bool IsMark = CurrentToken->is(Keywords.kw_mark);
+  bool IsMarkOrRegion =
+  CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_region);
   next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
-if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
+if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator)) {
   CurrentToken->setType(TT_ImplicitStringLiteral);
 next();
-  }
+}
 }
   }
 


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -649,6 +649,22 @@
   EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsPragmaRegion) {
+  // Everything after #pragma region should be ImplicitStringLiteral
+  auto Tokens = annotate("#pragma region Foo(Bar: Hello)");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[6], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_ImplicitStringLiteral);
+
+  // Make sure it's annotated correctly inside a function as well
+  Tokens = annotate("void test(){\n#pragma region Foo(Bar: Hello)\n}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[11], tok::colon, TT_ImplicitStringLiteral);
+  EXPECT_TOKEN(Tokens[12], tok::identifier, TT_ImplicitStringLiteral);
+}
+
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
   const char *BaseCode = nullptr;
   const char *ConstrainedCode = nullptr;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19965,9 +19965,7 @@
 TEST_F(FormatTest, UnderstandPragmaRegion) {
   auto Style = getLLVMStyleWithColumns(0);
   verifyFormat("#pragma region TEST(FOO : BAR)", Style);
-

[PATCH] D136474: [CodeView][clang] Disable emitting command line into CodeView by default and add flag to enable

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

We have tooling that uses this command line from the PDB. I think the 
conservative approach is to emit the command line by default since this is how 
MSVC behaves. I don't mind this flag now that I know about it, but I think we 
should keep the default to what MSVC does for clang-cl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136474

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

[PATCH] D136474: [CodeView][clang] Disable emitting command line into CodeView by default and add flag to enable

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Also this should have a release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136474

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

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1345
   while (CurrentToken) {
-if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
+if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator)) {
   CurrentToken->setType(TT_ImplicitStringLiteral);

HazardyKnusperkeks wrote:
> This shouldn't be here. There is no closing brace for this, or am I missing 
> something?
No you are right it was a miss in the rebase. But it's fixed on main now. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136336

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

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

We have been building a two stage clang and our internal projects with this 
version of rpmalloc and clang 15.x a while now and I haven't see the issue you 
see @aganea. I don't think this patch is the problem as @efriedma suggested, 
could be in rpmalloc but since I have used this successfully already I am not 
sure about that either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126903

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

[PATCH] D136474: [CodeView][clang] Disable emitting command line into CodeView by default and add flag to enable

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

I am fine with this plan as well. And I am willing to chip in and try to fix 
the issues with the current implementation since we actually use the feature.

@thakis do you mind explaining the issues you see and how to replicate them and 
I can have a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136474

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:57
+
+* GCC >= 7.1
+* Clang >= 5.0

xbolva00 wrote:
> Do we have bots which uses last supported compiler versions? GCC 7.1, Clang 
> 5.0 and MSVC 16.7.
> 
> It is bad to promise something and then breakages here and there, for 
> example: https://github.com/llvm/llvm-project/issues/57057 so probably no 
> such bots.
I am not sure. I think it would be good if you posted that to discourse so that 
bot owners can reply to that. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

Hi @Sockke - this seems to be ready to land. Is there something holding it 
back? We ran into the same issue recently.


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

https://reviews.llvm.org/D127293

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

[PATCH] D132486: Revert "libclang.so: Make SONAME the same as LLVM version"

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
Herald added subscribers: fedor.sergeev, mgorny.
Herald added a project: All.
thieta requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This reverts commit bc39d7bdd4977a953b2e102f8f7eb479ad78984e 
.

rename CLANG_SONAME to LIBCLANG_SOVERSION

[clang][cmake] introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132486

Files:
  clang/CMakeLists.txt
  clang/docs/ReleaseNotes.rst
  clang/tools/libclang/CMakeLists.txt
  clang/tools/libclang/libclang.exports
  clang/tools/libclang/libclang.map
  clang/tools/libclang/linker-script-to-export-list.py

Index: clang/tools/libclang/linker-script-to-export-list.py
===
--- /dev/null
+++ clang/tools/libclang/linker-script-to-export-list.py
@@ -0,0 +1,11 @@
+import re
+import os
+import sys
+
+input_file = open(sys.argv[1])
+output_file = open(sys.argv[2], 'w')
+
+for line in input_file:
+m = re.search('^\s+(clang_[^;]+)', line)
+if m:
+output_file.write(m.group(1) + "\n")
Index: clang/tools/libclang/libclang.map
===
--- /dev/null
+++ clang/tools/libclang/libclang.map
@@ -0,0 +1,413 @@
+# If you add a symbol to this file, make sure to add it with the correct
+# version.  For example, if the LLVM main branch is LLVM 14.0.0, add new
+# symbols with the version LLVM_14.
+# On platforms where versions scripts are not used, this file will be used to
+# generate a list of exports for libclang.so
+
+LLVM_13 {
+  global:
+clang_BlockCommandComment_getArgText;
+clang_BlockCommandComment_getCommandName;
+clang_BlockCommandComment_getNumArgs;
+clang_BlockCommandComment_getParagraph;
+clang_CXCursorSet_contains;
+clang_CXCursorSet_insert;
+clang_CXIndex_getGlobalOptions;
+clang_CXIndex_setGlobalOptions;
+clang_CXIndex_setInvocationEmissionPathOption;
+clang_CXRewriter_create;
+clang_CXRewriter_dispose;
+clang_CXRewriter_insertTextBefore;
+clang_CXRewriter_overwriteChangedFiles;
+clang_CXRewriter_removeText;
+clang_CXRewriter_replaceText;
+clang_CXRewriter_writeMainFileToStdOut;
+clang_CXXConstructor_isConvertingConstructor;
+clang_CXXConstructor_isCopyConstructor;
+clang_CXXConstructor_isDefaultConstructor;
+clang_CXXConstructor_isMoveConstructor;
+clang_CXXField_isMutable;
+clang_CXXMethod_isConst;
+clang_CXXMethod_isDefaulted;
+clang_CXXMethod_isPureVirtual;
+clang_CXXMethod_isStatic;
+clang_CXXMethod_isVirtual;
+clang_CXXRecord_isAbstract;
+clang_Comment_getChild;
+clang_Comment_getKind;
+clang_Comment_getNumChildren;
+clang_Comment_isWhitespace;
+clang_CompilationDatabase_dispose;
+clang_CompilationDatabase_fromDirectory;
+clang_CompilationDatabase_getAllCompileCommands;
+clang_CompilationDatabase_getCompileCommands;
+clang_CompileCommand_getArg;
+clang_CompileCommand_getDirectory;
+clang_CompileCommand_getFilename;
+clang_CompileCommand_getMappedSourceContent;
+clang_CompileCommand_getMappedSourcePath;
+clang_CompileCommand_getNumArgs;
+clang_CompileCommand_getNumMappedSources;
+clang_CompileCommands_dispose;
+clang_CompileCommands_getCommand;
+clang_CompileCommands_getSize;
+clang_Cursor_Evaluate;
+clang_Cursor_getArgument;
+clang_Cursor_getBriefCommentText;
+clang_Cursor_getCXXManglings;
+clang_Cursor_getCommentRange;
+clang_Cursor_getMangling;
+clang_Cursor_getModule;
+clang_Cursor_getNumArguments;
+clang_Cursor_getNumTemplateArguments;
+clang_Cursor_getObjCDeclQualifiers;
+clang_Cursor_getObjCManglings;
+clang_Cursor_getObjCPropertyAttributes;
+clang_Cursor_getObjCPropertyGetterName;
+clang_Cursor_getObjCPropertySetterName;
+clang_Cursor_getObjCSelectorIndex;
+clang_Cursor_getOffsetOfField;
+clang_Cursor_getParsedComment;
+clang_Cursor_getRawCommentText;
+clang_Cursor_getReceiverType;
+clang_Cursor_getSpellingNameRange;
+clang_Cursor_getStorageClass;
+clang_Cursor_getTemplateArgumentKind;
+clang_Cursor_getTemplateArgumentType;
+clang_Cursor_getTemplateArgumentUnsignedValue;
+clang_Cursor_getTemplateArgumentValue;
+clang_Cursor_getTranslationUnit;
+clang_Cursor_getVarDeclInitializer;
+clang_Cursor_hasAttrs;
+clang_Cursor_hasVarDeclExternalStorage;
+clang_Cursor_hasVarDeclGlobalStorage;
+clang_Cursor_isAnonymous;
+clang_Cursor_isAnonymousRecordDecl;
+clang_Cursor_isBitField;
+clang_Cursor_isDynamicCall;
+clang_Cursor_isExternalSymbol;
+clang_Cursor_isFunctionInlined;
+clang_Cursor_isInlineNamespace;
+clang_Cursor_isMacroBuiltin;
+clang_Cursor_isMacroFunctionLike;
+clang_Cursor_is

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 455087.
thieta added a comment.

Updated the option to default to ON - meaning we keep the current 
behavior found in main. This rationel for this is explained in my post here:

https://discourse.llvm.org/t/rationale-for-removing-versioned-libclang-middle-ground-to-keep-it-behind-option/64410/32


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132486

Files:
  clang/CMakeLists.txt
  clang/docs/ReleaseNotes.rst
  clang/tools/libclang/CMakeLists.txt
  clang/tools/libclang/libclang.exports
  clang/tools/libclang/libclang.map
  clang/tools/libclang/linker-script-to-export-list.py

Index: clang/tools/libclang/linker-script-to-export-list.py
===
--- /dev/null
+++ clang/tools/libclang/linker-script-to-export-list.py
@@ -0,0 +1,11 @@
+import re
+import os
+import sys
+
+input_file = open(sys.argv[1])
+output_file = open(sys.argv[2], 'w')
+
+for line in input_file:
+m = re.search('^\s+(clang_[^;]+)', line)
+if m:
+output_file.write(m.group(1) + "\n")
Index: clang/tools/libclang/libclang.map
===
--- /dev/null
+++ clang/tools/libclang/libclang.map
@@ -0,0 +1,413 @@
+# If you add a symbol to this file, make sure to add it with the correct
+# version.  For example, if the LLVM main branch is LLVM 14.0.0, add new
+# symbols with the version LLVM_14.
+# On platforms where versions scripts are not used, this file will be used to
+# generate a list of exports for libclang.so
+
+LLVM_13 {
+  global:
+clang_BlockCommandComment_getArgText;
+clang_BlockCommandComment_getCommandName;
+clang_BlockCommandComment_getNumArgs;
+clang_BlockCommandComment_getParagraph;
+clang_CXCursorSet_contains;
+clang_CXCursorSet_insert;
+clang_CXIndex_getGlobalOptions;
+clang_CXIndex_setGlobalOptions;
+clang_CXIndex_setInvocationEmissionPathOption;
+clang_CXRewriter_create;
+clang_CXRewriter_dispose;
+clang_CXRewriter_insertTextBefore;
+clang_CXRewriter_overwriteChangedFiles;
+clang_CXRewriter_removeText;
+clang_CXRewriter_replaceText;
+clang_CXRewriter_writeMainFileToStdOut;
+clang_CXXConstructor_isConvertingConstructor;
+clang_CXXConstructor_isCopyConstructor;
+clang_CXXConstructor_isDefaultConstructor;
+clang_CXXConstructor_isMoveConstructor;
+clang_CXXField_isMutable;
+clang_CXXMethod_isConst;
+clang_CXXMethod_isDefaulted;
+clang_CXXMethod_isPureVirtual;
+clang_CXXMethod_isStatic;
+clang_CXXMethod_isVirtual;
+clang_CXXRecord_isAbstract;
+clang_Comment_getChild;
+clang_Comment_getKind;
+clang_Comment_getNumChildren;
+clang_Comment_isWhitespace;
+clang_CompilationDatabase_dispose;
+clang_CompilationDatabase_fromDirectory;
+clang_CompilationDatabase_getAllCompileCommands;
+clang_CompilationDatabase_getCompileCommands;
+clang_CompileCommand_getArg;
+clang_CompileCommand_getDirectory;
+clang_CompileCommand_getFilename;
+clang_CompileCommand_getMappedSourceContent;
+clang_CompileCommand_getMappedSourcePath;
+clang_CompileCommand_getNumArgs;
+clang_CompileCommand_getNumMappedSources;
+clang_CompileCommands_dispose;
+clang_CompileCommands_getCommand;
+clang_CompileCommands_getSize;
+clang_Cursor_Evaluate;
+clang_Cursor_getArgument;
+clang_Cursor_getBriefCommentText;
+clang_Cursor_getCXXManglings;
+clang_Cursor_getCommentRange;
+clang_Cursor_getMangling;
+clang_Cursor_getModule;
+clang_Cursor_getNumArguments;
+clang_Cursor_getNumTemplateArguments;
+clang_Cursor_getObjCDeclQualifiers;
+clang_Cursor_getObjCManglings;
+clang_Cursor_getObjCPropertyAttributes;
+clang_Cursor_getObjCPropertyGetterName;
+clang_Cursor_getObjCPropertySetterName;
+clang_Cursor_getObjCSelectorIndex;
+clang_Cursor_getOffsetOfField;
+clang_Cursor_getParsedComment;
+clang_Cursor_getRawCommentText;
+clang_Cursor_getReceiverType;
+clang_Cursor_getSpellingNameRange;
+clang_Cursor_getStorageClass;
+clang_Cursor_getTemplateArgumentKind;
+clang_Cursor_getTemplateArgumentType;
+clang_Cursor_getTemplateArgumentUnsignedValue;
+clang_Cursor_getTemplateArgumentValue;
+clang_Cursor_getTranslationUnit;
+clang_Cursor_getVarDeclInitializer;
+clang_Cursor_hasAttrs;
+clang_Cursor_hasVarDeclExternalStorage;
+clang_Cursor_hasVarDeclGlobalStorage;
+clang_Cursor_isAnonymous;
+clang_Cursor_isAnonymousRecordDecl;
+clang_Cursor_isBitField;
+clang_Cursor_isDynamicCall;
+clang_Cursor_isExternalSymbol;
+clang_Cursor_isFunctionInlined;
+clang_Cursor_isInlineNamespace;
+clang_Cursor_isMacroBuiltin;
+clang_Cursor_isMacroFunctionLike;
+clang_Cursor_isNull;
+clang_Cursor_isObjCOptional;
+clang_Cursor_isVariadic;

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

[Tobias Hieta via Phabricator via cfe-commits]

thieta updated this revision to Diff 455088.
thieta added a comment.

Fixed variable name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132486

Files:
  clang/CMakeLists.txt
  clang/docs/ReleaseNotes.rst
  clang/tools/libclang/CMakeLists.txt
  clang/tools/libclang/libclang.exports
  clang/tools/libclang/libclang.map
  clang/tools/libclang/linker-script-to-export-list.py

Index: clang/tools/libclang/linker-script-to-export-list.py
===
--- /dev/null
+++ clang/tools/libclang/linker-script-to-export-list.py
@@ -0,0 +1,11 @@
+import re
+import os
+import sys
+
+input_file = open(sys.argv[1])
+output_file = open(sys.argv[2], 'w')
+
+for line in input_file:
+m = re.search('^\s+(clang_[^;]+)', line)
+if m:
+output_file.write(m.group(1) + "\n")
Index: clang/tools/libclang/libclang.map
===
--- /dev/null
+++ clang/tools/libclang/libclang.map
@@ -0,0 +1,413 @@
+# If you add a symbol to this file, make sure to add it with the correct
+# version.  For example, if the LLVM main branch is LLVM 14.0.0, add new
+# symbols with the version LLVM_14.
+# On platforms where versions scripts are not used, this file will be used to
+# generate a list of exports for libclang.so
+
+LLVM_13 {
+  global:
+clang_BlockCommandComment_getArgText;
+clang_BlockCommandComment_getCommandName;
+clang_BlockCommandComment_getNumArgs;
+clang_BlockCommandComment_getParagraph;
+clang_CXCursorSet_contains;
+clang_CXCursorSet_insert;
+clang_CXIndex_getGlobalOptions;
+clang_CXIndex_setGlobalOptions;
+clang_CXIndex_setInvocationEmissionPathOption;
+clang_CXRewriter_create;
+clang_CXRewriter_dispose;
+clang_CXRewriter_insertTextBefore;
+clang_CXRewriter_overwriteChangedFiles;
+clang_CXRewriter_removeText;
+clang_CXRewriter_replaceText;
+clang_CXRewriter_writeMainFileToStdOut;
+clang_CXXConstructor_isConvertingConstructor;
+clang_CXXConstructor_isCopyConstructor;
+clang_CXXConstructor_isDefaultConstructor;
+clang_CXXConstructor_isMoveConstructor;
+clang_CXXField_isMutable;
+clang_CXXMethod_isConst;
+clang_CXXMethod_isDefaulted;
+clang_CXXMethod_isPureVirtual;
+clang_CXXMethod_isStatic;
+clang_CXXMethod_isVirtual;
+clang_CXXRecord_isAbstract;
+clang_Comment_getChild;
+clang_Comment_getKind;
+clang_Comment_getNumChildren;
+clang_Comment_isWhitespace;
+clang_CompilationDatabase_dispose;
+clang_CompilationDatabase_fromDirectory;
+clang_CompilationDatabase_getAllCompileCommands;
+clang_CompilationDatabase_getCompileCommands;
+clang_CompileCommand_getArg;
+clang_CompileCommand_getDirectory;
+clang_CompileCommand_getFilename;
+clang_CompileCommand_getMappedSourceContent;
+clang_CompileCommand_getMappedSourcePath;
+clang_CompileCommand_getNumArgs;
+clang_CompileCommand_getNumMappedSources;
+clang_CompileCommands_dispose;
+clang_CompileCommands_getCommand;
+clang_CompileCommands_getSize;
+clang_Cursor_Evaluate;
+clang_Cursor_getArgument;
+clang_Cursor_getBriefCommentText;
+clang_Cursor_getCXXManglings;
+clang_Cursor_getCommentRange;
+clang_Cursor_getMangling;
+clang_Cursor_getModule;
+clang_Cursor_getNumArguments;
+clang_Cursor_getNumTemplateArguments;
+clang_Cursor_getObjCDeclQualifiers;
+clang_Cursor_getObjCManglings;
+clang_Cursor_getObjCPropertyAttributes;
+clang_Cursor_getObjCPropertyGetterName;
+clang_Cursor_getObjCPropertySetterName;
+clang_Cursor_getObjCSelectorIndex;
+clang_Cursor_getOffsetOfField;
+clang_Cursor_getParsedComment;
+clang_Cursor_getRawCommentText;
+clang_Cursor_getReceiverType;
+clang_Cursor_getSpellingNameRange;
+clang_Cursor_getStorageClass;
+clang_Cursor_getTemplateArgumentKind;
+clang_Cursor_getTemplateArgumentType;
+clang_Cursor_getTemplateArgumentUnsignedValue;
+clang_Cursor_getTemplateArgumentValue;
+clang_Cursor_getTranslationUnit;
+clang_Cursor_getVarDeclInitializer;
+clang_Cursor_hasAttrs;
+clang_Cursor_hasVarDeclExternalStorage;
+clang_Cursor_hasVarDeclGlobalStorage;
+clang_Cursor_isAnonymous;
+clang_Cursor_isAnonymousRecordDecl;
+clang_Cursor_isBitField;
+clang_Cursor_isDynamicCall;
+clang_Cursor_isExternalSymbol;
+clang_Cursor_isFunctionInlined;
+clang_Cursor_isInlineNamespace;
+clang_Cursor_isMacroBuiltin;
+clang_Cursor_isMacroFunctionLike;
+clang_Cursor_isNull;
+clang_Cursor_isObjCOptional;
+clang_Cursor_isVariadic;
+clang_EnumDecl_isScoped;
+clang_EvalResult_dispose;
+clang_EvalResult_getAsDouble;
+clang_EvalResult_getAsInt;
+clang_EvalResult_getAsLongLong;
+clang_EvalResult_getAsStr;
+clang_EvalResult_getAsUnsigned;
+clang

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

@jrtc27 What do you think about this patch with the default flipped? I think 
this is how it should land personally as discussed on discourse.

I have been trying to listen in how people want to handle this and I hope this 
is a good middle ground that we can agree on for 15.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132486

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

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

[Tobias Hieta via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f28d4856630: SONAME introduce option 
CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION (authored by h-vetinari, committed by 
thieta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132486

Files:
  clang/CMakeLists.txt
  clang/docs/ReleaseNotes.rst
  clang/tools/libclang/CMakeLists.txt
  clang/tools/libclang/libclang.exports
  clang/tools/libclang/libclang.map
  clang/tools/libclang/linker-script-to-export-list.py

Index: clang/tools/libclang/linker-script-to-export-list.py
===
--- /dev/null
+++ clang/tools/libclang/linker-script-to-export-list.py
@@ -0,0 +1,11 @@
+import re
+import os
+import sys
+
+input_file = open(sys.argv[1])
+output_file = open(sys.argv[2], 'w')
+
+for line in input_file:
+m = re.search('^\s+(clang_[^;]+)', line)
+if m:
+output_file.write(m.group(1) + "\n")
Index: clang/tools/libclang/libclang.map
===
--- /dev/null
+++ clang/tools/libclang/libclang.map
@@ -0,0 +1,413 @@
+# If you add a symbol to this file, make sure to add it with the correct
+# version.  For example, if the LLVM main branch is LLVM 14.0.0, add new
+# symbols with the version LLVM_14.
+# On platforms where versions scripts are not used, this file will be used to
+# generate a list of exports for libclang.so
+
+LLVM_13 {
+  global:
+clang_BlockCommandComment_getArgText;
+clang_BlockCommandComment_getCommandName;
+clang_BlockCommandComment_getNumArgs;
+clang_BlockCommandComment_getParagraph;
+clang_CXCursorSet_contains;
+clang_CXCursorSet_insert;
+clang_CXIndex_getGlobalOptions;
+clang_CXIndex_setGlobalOptions;
+clang_CXIndex_setInvocationEmissionPathOption;
+clang_CXRewriter_create;
+clang_CXRewriter_dispose;
+clang_CXRewriter_insertTextBefore;
+clang_CXRewriter_overwriteChangedFiles;
+clang_CXRewriter_removeText;
+clang_CXRewriter_replaceText;
+clang_CXRewriter_writeMainFileToStdOut;
+clang_CXXConstructor_isConvertingConstructor;
+clang_CXXConstructor_isCopyConstructor;
+clang_CXXConstructor_isDefaultConstructor;
+clang_CXXConstructor_isMoveConstructor;
+clang_CXXField_isMutable;
+clang_CXXMethod_isConst;
+clang_CXXMethod_isDefaulted;
+clang_CXXMethod_isPureVirtual;
+clang_CXXMethod_isStatic;
+clang_CXXMethod_isVirtual;
+clang_CXXRecord_isAbstract;
+clang_Comment_getChild;
+clang_Comment_getKind;
+clang_Comment_getNumChildren;
+clang_Comment_isWhitespace;
+clang_CompilationDatabase_dispose;
+clang_CompilationDatabase_fromDirectory;
+clang_CompilationDatabase_getAllCompileCommands;
+clang_CompilationDatabase_getCompileCommands;
+clang_CompileCommand_getArg;
+clang_CompileCommand_getDirectory;
+clang_CompileCommand_getFilename;
+clang_CompileCommand_getMappedSourceContent;
+clang_CompileCommand_getMappedSourcePath;
+clang_CompileCommand_getNumArgs;
+clang_CompileCommand_getNumMappedSources;
+clang_CompileCommands_dispose;
+clang_CompileCommands_getCommand;
+clang_CompileCommands_getSize;
+clang_Cursor_Evaluate;
+clang_Cursor_getArgument;
+clang_Cursor_getBriefCommentText;
+clang_Cursor_getCXXManglings;
+clang_Cursor_getCommentRange;
+clang_Cursor_getMangling;
+clang_Cursor_getModule;
+clang_Cursor_getNumArguments;
+clang_Cursor_getNumTemplateArguments;
+clang_Cursor_getObjCDeclQualifiers;
+clang_Cursor_getObjCManglings;
+clang_Cursor_getObjCPropertyAttributes;
+clang_Cursor_getObjCPropertyGetterName;
+clang_Cursor_getObjCPropertySetterName;
+clang_Cursor_getObjCSelectorIndex;
+clang_Cursor_getOffsetOfField;
+clang_Cursor_getParsedComment;
+clang_Cursor_getRawCommentText;
+clang_Cursor_getReceiverType;
+clang_Cursor_getSpellingNameRange;
+clang_Cursor_getStorageClass;
+clang_Cursor_getTemplateArgumentKind;
+clang_Cursor_getTemplateArgumentType;
+clang_Cursor_getTemplateArgumentUnsignedValue;
+clang_Cursor_getTemplateArgumentValue;
+clang_Cursor_getTranslationUnit;
+clang_Cursor_getVarDeclInitializer;
+clang_Cursor_hasAttrs;
+clang_Cursor_hasVarDeclExternalStorage;
+clang_Cursor_hasVarDeclGlobalStorage;
+clang_Cursor_isAnonymous;
+clang_Cursor_isAnonymousRecordDecl;
+clang_Cursor_isBitField;
+clang_Cursor_isDynamicCall;
+clang_Cursor_isExternalSymbol;
+clang_Cursor_isFunctionInlined;
+clang_Cursor_isInlineNamespace;
+clang_Cursor_isMacroBuiltin;
+clang_Cursor_isMacroFunctionLike;
+clang_Cursor_isNull;
+clang_Cursor_isObjCOptional;
+clang_Cursor_isVariadic;
+clang_EnumDecl_isScoped;
+