Re: r353590 - This reverts commit 1440a848a635849b97f7a5cfa0ecc40d37451f5b.

[Roman Lebedev via cfe-commits]

On Sat, Feb 9, 2019 at 3:45 AM Mikhail R. Gadelha via cfe-commits
 wrote:
>
> Author: mramalho
> Date: Fri Feb  8 16:46:12 2019
> New Revision: 353590
>
> URL: http://llvm.org/viewvc/llvm-project?rev=353590=rev
> Log:
> This reverts commit 1440a848a635849b97f7a5cfa0ecc40d37451f5b.
> and commit a1853e834c65751f92521f7481b15cf0365e796b.
For future reference, this isn't very useful commit message.
First, it should still talk about svn commit numbers.
Second, original commit subject should still be present - usually
people rarely can remember commit msg based on commit id/hash.

> They broke arm and aarch64
>
> ...
Roman.

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

[PATCH] D57896: Variable names rule

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

Is this actually any better?  Whereas before we can’t differentiate type names 
and variable names, under this proposal we can’t differentiate type names and 
function names.  So it seems a bit of “6 of 1, half dozen of another”


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Thomas Lively via Phabricator via cfe-commits]

tlively added inline comments.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:29
+  Pthread =
+  DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false);
+  ThreadModel =

aheejin wrote:
> tlively wrote:
> > Shouldn't every use of `hasFlag` be `getLastArgValue` instead?
> `hasFlag` is a convenient way to check everything with one function call. 
> with `getLastArgValue` we have to call it twice (for example, for `-pthread` 
> and for `-no-pthread`), and most importantly when both of them are given, 
> calling `getLastArgValue` doesn't give you information on which one is the 
> last. `hasFlag` takes care of that by taking the last one when both of them 
> are given. So `-pthread -no-pthread` will return false, and `-no-pthread 
> -pthread` will return true.
> 
> The reason I used `hasArg` below is just to check if the user gave it 
> explicitly or not. And that's the reason I named variables `Pthread` and 
> `HasPthread`.
I see! I had misunderstood hasFlag. Thanks for explaining it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57532: [Index] Make sure c-index-test finds libc++ on Mac

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57532



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

[PATCH] D57930: [Driver] Verify GCCInstallation is valid

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:259-260
 
   if (GCCInstallation.getParentLibPath().find("opt/rh/devtoolset") !=
   StringRef::npos)
 // With devtoolset on RHEL, we want to add a bin directory that is relative

Do we need to add the same check here too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57930



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

[PATCH] D57207: [clang-tidy] Make google-objc-function-naming ignore implicit functions 

[Stephane Moore via Phabricator via cfe-commits]

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



Comment at: test/clang-tidy/google-objc-function-naming.m:3
 
+#import 
+

aaron.ballman wrote:
> stephanemoore wrote:
> > It turns out importing  is problematic and breaks the build 
> > (though everything built successfully for me locally 樂). I believe that the 
> > import is not strictly necessary and I can embed a function declaration for 
> > `printf` to reproduce the implicit function declaration with the caveat 
> > that the check will trigger on a `printf` declaration that is not from a 
> > system header. I suppose it might be reasonable to suppress the check 
> > output on `printf`?
> > 
> > I have reverted this change for now and will follow up with appropriate 
> > fixes.
> > It turns out importing  is problematic and breaks the build 
> > (though everything built successfully for me locally 樂).
> 
> Sorry about not catching that during review. I'm used to mentioning that you 
> can't do `#include` but I didn't recall if the same was true for `#import`.
> 
> > I believe that the import is not strictly necessary and I can embed a 
> > function declaration for printf to reproduce the implicit function 
> > declaration with the caveat that the check will trigger on a printf 
> > declaration that is not from a system header. I suppose it might be 
> > reasonable to suppress the check output on printf?
> 
> You can use line markers to specify that the declaration is part of a system 
> header with something like this:
> ```
> # 1 "system_header.h" 3
> int printf(const char *, ...);
> # 1 "google-objc-function-naming.m" 1
> ```
> (Note, you may need to pull a more decorated signature for `printf()` to be 
> accurate.)
Thanks for the tip! I'll try that out and send out a followup!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57207



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

r353599 - [StaticAnalyzer] Add missing include to SMTAPI.h. [NFC]

[David L. Jones via cfe-commits]

Author: dlj
Date: Fri Feb  8 18:41:55 2019
New Revision: 353599

URL: http://llvm.org/viewvc/llvm-project?rev=353599=rev
Log:
[StaticAnalyzer] Add missing include to SMTAPI.h. [NFC]

This include is needed for types within SMTAPI.h. (It's very possible that
compilation would succeed without it, but that's only by chance.)

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTAPI.h

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTAPI.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTAPI.h?rev=353599=353598=353599=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTAPI.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTAPI.h Fri Feb  
8 18:41:55 2019
@@ -16,6 +16,7 @@
 
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/APSInt.h"
+#include "llvm/ADT/FoldingSet.h"
 
 namespace clang {
 namespace ento {


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

[PATCH] D57860: [analyzer] Validate checker option names and values

[Artem Dergachev via Phabricator via cfe-commits]

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

> (already in the works!) Document the frontend of the analyzer, sneak peak 
> here.

Whoa!!

*celebrates the graphomania epidemic*


Repository:
  rC Clang

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

https://reviews.llvm.org/D57860



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

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

[Artem Dergachev via Phabricator via cfe-commits]

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

I think this is awesome!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57858



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

Re: r353590 - This reverts commit 1440a848a635849b97f7a5cfa0ecc40d37451f5b.

[Shoaib Meenai via cfe-commits]

I was gonna point to the directions for using llvm/utils/git-svn/git-svnrevert, 
but it looks like that script only works with git svn and not with the git 
monorepo. James, was updating the revert script something on your roadmap? If 
not, I'd be happy to take a stab at it (and I'm wondering if it makes sense to 
introduce a revert subcommand to `git llvm` rather than having a separate 
script).

On 2/8/19, 5:30 PM, "cfe-commits on behalf of Richard Smith via cfe-commits" 
 
wrote:

On Fri, 8 Feb 2019 at 16:45, Mikhail R. Gadelha via cfe-commits
 wrote:
> Author: mramalho
> Date: Fri Feb  8 16:46:12 2019
> New Revision: 353590
>
> URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D353590-26view-3Drev=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=aepigNtcB7YIT8UBidW_1L_WrJGW-Zw_5JscfbewUQw=HF64M7rSDlri8fYyPLRCpEDRRnw_8cw91dU3-ZX_HY4=
> Log:
> This reverts commit 1440a848a635849b97f7a5cfa0ecc40d37451f5b.
> and commit a1853e834c65751f92521f7481b15cf0365e796b.

Please include SVN revision numbers in revert messages until LLVM
moves over to git.

> They broke arm and aarch64
>
> Added:
> cfe/trunk/cmake/modules/FindZ3.cmake
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTAPI.h
> cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
> Removed:
> cfe/trunk/lib/StaticAnalyzer/Core/SMTConstraintManager.cpp
> Modified:
> cfe/trunk/CMakeLists.txt
> cfe/trunk/include/clang/Config/config.h.cmake
> 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
> cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
> cfe/trunk/test/CMakeLists.txt
> cfe/trunk/test/lit.site.cfg.py.in
>
> Modified: cfe/trunk/CMakeLists.txt
> URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_CMakeLists.txt-3Frev-3D353590-26r1-3D353589-26r2-3D353590-26view-3Ddiff=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=aepigNtcB7YIT8UBidW_1L_WrJGW-Zw_5JscfbewUQw=PSdqknpf3IrSNcvN1YRqhUvqdPEQHudcT7ORCKdt2AY=
> 
==
> --- cfe/trunk/CMakeLists.txt (original)
> +++ cfe/trunk/CMakeLists.txt Fri Feb  8 16:46:12 2019
> @@ -411,9 +411,34 @@ option(CLANG_BUILD_TOOLS
>  option(CLANG_ENABLE_ARCMT "Build ARCMT." ON)
>  option(CLANG_ENABLE_STATIC_ANALYZER "Build static analyzer." ON)
>
> +set(CLANG_ANALYZER_Z3_INSTALL_DIR "" CACHE STRING "Install directory of 
the Z3 solver.")
> +
> +find_package(Z3 4.7.1)
> +
> +if (CLANG_ANALYZER_Z3_INSTALL_DIR)
> +  if (NOT Z3_FOUND)
> +message(FATAL_ERROR "Z3 4.7.1 has not been found in 
CLANG_ANALYZER_Z3_INSTALL_DIR: ${CLANG_ANALYZER_Z3_INSTALL_DIR}.")
> +  endif()
> +endif()
> +
> +set(CLANG_ANALYZER_ENABLE_Z3_SOLVER_DEFAULT "${Z3_FOUND}")
> +
> +option(CLANG_ANALYZER_ENABLE_Z3_SOLVER
> +  "Enable Support for the Z3 constraint solver in the Clang Static 
Analyzer."
> +  ${CLANG_ANALYZER_ENABLE_Z3_SOLVER_DEFAULT}
> +)
> +
> +if (CLANG_ANALYZER_ENABLE_Z3_SOLVER)
> +  if (NOT Z3_FOUND)
> +message(FATAL_ERROR "CLANG_ANALYZER_ENABLE_Z3_SOLVER cannot be 
enabled when Z3 is not available.")
> +  endif()
> +
> +  set(CLANG_ANALYZER_WITH_Z3 1)
> +endif()
> +
>  option(CLANG_ENABLE_PROTO_FUZZER "Build Clang protobuf fuzzer." OFF)
>
> -if(NOT CLANG_ENABLE_STATIC_ANALYZER AND CLANG_ENABLE_ARCMT)
> +if(NOT CLANG_ENABLE_STATIC_ANALYZER AND (CLANG_ENABLE_ARCMT OR 
CLANG_ANALYZER_ENABLE_Z3_SOLVER))
>message(FATAL_ERROR "Cannot disable static analyzer while enabling 
ARCMT or Z3")
>  endif()
>
>
> Added: cfe/trunk/cmake/modules/FindZ3.cmake
> URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_cmake_modules_FindZ3.cmake-3Frev-3D353590-26view-3Dauto=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=aepigNtcB7YIT8UBidW_1L_WrJGW-Zw_5JscfbewUQw=yw58_RYhB1Z7hXxc4_EPOOivPWp9UOdBBnyvwRHLe7M=
> 
==
> --- cfe/trunk/cmake/modules/FindZ3.cmake (added)
> +++ cfe/trunk/cmake/modules/FindZ3.cmake Fri Feb  8 16:46:12 2019
> @@ -0,0 +1,51 @@
> +# Looking for Z3 in CLANG_ANALYZER_Z3_INSTALL_DIR
> +find_path(Z3_INCLUDE_DIR NAMES z3.h
> +   NO_DEFAULT_PATH
> +   PATHS ${CLANG_ANALYZER_Z3_INSTALL_DIR}/include
> +   PATH_SUFFIXES libz3 z3
> +   )
> +
> +find_library(Z3_LIBRARIES NAMES z3 libz3
> +   NO_DEFAULT_PATH
> +   PATHS ${CLANG_ANALYZER_Z3_INSTALL_DIR}
> +   PATH_SUFFIXES 

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin marked 4 inline comments as done.
aheejin added inline comments.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:29
+  Pthread =
+  DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false);
+  ThreadModel =

tlively wrote:
> Shouldn't every use of `hasFlag` be `getLastArgValue` instead?
`hasFlag` is a convenient way to check everything with one function call. with 
`getLastArgValue` we have to call it twice (for example, for `-pthread` and for 
`-no-pthread`), and most importantly when both of them are given, calling 
`getLastArgValue` doesn't give you information on which one is the last. 
`hasFlag` takes care of that by taking the last one when both of them are 
given. So `-pthread -no-pthread` will return false, and `-no-pthread -pthread` 
will return true.

The reason I used `hasArg` below is just to check if the user gave it 
explicitly or not. And that's the reason I named variables `Pthread` and 
`HasPthread`.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:36
+  // Did user explicitly specify -mthread-model / -pthread?
+  bool HasThreadModel = DriverArgs.hasArg(options::OPT_mthread_model);
+  bool HasPthread = Pthread && DriverArgs.hasArg(options::OPT_pthread);

tlively wrote:
> It doesn't matter whether the user included the -pthread flag if they later 
> included the -no-pthread flag.
This `HasThreadModel` is only used with `HasPthread` below.
```
if (HasPthread && HasThreadModel && ThreadModel != "posix")
```

So this is just to check if this thread model value came from the default value 
or the user explicitly gave it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

RE: r353569 - [Sema] Make string literal init an rvalue.

[Eli Friedman via cfe-commits]

Fixed in r353598.

-Eli

> -Original Message-
> From: cfe-commits  On Behalf Of Eli
> Friedman via cfe-commits
> Sent: Friday, February 8, 2019 6:06 PM
> To: douglas.y...@sony.com
> Cc: cfe-commits@lists.llvm.org
> Subject: [EXT] RE: r353569 - [Sema] Make string literal init an rvalue.
> 
> Looking.  It looks like this only fails with clang-tidy, so I'll give myself 
> a few
> minutes to look before reverting.
> 
> -Eli
> 
> > -Original Message-
> > From: douglas.y...@sony.com 
> > Sent: Friday, February 8, 2019 3:58 PM
> > To: Eli Friedman 
> > Cc: cfe-commits@lists.llvm.org
> > Subject: [EXT] RE: r353569 - [Sema] Make string literal init an rvalue.
> >
> > Hi Eli,
> >
> > Your commit is causing a failure on the PS4 linux bot, can you please take a
> > look?
> >
> > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-
> > fast/builds/43625
> >
> > FAIL: Clang Tools :: clang-tidy/bugprone-string-constructor.cpp (14325 of
> > 46835)
> >  TEST 'Clang Tools :: clang-tidy/bugprone-string-
> > constructor.cpp' FAILED 
> > Script:
> > --
> > : 'RUN: at line 1';   /usr/bin/python2.7 
> > /home/buildslave/ps4-buildslave4/llvm-
> > clang-lld-x86_64-scei-ps4-ubuntu-
> > fast/llvm.src/tools/clang/tools/extra/test/../test/clang-
> > tidy/check_clang_tidy.py /home/buildslave/ps4-buildslave4/llvm-clang-lld-
> > x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-
> > tidy/bugprone-string-constructor.cpp bugprone-string-constructor
> > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-
> > fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/bugprone-string-
> > constructor.cpp.tmp
> > --
> > Exit Code: 1
> >
> > Command Output (stdout):
> > --
> > Running ['clang-tidy', '/home/buildslave/ps4-buildslave4/llvm-clang-lld-
> x86_64-
> > scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-
> > tidy/Output/bugprone-string-constructor.cpp.tmp.cpp', '-fix', '--checks=-
> > *,bugprone-string-constructor', '--', '--std=c++11', '-nostdinc++']...
> > clang-tidy failed:
> > clang-tidy: /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-
> > ubuntu-fast/llvm.src/tools/clang/lib/AST/ExprConstant.cpp:3374: bool
> > handleLValueToRValueConversion((anonymous namespace)::EvalInfo &, const
> > clang::Expr *, clang::QualType, const (anonymous namespace)::LValue &,
> > clang::APValue &): Assertion `LVal.Designator.Entries.size() == 1 && "Can 
> > only
> > read characters from string literals"' failed.
> >  #0 0x004ad2c4 (clang-tidy+0x4ad2c4)
> >  #1 0x004ab03c (clang-tidy+0x4ab03c)
> >  #2 0x004ad878 (clang-tidy+0x4ad878)
> >  #3 0x7f034560b890 __restore_rt (/lib/x86_64-linux-
> > gnu/libpthread.so.0+0x12890)
> >  #4 0x7f03442d1e97 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x3ee97)
> >  #5 0x7f03442d3801 abort (/lib/x86_64-linux-gnu/libc.so.6+0x40801)
> >  #6 0x7f03442c339a (/lib/x86_64-linux-gnu/libc.so.6+0x3039a)
> >  #7 0x7f03442c3412 (/lib/x86_64-linux-gnu/libc.so.6+0x30412)
> >  #8 0x01941c9d (clang-tidy+0x1941c9d)
> >  #9 0x0192b797 (clang-tidy+0x192b797)
> > #10 0x019266be (clang-tidy+0x19266be)
> > #11 0x005771f9 (clang-tidy+0x5771f9)
> > #12 0x01769d11 (clang-tidy+0x1769d11)
> > #13 0x01782d4b (clang-tidy+0x1782d4b)
> > #14 0x01769497 (clang-tidy+0x1769497)
> > #15 0x01774613 (clang-tidy+0x1774613)
> > #16 0x0177161f (clang-tidy+0x177161f)
> > #17 0x01782ad2 (clang-tidy+0x1782ad2)
> > #18 0x0176b3e6 (clang-tidy+0x176b3e6)
> > #19 0x0177f17d (clang-tidy+0x177f17d)
> > #20 0x0177161f (clang-tidy+0x177161f)
> > #21 0x017829f3 (clang-tidy+0x17829f3)
> > #22 0x0176b0bd (clang-tidy+0x176b0bd)
> > #23 0x0176e8b7 (clang-tidy+0x176e8b7)
> > #24 0x0176cfae (clang-tidy+0x176cfae)
> > #25 0x0174981f (clang-tidy+0x174981f)
> > #26 0x00c5020c (clang-tidy+0xc5020c)
> > #27 0x00d98873 (clang-tidy+0xd98873)
> > #28 0x00c381c0 (clang-tidy+0xc381c0)
> > #29 0x00bdcd31 (clang-tidy+0xbdcd31)
> > #30 0x00834386 (clang-tidy+0x834386)
> > #31 0x004ccba5 (clang-tidy+0x4ccba5)
> > #32 0x008340f6 (clang-tidy+0x8340f6)
> > #33 0x00833997 (clang-tidy+0x833997)
> > #34 0x0083579c (clang-tidy+0x83579c)
> > #35 0x004c9505 (clang-tidy+0x4c9505)
> > #36 0x0041d427 (clang-tidy+0x41d427)
> > #37 0x7f03442b4b97 __libc_start_main (/lib/x86_64-linux-
> > gnu/libc.so.6+0x21b97)
> > #38 0x0041b2fa (clang-tidy+0x41b2fa)
> >
> >
> > --
> > Command Output (stderr):
> > --
> > Traceback (most recent call last):
> >   File "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-
> ubuntu-
> > fast/llvm.src/tools/clang/tools/extra/test/../test/clang-
> > tidy/check_clang_tidy.py", line 207, in 
> > main()
> >   File 

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin updated this revision to Diff 186093.
aheejin added a comment.

- Address comments


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874

Files:
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  test/Driver/wasm-toolchain.c
  test/Preprocessor/wasm-target-features.c

Index: test/Preprocessor/wasm-target-features.c
===
--- test/Preprocessor/wasm-target-features.c
+++ test/Preprocessor/wasm-target-features.c
@@ -52,11 +52,13 @@
 //
 // BULK-MEMORY:#define __wasm_bulk_memory__ 1{{$}}
 //
+// We don't use -matomics directly and '-mthread-model posix' sets the atomics
+// target feature.
 // RUN: %clang -E -dM %s -o - 2>&1 \
-// RUN: -target wasm32-unknown-unknown -matomics \
+// RUN: -target wasm32-unknown-unknown -mthread-model posix \
 // RUN:   | FileCheck %s -check-prefix=ATOMICS
 // RUN: %clang -E -dM %s -o - 2>&1 \
-// RUN: -target wasm64-unknown-unknown -matomics \
+// RUN: -target wasm64-unknown-unknown -mthread-model posix \
 // RUN:   | FileCheck %s -check-prefix=ATOMICS
 //
 // ATOMICS:#define __wasm_atomics__ 1{{$}}
Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -38,3 +38,16 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+
+// Thread-related command line tests.
+// - '-mthread-model' sets '-target-feature +matomics'
+// - '-phread' sets both '-target-featuer +atomics' and '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mthread-model posix 2>&1 | FileCheck -check-prefix=POSIX %s
+// POSIX: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread 2>&1 | FileCheck -check-prefix=PTHREAD %s
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR %s
+// THREAD_OPT_ERROR: invalid argument '-pthread' not allowed with '-mthread-model single'
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -64,7 +64,7 @@
   llvm::opt::ArgStringList ) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
-  std::string getThreadModel() const override;
+  std::string getThreadModel(const llvm::opt::ArgList &) const override;
 
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -20,6 +20,27 @@
 using namespace clang;
 using namespace llvm::opt;
 
+void parseThreadArgs(const Driver , const ArgList ,
+ bool , StringRef ,
+ bool CheckForErrors = true) {
+  // Default value for -pthread / -mthread-model options, each being false /
+  // "single".
+  Pthread =
+  DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false);
+  ThreadModel =
+  DriverArgs.getLastArgValue(options::OPT_mthread_model, "single");
+  if (!CheckForErrors)
+return;
+
+  // Did user explicitly specify -mthread-model / -pthread?
+  bool HasThreadModel = DriverArgs.hasArg(options::OPT_mthread_model);
+  bool HasPthread = Pthread && DriverArgs.hasArg(options::OPT_pthread);
+  // '-pthread' cannot be used with '-mthread-model single'
+  if (HasPthread && HasThreadModel && ThreadModel == "single")
+Driver.Diag(diag::err_drv_argument_not_allowed_with)
+<< "-pthread" << "-mthread-model single";
+}
+
 wasm::Linker::Linker(const ToolChain )
 : GnuTool("wasm::Linker", "lld", TC) {}
 
@@ -123,6 +144,17 @@
   if (DriverArgs.hasFlag(clang::driver::options::OPT_fuse_init_array,
  options::OPT_fno_use_init_array, true))
 CC1Args.push_back("-fuse-init-array");
+
+  // Either '-mthread-model posix' or '-pthread' sets '-target-feature
+  // +atomics'. We intentionally didn't create '-matomics' and set the atomics
+  // target feature here depending on the other two options.
+  bool 

r353598 - Fix buildbot failure from r353569.

[Eli Friedman via cfe-commits]

Author: efriedma
Date: Fri Feb  8 18:22:17 2019
New Revision: 353598

URL: http://llvm.org/viewvc/llvm-project?rev=353598=rev
Log:
Fix buildbot failure from r353569.

I assumed lvalue-to-rvalue conversions of array type would never
happen, but apparently clang-tidy tries in some cases.


Modified:
cfe/trunk/lib/AST/ExprConstant.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=353598=353597=353598=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Feb  8 18:22:17 2019
@@ -3370,8 +3370,15 @@ static bool handleLValueToRValueConversi
 } else if (isa(Base) || isa(Base)) {
   // Special-case character extraction so we don't have to construct an
   // APValue for the whole string.
-  assert(LVal.Designator.Entries.size() == 1 &&
+  assert(LVal.Designator.Entries.size() <= 1 &&
  "Can only read characters from string literals");
+  if (LVal.Designator.Entries.empty()) {
+// Fail for now for LValue to RValue conversion of an array.
+// (This shouldn't show up in C/C++, but it could be triggered by a
+// weird EvaluateAsRValue call from a tool.)
+Info.FFDiag(Conv);
+return false;
+  }
   if (LVal.Designator.isOnePastTheEnd()) {
 if (Info.getLangOpts().CPlusPlus11)
   Info.FFDiag(Conv, diag::note_constexpr_access_past_end) << AK_Read;


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

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks cool, thanks!!




Comment at: include/clang/Basic/DiagnosticDriverKinds.td:304-305
 def err_analyzer_config_unknown : Error<"unknown analyzer-config '%0'">;
+def err_analyzer_checker_option_invalid_input : Error<
+  "invalid input for checker option '%0', that expects %1 value">;
 

I suggest hardcoding the word "value" in every message rather than putting it 
here, because it may be desirable to substitute it with something more specific 
or more accurate in every checker's specific circumstances.

Eg., "a non-negative integer" would be more precise than "a non-negative value".



Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:352-355
+  if (Checker->AllowedPad < 0)
+
Mgr.getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input)
+<< (llvm::Twine() + Checker->getTagDescription() + ":AllowedPad").str()
+<< "a non-negative";


I passively wish for a certain amount of de-duplication that wouldn't require 
every checker to obtain a diagnostics engine every time it tries to read an 
option. Eg.,
```lang=c++
  auto *Checker = Mgr.registerChecker();
  Checker->AllowedPad = Mgr.getAnalyzerOptions()
  .getCheckerIntegerOption(Checker, "AllowedPad", 24);
  if (Checker->AllowedPad < 0)
Mgr.reportInvalidOptionValue(Checker, "AllowedPad", "a non-negative");
```

Or maybe even something like that:

```lang=c++
  auto *Checker = Mgr.registerChecker();
  Checker->AllowedPad = Mgr.getAnalyzerOptions()
  .getCheckerIntegerOption(Checker, "AllowedPad", 24,
  [](int x) -> Option {
  if (x < 0) {
// Makes getCheckerIntegerOption() emit a diagnostic
// and return the default value.
return "a non-negative";
  }
  // Makes getCheckerIntegerOption() successfully return
  // the user-specified value.
  return None;
  });
```
I.e., a validator lambda.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57850



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

RE: r353569 - [Sema] Make string literal init an rvalue.

[Eli Friedman via cfe-commits]

Looking.  It looks like this only fails with clang-tidy, so I'll give myself a 
few minutes to look before reverting.
 
-Eli 

> -Original Message-
> From: douglas.y...@sony.com 
> Sent: Friday, February 8, 2019 3:58 PM
> To: Eli Friedman 
> Cc: cfe-commits@lists.llvm.org
> Subject: [EXT] RE: r353569 - [Sema] Make string literal init an rvalue.
> 
> Hi Eli,
> 
> Your commit is causing a failure on the PS4 linux bot, can you please take a
> look?
> 
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-
> fast/builds/43625
> 
> FAIL: Clang Tools :: clang-tidy/bugprone-string-constructor.cpp (14325 of
> 46835)
>  TEST 'Clang Tools :: clang-tidy/bugprone-string-
> constructor.cpp' FAILED 
> Script:
> --
> : 'RUN: at line 1';   /usr/bin/python2.7 
> /home/buildslave/ps4-buildslave4/llvm-
> clang-lld-x86_64-scei-ps4-ubuntu-
> fast/llvm.src/tools/clang/tools/extra/test/../test/clang-
> tidy/check_clang_tidy.py /home/buildslave/ps4-buildslave4/llvm-clang-lld-
> x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-
> tidy/bugprone-string-constructor.cpp bugprone-string-constructor
> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-
> fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/bugprone-string-
> constructor.cpp.tmp
> --
> Exit Code: 1
> 
> Command Output (stdout):
> --
> Running ['clang-tidy', 
> '/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-
> scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-
> tidy/Output/bugprone-string-constructor.cpp.tmp.cpp', '-fix', '--checks=-
> *,bugprone-string-constructor', '--', '--std=c++11', '-nostdinc++']...
> clang-tidy failed:
> clang-tidy: /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-
> ubuntu-fast/llvm.src/tools/clang/lib/AST/ExprConstant.cpp:3374: bool
> handleLValueToRValueConversion((anonymous namespace)::EvalInfo &, const
> clang::Expr *, clang::QualType, const (anonymous namespace)::LValue &,
> clang::APValue &): Assertion `LVal.Designator.Entries.size() == 1 && "Can only
> read characters from string literals"' failed.
>  #0 0x004ad2c4 (clang-tidy+0x4ad2c4)
>  #1 0x004ab03c (clang-tidy+0x4ab03c)
>  #2 0x004ad878 (clang-tidy+0x4ad878)
>  #3 0x7f034560b890 __restore_rt (/lib/x86_64-linux-
> gnu/libpthread.so.0+0x12890)
>  #4 0x7f03442d1e97 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x3ee97)
>  #5 0x7f03442d3801 abort (/lib/x86_64-linux-gnu/libc.so.6+0x40801)
>  #6 0x7f03442c339a (/lib/x86_64-linux-gnu/libc.so.6+0x3039a)
>  #7 0x7f03442c3412 (/lib/x86_64-linux-gnu/libc.so.6+0x30412)
>  #8 0x01941c9d (clang-tidy+0x1941c9d)
>  #9 0x0192b797 (clang-tidy+0x192b797)
> #10 0x019266be (clang-tidy+0x19266be)
> #11 0x005771f9 (clang-tidy+0x5771f9)
> #12 0x01769d11 (clang-tidy+0x1769d11)
> #13 0x01782d4b (clang-tidy+0x1782d4b)
> #14 0x01769497 (clang-tidy+0x1769497)
> #15 0x01774613 (clang-tidy+0x1774613)
> #16 0x0177161f (clang-tidy+0x177161f)
> #17 0x01782ad2 (clang-tidy+0x1782ad2)
> #18 0x0176b3e6 (clang-tidy+0x176b3e6)
> #19 0x0177f17d (clang-tidy+0x177f17d)
> #20 0x0177161f (clang-tidy+0x177161f)
> #21 0x017829f3 (clang-tidy+0x17829f3)
> #22 0x0176b0bd (clang-tidy+0x176b0bd)
> #23 0x0176e8b7 (clang-tidy+0x176e8b7)
> #24 0x0176cfae (clang-tidy+0x176cfae)
> #25 0x0174981f (clang-tidy+0x174981f)
> #26 0x00c5020c (clang-tidy+0xc5020c)
> #27 0x00d98873 (clang-tidy+0xd98873)
> #28 0x00c381c0 (clang-tidy+0xc381c0)
> #29 0x00bdcd31 (clang-tidy+0xbdcd31)
> #30 0x00834386 (clang-tidy+0x834386)
> #31 0x004ccba5 (clang-tidy+0x4ccba5)
> #32 0x008340f6 (clang-tidy+0x8340f6)
> #33 0x00833997 (clang-tidy+0x833997)
> #34 0x0083579c (clang-tidy+0x83579c)
> #35 0x004c9505 (clang-tidy+0x4c9505)
> #36 0x0041d427 (clang-tidy+0x41d427)
> #37 0x7f03442b4b97 __libc_start_main (/lib/x86_64-linux-
> gnu/libc.so.6+0x21b97)
> #38 0x0041b2fa (clang-tidy+0x41b2fa)
> 
> 
> --
> Command Output (stderr):
> --
> Traceback (most recent call last):
>   File 
> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-
> fast/llvm.src/tools/clang/tools/extra/test/../test/clang-
> tidy/check_clang_tidy.py", line 207, in 
> main()
>   File 
> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-
> fast/llvm.src/tools/clang/tools/extra/test/../test/clang-
> tidy/check_clang_tidy.py", line 147, in main
> subprocess.check_output(args, stderr=subprocess.STDOUT).decode()
>   File "/usr/lib/python2.7/subprocess.py", line 223, in check_output
> raise CalledProcessError(retcode, cmd, output=output)
> subprocess.CalledProcessError: Command '['clang-tidy', '/home/buildslave/ps4-
> buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-
> 

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Thomas Lively via Phabricator via cfe-commits]

tlively added inline comments.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:29
+  Pthread =
+  DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false);
+  ThreadModel =

Shouldn't every use of `hasFlag` be `getLastArgValue` instead?



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:36
+  // Did user explicitly specify -mthread-model / -pthread?
+  bool HasThreadModel = DriverArgs.hasArg(options::OPT_mthread_model);
+  bool HasPthread = Pthread && DriverArgs.hasArg(options::OPT_pthread);

It doesn't matter whether the user included the -pthread flag if they later 
included the -no-pthread flag.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:42
+  // "posix")
+  if (HasPthread && HasThreadModel && ThreadModel != "posix")
+Driver.Diag(diag::err_drv_argument_not_allowed_with)

This should be `ThreadModel == "single"`, since the distinction we care about 
is single-threaded vs no single-threaded. If in the future there are dozens of 
thread models besides "posix" this logic should still work.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:157
+  parseThreadArgs(getDriver(), DriverArgs, Pthread, ThreadModel);
+  if (Pthread || ThreadModel == "posix") {
+CC1Args.push_back("-target-feature");

Same here. Should compare ThreadModel with "single".


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

In D57874#1389981 , @sunfish wrote:

> > - `-matomics` means `-mthread-model posix`
>
> The others sound reasonable, though this one seems a little surprising -- a 
> user might have -matomics enabled because they're targeting a VM that has 
> atomics, but still not want to use -mthread-model posix because their code 
> doesn't actually using threads.


FYI, we don't have `-matomics` anymore.




Comment at: lib/Driver/ToolChains/WebAssembly.cpp:50
+  bool HasNoPthread =
+  !Pthread && DriverArgs.hasArg(clang::driver::options::OPT_no_pthread);
+

sbc100 wrote:
> tlively wrote:
> > Should this logic use `getLastArg` or perhaps `getLastArgNoClaim` to check 
> > only that the final requested configuration is consistent rather than 
> > checking all intermediate configurations?
> Can you remove all the "clang::driver" namspace qualification here since 
> there is  a "using" above?
@sbc100 Yeah good catch.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:50
+  bool HasNoPthread =
+  !Pthread && DriverArgs.hasArg(clang::driver::options::OPT_no_pthread);
+

aheejin wrote:
> sbc100 wrote:
> > tlively wrote:
> > > Should this logic use `getLastArg` or perhaps `getLastArgNoClaim` to 
> > > check only that the final requested configuration is consistent rather 
> > > than checking all intermediate configurations?
> > Can you remove all the "clang::driver" namspace qualification here since 
> > there is  a "using" above?
> @sbc100 Yeah good catch.
@tlively I used `getLastArgValue` when I get the thread model string above:
```
ThreadModel = DriverArgs.getLastArgValue(
  clang::driver::options::OPT_mthread_model, "single");
```
This takes the last occurrence of this argument, and the second argument is the 
default value when the user didn't specify that option.

Here I used `hasArg` just to determine whether the user gave it explicitly or 
not, because we error out only when a user explicitly gives conflicting set of 
options.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:62
+Driver.Diag(diag::err_drv_argument_not_allowed_with) << "-matomics"
+ << "-no-pthread";
+  // '-mno-atomics' cannot be used with '-mthread-model posix'

tlively wrote:
> I'm not sure about this one, either. What if I want atomics for 
> multithreading but I don't want to link with libpthread?
Yeah good catch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin updated this revision to Diff 186090.
aheejin added a comment.

- Replace -matomics with -mthread-model posix in preprocessor test


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874

Files:
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  test/Driver/wasm-toolchain.c
  test/Preprocessor/wasm-target-features.c

Index: test/Preprocessor/wasm-target-features.c
===
--- test/Preprocessor/wasm-target-features.c
+++ test/Preprocessor/wasm-target-features.c
@@ -52,11 +52,13 @@
 //
 // BULK-MEMORY:#define __wasm_bulk_memory__ 1{{$}}
 //
+// We don't use -matomics directly and '-mthread-model posix' sets the atomics
+// target feature.
 // RUN: %clang -E -dM %s -o - 2>&1 \
-// RUN: -target wasm32-unknown-unknown -matomics \
+// RUN: -target wasm32-unknown-unknown -mthread-model posix \
 // RUN:   | FileCheck %s -check-prefix=ATOMICS
 // RUN: %clang -E -dM %s -o - 2>&1 \
-// RUN: -target wasm64-unknown-unknown -matomics \
+// RUN: -target wasm64-unknown-unknown -mthread-model posix \
 // RUN:   | FileCheck %s -check-prefix=ATOMICS
 //
 // ATOMICS:#define __wasm_atomics__ 1{{$}}
Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -38,3 +38,16 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+
+// Thread-related command line tests.
+// - '-mthread-model' sets '-target-feature +matomics'
+// - '-phread' sets both '-target-featuer +atomics' and '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mthread-model posix 2>&1 | FileCheck -check-prefix=POSIX %s
+// POSIX: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread 2>&1 | FileCheck -check-prefix=PTHREAD %s
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR %s
+// THREAD_OPT_ERROR: invalid argument '-pthread' not allowed with '-mthread-model single'
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -64,7 +64,7 @@
   llvm::opt::ArgStringList ) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
-  std::string getThreadModel() const override;
+  std::string getThreadModel(const llvm::opt::ArgList &) const override;
 
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -20,6 +20,30 @@
 using namespace clang;
 using namespace llvm::opt;
 
+void parseThreadArgs(const Driver , const ArgList ,
+ bool , StringRef ,
+ bool CheckForErrors = true) {
+  // Default value for -pthread / -mthread-model options, each being false /
+  // "single".
+  Pthread =
+  DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false);
+  ThreadModel =
+  DriverArgs.getLastArgValue(options::OPT_mthread_model, "single");
+  if (!CheckForErrors)
+return;
+
+  // Did user explicitly specify -mthread-model / -pthread?
+  bool HasThreadModel = DriverArgs.hasArg(options::OPT_mthread_model);
+  bool HasPthread = Pthread && DriverArgs.hasArg(options::OPT_pthread);
+  std::string ThreadModelOpt =
+  std::string("-mthread-model ") + ThreadModel.data();
+  // '-pthread' cannot be used with '-mthread-model single' (or anything not
+  // "posix")
+  if (HasPthread && HasThreadModel && ThreadModel != "posix")
+Driver.Diag(diag::err_drv_argument_not_allowed_with)
+<< "-pthread" << ThreadModelOpt;
+}
+
 wasm::Linker::Linker(const ToolChain )
 : GnuTool("wasm::Linker", "lld", TC) {}
 
@@ -123,6 +147,17 @@
   if (DriverArgs.hasFlag(clang::driver::options::OPT_fuse_init_array,
  options::OPT_fno_use_init_array, true))
 CC1Args.push_back("-fuse-init-array");
+
+  // Either '-mthread-model posix' or '-pthread' sets 

[PATCH] D57991: [Driver][Darwin] Emit an error when using -pg on OS without support for it.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: dexonsmith, bob.wilson, steven_wu.
Herald added a subscriber: jkorous.

Instead of letting a program fail at runtime, emit an error during
compilation.

rdar://problem/12206955


https://reviews.llvm.org/D57991

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld.c


Index: clang/test/Driver/darwin-ld.c
===
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -203,6 +203,14 @@
 // LINK_PG: -lgcrt1.o
 // LINK_PG: -no_new_main
 
+// RUN: %clang -target i386-apple-darwin13 -pg -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=LINK_PG_NO_SUPPORT_OSX %s < %t.log
+// LINK_PG_NO_SUPPORT_OSX: error: the clang compiler does not support -pg 
option on versions of OS X
+
+// RUN: %clang -target x86_64-apple-ios5.0 -pg -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=LINK_PG_NO_SUPPORT %s < %t.log
+// LINK_PG_NO_SUPPORT: error: the clang compiler does not support -pg option 
on Darwin
+
 // Check that clang links with libgcc_s.1 for iOS 4 and earlier, but not arm64.
 // RUN: %clang -target armv7-apple-ios4.0 -miphoneos-version-min=4.0 -### %t.o 
2> %t.log
 // RUN: FileCheck -check-prefix=LINK_IOS_LIBGCC_S %s < %t.log
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2289,22 +2289,29 @@
   }
 } else {
   if (Args.hasArg(options::OPT_pg) && SupportsProfiling()) {
-if (Args.hasArg(options::OPT_static) ||
-Args.hasArg(options::OPT_object) ||
-Args.hasArg(options::OPT_preload)) {
-  CmdArgs.push_back("-lgcrt0.o");
-} else {
-  CmdArgs.push_back("-lgcrt1.o");
+if (isTargetMacOS() && isMacosxVersionLT(10, 9)) {
+  if (Args.hasArg(options::OPT_static) ||
+  Args.hasArg(options::OPT_object) ||
+  Args.hasArg(options::OPT_preload)) {
+CmdArgs.push_back("-lgcrt0.o");
+  } else {
+CmdArgs.push_back("-lgcrt1.o");
 
-  // darwin_crt2 spec is empty.
+// darwin_crt2 spec is empty.
+  }
+  // By default on OS X 10.8 and later, we don't link with a crt1.o
+  // file and the linker knows to use _main as the entry point.  But,
+  // when compiling with -pg, we need to link with the gcrt1.o file,
+  // so pass the -no_new_main option to tell the linker to use the
+  // "start" symbol as the entry point.
+  if (isTargetMacOS() && !isMacosxVersionLT(10, 8))
+CmdArgs.push_back("-no_new_main");
+} else {
+  getDriver().Diag(
+  isTargetMacOS()
+  ? diag::err_drv_clang_unsupported_opt_pg_darwin_osx
+  : diag::err_drv_clang_unsupported_opt_pg_darwin);
 }
-// By default on OS X 10.8 and later, we don't link with a crt1.o
-// file and the linker knows to use _main as the entry point.  But,
-// when compiling with -pg, we need to link with the gcrt1.o file,
-// so pass the -no_new_main option to tell the linker to use the
-// "start" symbol as the entry point.
-if (isTargetMacOS() && !isMacosxVersionLT(10, 8))
-  CmdArgs.push_back("-no_new_main");
   } else {
 if (Args.hasArg(options::OPT_static) ||
 Args.hasArg(options::OPT_object) ||
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -96,6 +96,10 @@
   "the clang compiler does not support '%0'">;
 def err_drv_clang_unsupported_opt_cxx_darwin_i386 : Error<
   "the clang compiler does not support '%0' for C++ on Darwin/i386">;
+def err_drv_clang_unsupported_opt_pg_darwin: Error<
+  "the clang compiler does not support -pg option on Darwin">;
+def err_drv_clang_unsupported_opt_pg_darwin_osx: Error<
+  "the clang compiler does not support -pg option on versions of OS X 10.9 and 
later">;
 def err_drv_clang_unsupported_opt_faltivec : Error<
   "the clang compiler does not support '%0', %1">;
 def err_drv_command_failed : Error<


Index: clang/test/Driver/darwin-ld.c
===
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -203,6 +203,14 @@
 // LINK_PG: -lgcrt1.o
 // LINK_PG: -no_new_main
 
+// RUN: %clang -target i386-apple-darwin13 -pg -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=LINK_PG_NO_SUPPORT_OSX %s < %t.log
+// LINK_PG_NO_SUPPORT_OSX: error: the clang compiler does not support -pg option on versions of OS X
+
+// RUN: %clang -target x86_64-apple-ios5.0 -pg -### %t.o 2> %t.log
+// 

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin updated this revision to Diff 186088.
aheejin marked 8 inline comments as done.
aheejin added a comment.

I had an offline discussion with @tlively and @dschuff and decided to remove 
`-atomics` option in the driver. Instead, `clang -cc1`'s `-target-feature 
+atomics` will be either of `-pthread` or `-mthread-model posix` is given. This 
is to reduce the total number of options and thus the total number of 
combinations of options. Sorry for frequent changes. Also addressed comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874

Files:
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  test/Driver/wasm-toolchain.c

Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -38,3 +38,16 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+
+// Thread-related command line tests.
+// - '-mthread-model' sets '-target-feature +matomics'
+// - '-phread' sets both '-target-featuer +atomics' and '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mthread-model posix 2>&1 | FileCheck -check-prefix=POSIX %s
+// POSIX: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread 2>&1 | FileCheck -check-prefix=PTHREAD %s
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR %s
+// THREAD_OPT_ERROR: invalid argument '-pthread' not allowed with '-mthread-model single'
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -64,7 +64,7 @@
   llvm::opt::ArgStringList ) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
-  std::string getThreadModel() const override;
+  std::string getThreadModel(const llvm::opt::ArgList &) const override;
 
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -20,6 +20,30 @@
 using namespace clang;
 using namespace llvm::opt;
 
+void parseThreadArgs(const Driver , const ArgList ,
+ bool , StringRef ,
+ bool CheckForErrors = true) {
+  // Default value for -pthread / -mthread-model options, each being false /
+  // "single".
+  Pthread =
+  DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false);
+  ThreadModel =
+  DriverArgs.getLastArgValue(options::OPT_mthread_model, "single");
+  if (!CheckForErrors)
+return;
+
+  // Did user explicitly specify -mthread-model / -pthread?
+  bool HasThreadModel = DriverArgs.hasArg(options::OPT_mthread_model);
+  bool HasPthread = Pthread && DriverArgs.hasArg(options::OPT_pthread);
+  std::string ThreadModelOpt =
+  std::string("-mthread-model ") + ThreadModel.data();
+  // '-pthread' cannot be used with '-mthread-model single' (or anything not
+  // "posix")
+  if (HasPthread && HasThreadModel && ThreadModel != "posix")
+Driver.Diag(diag::err_drv_argument_not_allowed_with)
+<< "-pthread" << ThreadModelOpt;
+}
+
 wasm::Linker::Linker(const ToolChain )
 : GnuTool("wasm::Linker", "lld", TC) {}
 
@@ -123,6 +147,17 @@
   if (DriverArgs.hasFlag(clang::driver::options::OPT_fuse_init_array,
  options::OPT_fno_use_init_array, true))
 CC1Args.push_back("-fuse-init-array");
+
+  // Either '-mthread-model posix' or '-pthread' sets '-target-feature
+  // +atomics'. We intentionally didn't create '-matomics' and set the atomics
+  // target feature here depending on the other two options.
+  bool Pthread = false;
+  StringRef ThreadModel = "";
+  parseThreadArgs(getDriver(), DriverArgs, Pthread, ThreadModel);
+  if (Pthread || ThreadModel == "posix") {
+CC1Args.push_back("-target-feature");
+CC1Args.push_back("+atomics");
+  }
 }
 
 ToolChain::RuntimeLibType WebAssembly::GetDefaultRuntimeLibType() const {
@@ -180,12 +215,15 @@
   }
 }
 
-std::string 

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953
+  "have bases of non-trivial class types|have virtual bases|"
+  "have __weak fields under ARC|have fields of non-trivial class types}0">;
 

Quuxplusone wrote:
> ahatanak wrote:
> > Quuxplusone wrote:
> > > nit: "of non-trivial class types" should be "of non-trivial class type" 
> > > in both places.
> > > 
> > > And I would write "are not move-constructible" rather than "don't have 
> > > non-deleted copy/move constructors". Double negations aren't non-bad.
> > > 
> > > Actually I would rephrase this as `'trivial_abi' is disallowed on this 
> > > class because it %select{is not move-constructible|is polymorphic|has a 
> > > base of non-trivial class type|has a virtual base|has a __weak field|has 
> > > a field of non-trivial class type}`, i.e., we're not just giving 
> > > information about "classes" in general, we're talking about "this class" 
> > > specifically. We could even name the class if we're feeling generous.
> > Does not being move-constructible imply that the class doesn't have a 
> > *public* copy or move constructor that isn't deleted? If it does, that is 
> > slightly different than saying the copy and move constructors of the class 
> > are all deleted. When the users see the message "is not 
> > move-constructible", they might think the copy or move constructor that 
> > isn't deleted has to be public in order to annotate the class with 
> > `trivial_abi`. For `trivial_abi`, a private one is good enough.
> > 
> > I think your other suggestions are all good ideas.
> > Does not being move-constructible imply that the class doesn't have a 
> > *public* copy or move constructor that isn't deleted?
> 
> Yeah, I suppose so. Okay.
I changed the message to "its copy constructors and move constructors are all 
deleted".



Comment at: test/SemaObjCXX/attr-trivial-abi.mm:108
+S18(const S18 &);
+S18(S18 &&);
+  };

The diagnostic note printed here  was actually "have fields of non-trivial 
class types", not "don't have non-deleted copy/move constructors", because the 
class had explicitly declared copy and move constructors. I removed both 
declarations to make clang print the latter.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626



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

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 186083.
ahatanak marked 4 inline comments as done.
ahatanak added a comment.

Improve diagnostic messages.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626

Files:
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclCXX.cpp
  test/SemaObjCXX/attr-trivial-abi.mm

Index: test/SemaObjCXX/attr-trivial-abi.mm
===
--- test/SemaObjCXX/attr-trivial-abi.mm
+++ test/SemaObjCXX/attr-trivial-abi.mm
@@ -10,23 +10,23 @@
   int a;
 };
 
-struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}}
+struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}}
+struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}} expected-note {{is polymorphic}}
   virtual void m();
 };
 
 struct S3_2 {
   virtual void m();
-} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}}
+} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}} expected-note {{is polymorphic}}
 
 struct S4 {
   int a;
 };
 
-struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}}
+struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}} expected-note {{has a virtual base}}
 };
 
 struct __attribute__((trivial_abi)) S9 : public S4 {
@@ -36,19 +36,19 @@
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}}
+struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}}
+struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}} expected-note {{has a __weak field}}
   __weak id a[2];
 };
 
-struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}}
+struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}} expected-note {{has a field of a non-trivial class type}}
   S6 a;
 };
 
-struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}}
+struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}} expected-note {{has a field of a non-trivial class type}}
   S6 a[2];
 };
 
@@ -66,7 +66,7 @@
 S10<__weak id> p2;
 
 template<>
-struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10'}}
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
@@ -90,8 +90,39 @@
 S16 s16;
 
 template
-struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}}
+struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
 S17 s17;
+
+namespace deletedCopyMoveConstructor {
+  struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
+CopyMoveDeleted(const CopyMoveDeleted &) = delete;
+CopyMoveDeleted(CopyMoveDeleted &&) = delete;
+  };
+
+  struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
+CopyMoveDeleted a;
+  };
+
+  struct __attribute__((trivial_abi)) CopyDeleted {
+CopyDeleted(const CopyDeleted &) = delete;
+CopyDeleted(CopyDeleted &&) = default;
+  };
+
+  struct __attribute__((trivial_abi)) MoveDeleted {
+MoveDeleted(const MoveDeleted &) = default;
+MoveDeleted(MoveDeleted &&) = delete;
+  };
+
+  struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{copy constructors and move constructors are all deleted}}
+CopyDeleted a;
+MoveDeleted b;
+  };
+
+  // This is fine since the move constructor isn't deleted.
+  struct __attribute__((trivial_abi)) S20 {
+int & // a member of rvalue reference type deletes the copy constructor.
+  };
+}
Index: 

Re: r353590 - This reverts commit 1440a848a635849b97f7a5cfa0ecc40d37451f5b.

[Richard Smith via cfe-commits]

On Fri, 8 Feb 2019 at 16:45, Mikhail R. Gadelha via cfe-commits
 wrote:
> Author: mramalho
> Date: Fri Feb  8 16:46:12 2019
> New Revision: 353590
>
> URL: http://llvm.org/viewvc/llvm-project?rev=353590=rev
> Log:
> This reverts commit 1440a848a635849b97f7a5cfa0ecc40d37451f5b.
> and commit a1853e834c65751f92521f7481b15cf0365e796b.

Please include SVN revision numbers in revert messages until LLVM
moves over to git.

> They broke arm and aarch64
>
> Added:
> cfe/trunk/cmake/modules/FindZ3.cmake
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTAPI.h
> cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
> Removed:
> cfe/trunk/lib/StaticAnalyzer/Core/SMTConstraintManager.cpp
> Modified:
> cfe/trunk/CMakeLists.txt
> cfe/trunk/include/clang/Config/config.h.cmake
> 
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
> cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
> cfe/trunk/test/CMakeLists.txt
> cfe/trunk/test/lit.site.cfg.py.in
>
> Modified: cfe/trunk/CMakeLists.txt
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=353590=353589=353590=diff
> ==
> --- cfe/trunk/CMakeLists.txt (original)
> +++ cfe/trunk/CMakeLists.txt Fri Feb  8 16:46:12 2019
> @@ -411,9 +411,34 @@ option(CLANG_BUILD_TOOLS
>  option(CLANG_ENABLE_ARCMT "Build ARCMT." ON)
>  option(CLANG_ENABLE_STATIC_ANALYZER "Build static analyzer." ON)
>
> +set(CLANG_ANALYZER_Z3_INSTALL_DIR "" CACHE STRING "Install directory of the 
> Z3 solver.")
> +
> +find_package(Z3 4.7.1)
> +
> +if (CLANG_ANALYZER_Z3_INSTALL_DIR)
> +  if (NOT Z3_FOUND)
> +message(FATAL_ERROR "Z3 4.7.1 has not been found in 
> CLANG_ANALYZER_Z3_INSTALL_DIR: ${CLANG_ANALYZER_Z3_INSTALL_DIR}.")
> +  endif()
> +endif()
> +
> +set(CLANG_ANALYZER_ENABLE_Z3_SOLVER_DEFAULT "${Z3_FOUND}")
> +
> +option(CLANG_ANALYZER_ENABLE_Z3_SOLVER
> +  "Enable Support for the Z3 constraint solver in the Clang Static Analyzer."
> +  ${CLANG_ANALYZER_ENABLE_Z3_SOLVER_DEFAULT}
> +)
> +
> +if (CLANG_ANALYZER_ENABLE_Z3_SOLVER)
> +  if (NOT Z3_FOUND)
> +message(FATAL_ERROR "CLANG_ANALYZER_ENABLE_Z3_SOLVER cannot be enabled 
> when Z3 is not available.")
> +  endif()
> +
> +  set(CLANG_ANALYZER_WITH_Z3 1)
> +endif()
> +
>  option(CLANG_ENABLE_PROTO_FUZZER "Build Clang protobuf fuzzer." OFF)
>
> -if(NOT CLANG_ENABLE_STATIC_ANALYZER AND CLANG_ENABLE_ARCMT)
> +if(NOT CLANG_ENABLE_STATIC_ANALYZER AND (CLANG_ENABLE_ARCMT OR 
> CLANG_ANALYZER_ENABLE_Z3_SOLVER))
>message(FATAL_ERROR "Cannot disable static analyzer while enabling ARCMT 
> or Z3")
>  endif()
>
>
> Added: cfe/trunk/cmake/modules/FindZ3.cmake
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/FindZ3.cmake?rev=353590=auto
> ==
> --- cfe/trunk/cmake/modules/FindZ3.cmake (added)
> +++ cfe/trunk/cmake/modules/FindZ3.cmake Fri Feb  8 16:46:12 2019
> @@ -0,0 +1,51 @@
> +# Looking for Z3 in CLANG_ANALYZER_Z3_INSTALL_DIR
> +find_path(Z3_INCLUDE_DIR NAMES z3.h
> +   NO_DEFAULT_PATH
> +   PATHS ${CLANG_ANALYZER_Z3_INSTALL_DIR}/include
> +   PATH_SUFFIXES libz3 z3
> +   )
> +
> +find_library(Z3_LIBRARIES NAMES z3 libz3
> +   NO_DEFAULT_PATH
> +   PATHS ${CLANG_ANALYZER_Z3_INSTALL_DIR}
> +   PATH_SUFFIXES lib bin
> +   )
> +
> +find_program(Z3_EXECUTABLE z3
> +   NO_DEFAULT_PATH
> +   PATHS ${CLANG_ANALYZER_Z3_INSTALL_DIR}
> +   PATH_SUFFIXES bin
> +   )
> +
> +# If Z3 has not been found in CLANG_ANALYZER_Z3_INSTALL_DIR look in the 
> default directories
> +find_path(Z3_INCLUDE_DIR NAMES z3.h
> +   PATH_SUFFIXES libz3 z3
> +   )
> +
> +find_library(Z3_LIBRARIES NAMES z3 libz3
> +   PATH_SUFFIXES lib bin
> +   )
> +
> +find_program(Z3_EXECUTABLE z3
> +   PATH_SUFFIXES bin
> +   )
> +
> +if(Z3_INCLUDE_DIR AND Z3_LIBRARIES AND Z3_EXECUTABLE)
> +execute_process (COMMAND ${Z3_EXECUTABLE} -version
> +  OUTPUT_VARIABLE libz3_version_str
> +  ERROR_QUIET
> +  OUTPUT_STRIP_TRAILING_WHITESPACE)
> +
> +string(REGEX REPLACE "^Z3 version ([0-9.]+)" "\\1"
> +   Z3_VERSION_STRING "${libz3_version_str}")
> +unset(libz3_version_str)
> +endif()
> +
> +# handle the QUIETLY and REQUIRED arguments and set Z3_FOUND to TRUE if
> +# all listed variables are TRUE
> +include(FindPackageHandleStandardArgs)
> +FIND_PACKAGE_HANDLE_STANDARD_ARGS(Z3
> +  REQUIRED_VARS Z3_LIBRARIES Z3_INCLUDE_DIR
> +  VERSION_VAR Z3_VERSION_STRING)
> +
> +mark_as_advanced(Z3_INCLUDE_DIR Z3_LIBRARIES)
>
> Modified: cfe/trunk/include/clang/Config/config.h.cmake
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Config/config.h.cmake?rev=353590=353589=353590=diff
> 

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D57874#1391384 , @aheejin wrote:

> In D57874#1391133 , @tlively wrote:
>
> > In D57874#1389953 , @aheejin wrote:
> >
> > > Anyway, moved all logic to the driver layer and did this:
> > >
> > > - `-matomics` means `-mthread-model posix`
> > > - `-mthread-model posix` means `-matomics`
> > > - `-pthread` means both `-matomics` and `-mthread-model posix`
> >
> >
> > If you replace "-matomics" with "-mbulk-memory," I plan to duplicate the 
> > logic for items 2 and 3 above, but not 1. For bulk memory even more than 
> > atomics, there is a legitimate usecase for enabling it even in single 
> > threaded contexts (namely getting to use memory.copy and memory.fill). I 
> > wonder if consistency with how bulk memory works is a strong enough 
> > argument for dropping item 1 for atomics as well.
>
>
> Then now `-mthread-model posix` means both `-matomics` and `-mbulk-memory`, 
> `-matomics` means `-mthread-model posix`, `-mbulk-memory` means 
> `-mthread-model posix` but NOT `-matomics` means `mbulk-memory` and vice 
> versa, right? Oh god it's complicated BTW if you are planning to use 
> `-mthread-model` for turning on and off bulk memory too, why not 1?


We talked about this offline, but recapping here. We can't have `-mbulk-memory` 
imply `-thread-model posix` because a user might want bulk memory for access to 
instructions like memory.copy and memory.set (which are just more efficient 
primitives for memcpy and memset), but NOT want multithreading.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

r353592 - [analyzer] Add a comment that FunctionCodeRegions may also need canonicalization

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri Feb  8 17:00:32 2019
New Revision: 353592

URL: http://llvm.org/viewvc/llvm-project?rev=353592=rev
Log:
[analyzer] Add a comment that FunctionCodeRegions may also need canonicalization

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=353592=353591=353592=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Fri Feb  8 17:00:32 2019
@@ -1009,6 +1009,7 @@ MemRegionManager::getElementRegion(QualT
 
 const FunctionCodeRegion *
 MemRegionManager::getFunctionCodeRegion(const NamedDecl *FD) {
+  // To think: should we canonicalize the declaration here?
   return getSubRegion(FD, getCodeRegion());
 }
 


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

[PATCH] D54978: Move the SMT API to LLVM

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho added a comment.

Hi everyone, I just saw your messages and reverted the commits.

Sorry for the inconvenience, but for some reason I didn't get any email from 
the bots. Could you send me the link with the failure?

@brzycki, I'm using Ubuntu 18.04.2 and I'll try to reproduce the error.

@thakis, do you have any idea why this was not sent to the llvm mailing lists?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

r353590 - This reverts commit 1440a848a635849b97f7a5cfa0ecc40d37451f5b.

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Fri Feb  8 16:46:12 2019
New Revision: 353590

URL: http://llvm.org/viewvc/llvm-project?rev=353590=rev
Log:
This reverts commit 1440a848a635849b97f7a5cfa0ecc40d37451f5b.
and commit a1853e834c65751f92521f7481b15cf0365e796b.

They broke arm and aarch64

Added:
cfe/trunk/cmake/modules/FindZ3.cmake
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTAPI.h
cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
Removed:
cfe/trunk/lib/StaticAnalyzer/Core/SMTConstraintManager.cpp
Modified:
cfe/trunk/CMakeLists.txt
cfe/trunk/include/clang/Config/config.h.cmake

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
cfe/trunk/test/CMakeLists.txt
cfe/trunk/test/lit.site.cfg.py.in

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=353590=353589=353590=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Fri Feb  8 16:46:12 2019
@@ -411,9 +411,34 @@ option(CLANG_BUILD_TOOLS
 option(CLANG_ENABLE_ARCMT "Build ARCMT." ON)
 option(CLANG_ENABLE_STATIC_ANALYZER "Build static analyzer." ON)
 
+set(CLANG_ANALYZER_Z3_INSTALL_DIR "" CACHE STRING "Install directory of the Z3 
solver.")
+
+find_package(Z3 4.7.1)
+
+if (CLANG_ANALYZER_Z3_INSTALL_DIR)
+  if (NOT Z3_FOUND)
+message(FATAL_ERROR "Z3 4.7.1 has not been found in 
CLANG_ANALYZER_Z3_INSTALL_DIR: ${CLANG_ANALYZER_Z3_INSTALL_DIR}.")
+  endif()
+endif()
+
+set(CLANG_ANALYZER_ENABLE_Z3_SOLVER_DEFAULT "${Z3_FOUND}")
+
+option(CLANG_ANALYZER_ENABLE_Z3_SOLVER
+  "Enable Support for the Z3 constraint solver in the Clang Static Analyzer."
+  ${CLANG_ANALYZER_ENABLE_Z3_SOLVER_DEFAULT}
+)
+
+if (CLANG_ANALYZER_ENABLE_Z3_SOLVER)
+  if (NOT Z3_FOUND)
+message(FATAL_ERROR "CLANG_ANALYZER_ENABLE_Z3_SOLVER cannot be enabled 
when Z3 is not available.")
+  endif()
+
+  set(CLANG_ANALYZER_WITH_Z3 1)
+endif()
+
 option(CLANG_ENABLE_PROTO_FUZZER "Build Clang protobuf fuzzer." OFF)
 
-if(NOT CLANG_ENABLE_STATIC_ANALYZER AND CLANG_ENABLE_ARCMT)
+if(NOT CLANG_ENABLE_STATIC_ANALYZER AND (CLANG_ENABLE_ARCMT OR 
CLANG_ANALYZER_ENABLE_Z3_SOLVER))
   message(FATAL_ERROR "Cannot disable static analyzer while enabling ARCMT or 
Z3")
 endif()
 

Added: cfe/trunk/cmake/modules/FindZ3.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/FindZ3.cmake?rev=353590=auto
==
--- cfe/trunk/cmake/modules/FindZ3.cmake (added)
+++ cfe/trunk/cmake/modules/FindZ3.cmake Fri Feb  8 16:46:12 2019
@@ -0,0 +1,51 @@
+# Looking for Z3 in CLANG_ANALYZER_Z3_INSTALL_DIR
+find_path(Z3_INCLUDE_DIR NAMES z3.h
+   NO_DEFAULT_PATH
+   PATHS ${CLANG_ANALYZER_Z3_INSTALL_DIR}/include
+   PATH_SUFFIXES libz3 z3
+   )
+
+find_library(Z3_LIBRARIES NAMES z3 libz3
+   NO_DEFAULT_PATH
+   PATHS ${CLANG_ANALYZER_Z3_INSTALL_DIR}
+   PATH_SUFFIXES lib bin
+   )
+
+find_program(Z3_EXECUTABLE z3
+   NO_DEFAULT_PATH
+   PATHS ${CLANG_ANALYZER_Z3_INSTALL_DIR}
+   PATH_SUFFIXES bin
+   )
+
+# If Z3 has not been found in CLANG_ANALYZER_Z3_INSTALL_DIR look in the 
default directories
+find_path(Z3_INCLUDE_DIR NAMES z3.h
+   PATH_SUFFIXES libz3 z3
+   )
+
+find_library(Z3_LIBRARIES NAMES z3 libz3
+   PATH_SUFFIXES lib bin
+   )
+
+find_program(Z3_EXECUTABLE z3
+   PATH_SUFFIXES bin
+   )
+
+if(Z3_INCLUDE_DIR AND Z3_LIBRARIES AND Z3_EXECUTABLE)
+execute_process (COMMAND ${Z3_EXECUTABLE} -version
+  OUTPUT_VARIABLE libz3_version_str
+  ERROR_QUIET
+  OUTPUT_STRIP_TRAILING_WHITESPACE)
+
+string(REGEX REPLACE "^Z3 version ([0-9.]+)" "\\1"
+   Z3_VERSION_STRING "${libz3_version_str}")
+unset(libz3_version_str)
+endif()
+
+# handle the QUIETLY and REQUIRED arguments and set Z3_FOUND to TRUE if
+# all listed variables are TRUE
+include(FindPackageHandleStandardArgs)
+FIND_PACKAGE_HANDLE_STANDARD_ARGS(Z3
+  REQUIRED_VARS Z3_LIBRARIES Z3_INCLUDE_DIR
+  VERSION_VAR Z3_VERSION_STRING)
+
+mark_as_advanced(Z3_INCLUDE_DIR Z3_LIBRARIES)

Modified: cfe/trunk/include/clang/Config/config.h.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Config/config.h.cmake?rev=353590=353589=353590=diff
==
--- cfe/trunk/include/clang/Config/config.h.cmake (original)
+++ cfe/trunk/include/clang/Config/config.h.cmake Fri Feb  8 16:46:12 2019
@@ -54,6 +54,9 @@
 /* Define if we have libxml2 */
 #cmakedefine CLANG_HAVE_LIBXML ${CLANG_HAVE_LIBXML}
 
+/* Define if we have z3 and want to build it */
+#cmakedefine CLANG_ANALYZER_WITH_Z3 ${CLANG_ANALYZER_WITH_Z3}
+
 /* 

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

tools/llvm-profdata/instr-remap.test is failing in the reverse iteration bot. 
See 
http://lab.llvm.org:8011/builders/reverse-iteration/builds/10546/steps/check_all/logs/stdio.

This is because FunctionData is iterated in InstrProfWriter to output function 
counts, etc. But for two functions with the same name the iteration order is 
not defined if we use a DenseMap. Changing this to MapVector resolves this.

Note: We can also sort FunctionData before iteration to fix this issue. But 
that would mean we would need to sort every time we iterate in order to print. 
Using a MapVector instead is a better option IMO.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57986



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

r353588 - [AMDGPU] Split dot-insts feature

[Stanislav Mekhanoshin via cfe-commits]

Author: rampitec
Date: Fri Feb  8 16:34:41 2019
New Revision: 353588

URL: http://llvm.org/viewvc/llvm-project?rev=353588=rev
Log:
[AMDGPU] Split dot-insts feature

Differential Revision: https://reviews.llvm.org/D57972

Modified:
cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl
cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-dl-insts-err.cl

Modified: cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def?rev=353588=353587=353588=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def Fri Feb  8 16:34:41 2019
@@ -145,13 +145,13 @@ TARGET_BUILTIN(__builtin_amdgcn_fmed3h,
 // Deep learning builtins.
 
//===--===//
 
-TARGET_BUILTIN(__builtin_amdgcn_fdot2, "fV2hV2hfIb", "nc", "dot-insts")
-TARGET_BUILTIN(__builtin_amdgcn_sdot2, "SiV2SsV2SsSiIb", "nc", "dot-insts")
-TARGET_BUILTIN(__builtin_amdgcn_udot2, "UiV2UsV2UsUiIb", "nc", "dot-insts")
-TARGET_BUILTIN(__builtin_amdgcn_sdot4, "SiSiSiSiIb", "nc", "dot-insts")
-TARGET_BUILTIN(__builtin_amdgcn_udot4, "UiUiUiUiIb", "nc", "dot-insts")
-TARGET_BUILTIN(__builtin_amdgcn_sdot8, "SiSiSiSiIb", "nc", "dot-insts")
-TARGET_BUILTIN(__builtin_amdgcn_udot8, "UiUiUiUiIb", "nc", "dot-insts")
+TARGET_BUILTIN(__builtin_amdgcn_fdot2, "fV2hV2hfIb", "nc", "dot2-insts")
+TARGET_BUILTIN(__builtin_amdgcn_sdot2, "SiV2SsV2SsSiIb", "nc", "dot2-insts")
+TARGET_BUILTIN(__builtin_amdgcn_udot2, "UiV2UsV2UsUiIb", "nc", "dot2-insts")
+TARGET_BUILTIN(__builtin_amdgcn_sdot4, "SiSiSiSiIb", "nc", "dot1-insts")
+TARGET_BUILTIN(__builtin_amdgcn_udot4, "UiUiUiUiIb", "nc", "dot2-insts")
+TARGET_BUILTIN(__builtin_amdgcn_sdot8, "SiSiSiSiIb", "nc", "dot1-insts")
+TARGET_BUILTIN(__builtin_amdgcn_udot8, "UiUiUiUiIb", "nc", "dot2-insts")
 
 
//===--===//
 // Special builtins.

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=353588=353587=353588=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Fri Feb  8 16:34:41 2019
@@ -136,7 +136,8 @@ bool AMDGPUTargetInfo::initFeatureMap(
 switch (llvm::AMDGPU::parseArchAMDGCN(CPU)) {
 case GK_GFX906:
   Features["dl-insts"] = true;
-  Features["dot-insts"] = true;
+  Features["dot1-insts"] = true;
+  Features["dot2-insts"] = true;
   LLVM_FALLTHROUGH;
 case GK_GFX909:
 case GK_GFX904:

Modified: cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl?rev=353588=353587=353588=diff
==
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl Fri Feb  8 16:34:41 2019
@@ -11,7 +11,7 @@
 // RUN: %clang_cc1 -triple amdgcn -target-cpu gfx601 -S -emit-llvm -o - %s | 
FileCheck --check-prefix=GFX601 %s
 
 // GFX904: 
"target-features"="+16-bit-insts,+ci-insts,+dpp,+fp32-denormals,+fp64-fp16-denormals,+gfx9-insts,+s-memrealtime,+vi-insts"
-// GFX906: 
"target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot-insts,+dpp,+fp32-denormals,+fp64-fp16-denormals,+gfx9-insts,+s-memrealtime,+vi-insts"
+// GFX906: 
"target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot2-insts,+dpp,+fp32-denormals,+fp64-fp16-denormals,+gfx9-insts,+s-memrealtime,+vi-insts"
 // GFX801: 
"target-features"="+16-bit-insts,+ci-insts,+dpp,+fp32-denormals,+fp64-fp16-denormals,+s-memrealtime,+vi-insts"
 // GFX700: "target-features"="+ci-insts,+fp64-fp16-denormals,-fp32-denormals"
 // GFX600: "target-features"="+fp64-fp16-denormals,-fp32-denormals"

Modified: cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-dl-insts-err.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-dl-insts-err.cl?rev=353588=353587=353588=diff
==
--- cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-dl-insts-err.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-dl-insts-err.cl Fri Feb  8 
16:34:41 2019
@@ -12,24 +12,24 @@ kernel void builtins_amdgcn_dl_insts_err
 half2 v2hA, half2 v2hB, float fC,
 short2 v2ssA, short2 v2ssB, int siA, int siB, int siC,
 ushort2 v2usA, ushort2 v2usB, uint uiA, uint uiB, uint uiC) {
-  fOut[0] = __builtin_amdgcn_fdot2(v2hA, v2hB, fC, false); // 
expected-error {{'__builtin_amdgcn_fdot2' needs target feature dot-insts}}
-  fOut[1] = 

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang created this revision.
mgrang added reviewers: rsmith, bogner.
Herald added a project: clang.

Repository:
  rC Clang

https://reviews.llvm.org/D57986

Files:
  include/llvm/ProfileData/InstrProfWriter.h


Index: include/llvm/ProfileData/InstrProfWriter.h
===
--- include/llvm/ProfileData/InstrProfWriter.h
+++ include/llvm/ProfileData/InstrProfWriter.h
@@ -14,7 +14,7 @@
 #ifndef LLVM_PROFILEDATA_INSTRPROFWRITER_H
 #define LLVM_PROFILEDATA_INSTRPROFWRITER_H
 
-#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Endian.h"
@@ -32,7 +32,7 @@
 
 class InstrProfWriter {
 public:
-  using ProfilingData = SmallDenseMap;
+  using ProfilingData = MapVector;
   enum ProfKind { PF_Unknown = 0, PF_FE, PF_IRLevel };
 
 private:


Index: include/llvm/ProfileData/InstrProfWriter.h
===
--- include/llvm/ProfileData/InstrProfWriter.h
+++ include/llvm/ProfileData/InstrProfWriter.h
@@ -14,7 +14,7 @@
 #ifndef LLVM_PROFILEDATA_INSTRPROFWRITER_H
 #define LLVM_PROFILEDATA_INSTRPROFWRITER_H
 
-#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Endian.h"
@@ -32,7 +32,7 @@
 
 class InstrProfWriter {
 public:
-  using ProfilingData = SmallDenseMap;
+  using ProfilingData = MapVector;
   enum ProfKind { PF_Unknown = 0, PF_FE, PF_IRLevel };
 
 private:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

In D57874#1391133 , @tlively wrote:

> In D57874#1389953 , @aheejin wrote:
>
> > Anyway, moved all logic to the driver layer and did this:
> >
> > - `-matomics` means `-mthread-model posix`
> > - `-mthread-model posix` means `-matomics`
> > - `-pthread` means both `-matomics` and `-mthread-model posix`
>
>
> If you replace "-matomics" with "-mbulk-memory," I plan to duplicate the 
> logic for items 2 and 3 above, but not 1. For bulk memory even more than 
> atomics, there is a legitimate usecase for enabling it even in single 
> threaded contexts (namely getting to use memory.copy and memory.fill). I 
> wonder if consistency with how bulk memory works is a strong enough argument 
> for dropping item 1 for atomics as well.


Then now `-mthread-model posix` means both `-matomics` and `-mbulk-memory`, 
`-matomics` means `-mthread-model posix`, `-mbulk-memory` means `-mthread-model 
posix` but NOT `-matomics` means `mbulk-memory` and vice versa, right? Oh god 
it's complicated BTW if you are planning to use `-mthread-model` for 
turning on and off bulk memory too, why not 1?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:50
+  bool HasNoPthread =
+  !Pthread && DriverArgs.hasArg(clang::driver::options::OPT_no_pthread);
+

tlively wrote:
> Should this logic use `getLastArg` or perhaps `getLastArgNoClaim` to check 
> only that the final requested configuration is consistent rather than 
> checking all intermediate configurations?
Can you remove all the "clang::driver" namspace qualification here since there 
is  a "using" above?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57540: [C++17] Don't crash while diagnosing different access specifier of a deduction guide.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: test/SemaCXX/cxx1z-class-template-argument-deduction.cpp:360-361
+  struct Type {
+Typo(); // expected-error{{deduction guide must be declared in the same 
scope}}
+// expected-error@-1{{deduction guide declaration without trailing return 
type}}
+  };

aaron.ballman wrote:
> Rakete wrote:
> > aaron.ballman wrote:
> > > These errors are pretty unfortunate -- they don't really help the user to 
> > > understand what's gone wrong here. They're an improvement over the crash, 
> > > but I think we should try to make the errors more useful if we can.
> > > 
> > > Why is `Typo()` being treated as a deduction guide? Perhaps we could look 
> > > to see if there is a `->` before making that determination?
> > > Perhaps we could look to see if there is a -> before making that 
> > > determination?
> > 
> > Yes that would be possible. The diagnostic would change for the following 
> > code:
> > 
> > ```
> > template 
> > struct Foo {};
> > 
> > Foo();// -> Foo; 
> > // currently: deduction guide missing ->
> > // after: C++ requires type specifier for every declaration
> > ```
> > 
> > Is that acceptable? Or I guess I could restrict this to partial deduction 
> > guides in classes.
> I think the original diagnostic is better in that case. If you restrict to 
> partial deduction guides, do we get all the good diagnostics?
I think the most likely intent in that case was to declare a constructor of 
`Foo`, and either it was accidentally written after the end of the class, or 
the in-class declaration got copy-pasted and the programmer forgot to fix it 
up. And moreso for a case such as

```
template 
struct Foo { Foo(); };

template
Foo() {
// ...
}
```

... where we currently give three errors about deduction guides and no hint 
that a qualified name is required to define a constructor.

I think it's comparatively unlikely that someone would forget the `->` in a 
deduction guide, given how central it is to the purpose of the declaration. So 
always disambiguating as a failed constructor rather than a failed deduction 
guide seems reasonable to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57540



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:66
+if (Args.hasFlag(clang::driver::options::OPT_pthread,
+ clang::driver::options::OPT_no_pthread),
+false)

aheejin wrote:
> tlively wrote:
> > sbc100 wrote:
> > > aheejin wrote:
> > > > This code is not strictly related, but `hasFlag` is better than 
> > > > `hasArg` when there are both positive and negative versions of an 
> > > > option exist.
> > > Hmm.. there are currently no other references to OPT_no_pthread in the 
> > > whole codebase.   Maybe better to simply remove the option?
> > > 
> > > I wouldn't want to commit this as that first use of the option as it 
> > > might make it hard to remove :)
> > I think commands generally come in pairs to make it possible to override 
> > previous settings by appending args to command lines. Counting uses of 
> > OPT_no_pthread without including uses of OP_pthread doesn't make much sense.
> Most true/false or enable/disable options exist in pairs. `-no-pthread` is 
> defined [[ 
> https://github.com/llvm/llvm-project/blob/91970564191bfc40ea9f2c8d32cc1fb6c314515c/clang/include/clang/Driver/Options.td#L2508
>  | here ]]. So this `ArgList::hasFlag` function checks the existence of both 
> the positive option and the negative option at the same time, and if neither 
> exists, takes the default value, which is the third argument. So yes, as 
> @tlively said, it's just a safety measure. Before it only checked the 
> existence of `-pthread` and didn't care if `-no-pthread` existed or not.
There no current usage of OPT_no_pthread anywhere in clang.  For this reason I 
think this option is ripe for removal completely.

Therefore I don't think its a good idea to introduce a usage here, as it could 
make the removal more difficult.  Especially as this part of the patch is kind 
of unrelated.. its kind of an authogonal cleanup .. which I think makes things 
worse.  So I vote to revert this line :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Thomas Lively via Phabricator via cfe-commits]

tlively added inline comments.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:50
+  bool HasNoPthread =
+  !Pthread && DriverArgs.hasArg(clang::driver::options::OPT_no_pthread);
+

Should this logic use `getLastArg` or perhaps `getLastArgNoClaim` to check only 
that the final requested configuration is consistent rather than checking all 
intermediate configurations?



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:62
+Driver.Diag(diag::err_drv_argument_not_allowed_with) << "-matomics"
+ << "-no-pthread";
+  // '-mno-atomics' cannot be used with '-mthread-model posix'

I'm not sure about this one, either. What if I want atomics for multithreading 
but I don't want to link with libpthread?



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:79
+Driver.Diag(diag::err_drv_argument_not_allowed_with)
+<< "-no-pthread" << ThreadModelOpt;
+}

Same question here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

I only added those test routines in `wasm-toolchain.c` and not 
`wasm-toolchain.cpp`, because they are basically the same.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin updated this revision to Diff 186068.
aheejin added a comment.

- Initialized ThreadModel vairables with ""


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  test/Driver/wasm-toolchain.c

Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -38,3 +38,35 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+
+// Thread-related command line tests.
+// - '-matomics' sets '-mthread-model posix'
+// - '-mthread-model' sets '-matomics'
+// - '-phread' sets both '-matomics' and '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics 2>&1 | FileCheck -check-prefix=ATOMICS %s
+// ATOMICS: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mthread-model posix 2>&1 | FileCheck -check-prefix=POSIX %s
+// POSIX: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread 2>&1 | FileCheck -check-prefix=PTHREAD %s
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR0 %s
+// THREAD_OPT_ERROR0: invalid argument '-matomics' not allowed with '-mthread-model single'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics -no-pthread 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR1 %s
+// THREAD_OPT_ERROR1: invalid argument '-matomics' not allowed with '-no-pthread'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mno-atomics -mthread-model posix 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR2 %s
+// THREAD_OPT_ERROR2: invalid argument '-mno-atomics' not allowed with '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mno-atomics -pthread 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR3 %s
+// THREAD_OPT_ERROR3: invalid argument '-mno-atomics' not allowed with '-pthread'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR4 %s
+// THREAD_OPT_ERROR4: invalid argument '-pthread' not allowed with '-mthread-model single'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -no-pthread -mthread-model posix 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR5 %s
+// THREAD_OPT_ERROR5: invalid argument '-no-pthread' not allowed with '-mthread-model posix'
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -64,7 +64,7 @@
   llvm::opt::ArgStringList ) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
-  std::string getThreadModel() const override;
+  std::string getThreadModel(const llvm::opt::ArgList &) const override;
 
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -20,6 +20,65 @@
 using namespace clang;
 using namespace llvm::opt;
 
+void parseThreadArgs(const Driver , const ArgList ,
+ bool , bool , StringRef ,
+ bool CheckForErrors = true) {
+  // Default value for -matomics / -pthread / -mthread-model options, each being
+  // false / false / "single".
+  Atomics = DriverArgs.hasFlag(clang::driver::options::OPT_matomics,
+   clang::driver::options::OPT_mno_atomics, false);
+  Pthread = DriverArgs.hasFlag(clang::driver::options::OPT_pthread,
+   clang::driver::options::OPT_no_pthread, false);
+  ThreadModel = DriverArgs.getLastArgValue(
+  clang::driver::options::OPT_mthread_model, "single");
+  if (!CheckForErrors)
+return;
+
+  // Error checking
+
+  // Did user explicitly specify 

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

In D57874#1389981 , @sunfish wrote:

> > - `-matomics` means `-mthread-model posix`
>
> The others sound reasonable, though this one seems a little surprising -- a 
> user might have -matomics enabled because they're targeting a VM that has 
> atomics, but still not want to use -mthread-model posix because their code 
> doesn't actually using threads.


As @sbc said, `-mthread-model` is only used in ARM and wasm backend to 
determine whether to lower away atomic instructions or not. So that the user 
gave `-matomics` means the VM can support atomic instructions, so even if we 
are not actually using threads, it's fine because the VM can handle them.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added reviewers: aaron.ballman, rjmccall.
Herald added a project: clang.

We used to get this wrong in three ways:

1. During parsing, an expression-statement followed by the }) ending a

statement expression was always treated as producing the value of the
statement expression. That's wrong for ({ if (1) expr; })

2. During template instantiation, various kinds of statement (most

statements not appearing directly in a compound-statement) were not
treated as discarded-value expressions, resulting in missing volatile
loads (etc).

3. In all contexts, an expression-statement with attributes was not

treated as producing the value of the statement expression, eg
({ [[attr]] expr; }).


Repository:
  rC Clang

https://reviews.llvm.org/D57984

Files:
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h
  include/clang/Basic/StmtNodes.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/Stmt.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Parse/ParseObjc.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  test/CodeGenCXX/stmtexpr.cpp
  test/CodeGenCXX/volatile.cpp

Index: test/CodeGenCXX/volatile.cpp
===
--- test/CodeGenCXX/volatile.cpp
+++ test/CodeGenCXX/volatile.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++98 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++98 -o - | FileCheck -check-prefix=CHECK -check-prefix=CHECK98 %s
 // RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -o - | FileCheck -check-prefix=CHECK -check-prefix=CHECK11 %s
 
 // Check that IR gen doesn't try to do an lvalue-to-rvalue conversion
@@ -33,3 +33,19 @@
 *x;
   }
 }
+
+namespace PR40642 {
+  template  struct S {
+// CHECK-LABEL: define {{.*}} @_ZN7PR406421SIiE3fooEv(
+void foo() {
+  // CHECK98-NOT: load volatile
+  // CHECK11: load volatile
+  if (true)
+reinterpret_cast(m_ptr)[0];
+  // CHECK: }
+}
+int *m_ptr;
+  };
+
+  void f(S *x) { x->foo(); }
+}
Index: test/CodeGenCXX/stmtexpr.cpp
===
--- test/CodeGenCXX/stmtexpr.cpp
+++ test/CodeGenCXX/stmtexpr.cpp
@@ -190,3 +190,79 @@
 // CHECK: %[[v2:[^ ]*]] = load float, float* %[[tmp2]]
 // CHECK: store float %[[v1]], float* %v.realp
 // CHECK: store float %[[v2]], float* %v.imagp
+
+extern "C" void then(int);
+
+// CHECK-LABEL: @{{.*}}volatile_load
+void volatile_load() {
+  volatile int n;
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({n;});
+
+  // CHECK-LABEL: @then(i32 1)
+  then(1);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({goto lab; lab: n;});
+
+  // CHECK-LABEL: @then(i32 2)
+  then(2);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({[[gsl::suppress("foo")]] n;});
+
+  // CHECK-LABEL: @then(i32 3)
+  then(3);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({if (true) n;});
+
+  // CHECK: }
+}
+
+// CHECK-LABEL: @{{.*}}volatile_load_template
+template
+void volatile_load_template() {
+  volatile T n;
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({n;});
+
+  // CHECK-LABEL: @then(i32 1)
+  then(1);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({goto lab; lab: n;});
+
+  // CHECK-LABEL: @then(i32 2)
+  then(2);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({[[gsl::suppress("foo")]] n;});
+
+  // CHECK-LABEL: @then(i32 3)
+  then(3);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({if (true) n;});
+
+  // CHECK: }
+}
+template void volatile_load_template();
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -318,6 +318,13 @@
   TypeSourceInfo *TransformTypeWithDeducedTST(TypeSourceInfo *DI);
   /// @}
 
+  /// The reason why the value of a statement is not discarded, if any.
+  enum StmtDiscardKind {
+SDK_Discarded,
+SDK_NotDiscarded,
+SDK_StmtExprResult,
+  };
+
   /// Transform the given statement.
   ///
   /// By default, this routine transforms a statement by delegating to the
@@ -327,7 +334,7 @@
   /// other mechanism.
   ///
   /// \returns the transformed statement.
-  StmtResult TransformStmt(Stmt *S, bool DiscardedValue = false);
+  StmtResult TransformStmt(Stmt *S, StmtDiscardKind SDK = SDK_Discarded);
 
   /// Transform the given statement.
   ///
@@ -672,6 +679,9 @@
 #define STMT(Node, Parent)\
   LLVM_ATTRIBUTE_NOINLINE \
   StmtResult Transform##Node(Node *S);

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin updated this revision to Diff 186065.
aheejin added a comment.

- Small fix


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  test/Driver/wasm-toolchain.c

Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -38,3 +38,35 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+
+// Thread-related command line tests.
+// - '-matomics' sets '-mthread-model posix'
+// - '-mthread-model' sets '-matomics'
+// - '-phread' sets both '-matomics' and '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics 2>&1 | FileCheck -check-prefix=ATOMICS %s
+// ATOMICS: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mthread-model posix 2>&1 | FileCheck -check-prefix=POSIX %s
+// POSIX: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread 2>&1 | FileCheck -check-prefix=PTHREAD %s
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR0 %s
+// THREAD_OPT_ERROR0: invalid argument '-matomics' not allowed with '-mthread-model single'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics -no-pthread 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR1 %s
+// THREAD_OPT_ERROR1: invalid argument '-matomics' not allowed with '-no-pthread'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mno-atomics -mthread-model posix 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR2 %s
+// THREAD_OPT_ERROR2: invalid argument '-mno-atomics' not allowed with '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mno-atomics -pthread 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR3 %s
+// THREAD_OPT_ERROR3: invalid argument '-mno-atomics' not allowed with '-pthread'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR4 %s
+// THREAD_OPT_ERROR4: invalid argument '-pthread' not allowed with '-mthread-model single'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -no-pthread -mthread-model posix 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR5 %s
+// THREAD_OPT_ERROR5: invalid argument '-no-pthread' not allowed with '-mthread-model posix'
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -64,7 +64,7 @@
   llvm::opt::ArgStringList ) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
-  std::string getThreadModel() const override;
+  std::string getThreadModel(const llvm::opt::ArgList &) const override;
 
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -20,6 +20,65 @@
 using namespace clang;
 using namespace llvm::opt;
 
+void parseThreadArgs(const Driver , const ArgList ,
+ bool , bool , StringRef ,
+ bool CheckForErrors = true) {
+  // Default value for -matomics / -pthread / -mthread-model options, each being
+  // false / false / "single".
+  Atomics = DriverArgs.hasFlag(clang::driver::options::OPT_matomics,
+   clang::driver::options::OPT_mno_atomics, false);
+  Pthread = DriverArgs.hasFlag(clang::driver::options::OPT_pthread,
+   clang::driver::options::OPT_no_pthread, false);
+  ThreadModel = DriverArgs.getLastArgValue(
+  clang::driver::options::OPT_mthread_model, "single");
+  if (!CheckForErrors)
+return;
+
+  // Error checking
+
+  // Did user explicitly specify -matomics / -mno-atomics / 

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

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



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:66
+if (Args.hasFlag(clang::driver::options::OPT_pthread,
+ clang::driver::options::OPT_no_pthread),
+false)

tlively wrote:
> sbc100 wrote:
> > aheejin wrote:
> > > This code is not strictly related, but `hasFlag` is better than `hasArg` 
> > > when there are both positive and negative versions of an option exist.
> > Hmm.. there are currently no other references to OPT_no_pthread in the 
> > whole codebase.   Maybe better to simply remove the option?
> > 
> > I wouldn't want to commit this as that first use of the option as it might 
> > make it hard to remove :)
> I think commands generally come in pairs to make it possible to override 
> previous settings by appending args to command lines. Counting uses of 
> OPT_no_pthread without including uses of OP_pthread doesn't make much sense.
Most true/false or enable/disable options exist in pairs. `-no-pthread` is 
defined [[ 
https://github.com/llvm/llvm-project/blob/91970564191bfc40ea9f2c8d32cc1fb6c314515c/clang/include/clang/Driver/Options.td#L2508
 | here ]]. So this `ArgList::hasFlag` function checks the existence of both 
the positive option and the negative option at the same time, and if neither 
exists, takes the default value, which is the third argument. So yes, as 
@tlively said, it's just a safety measure. Before it only checked the existence 
of `-pthread` and didn't care if `-no-pthread` existed or not.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57981: [analyzer] strlcat() syntax check: Fix an off-by-one error.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353583: [analyzer] CStringSyntaxChecks: Fix an off-by-one 
error in the strlcat() check. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57981?vs=186049=186064#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57981

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
  cfe/trunk/test/Analysis/cstring-syntax.c


Index: cfe/trunk/test/Analysis/cstring-syntax.c
===
--- cfe/trunk/test/Analysis/cstring-syntax.c
+++ cfe/trunk/test/Analysis/cstring-syntax.c
@@ -33,6 +33,7 @@
   strlcpy(dest, src, ulen);
   strlcpy(dest + 5, src, 5);
   strlcpy(dest + 5, src, 10); // expected-warning {{The third argument allows 
to potentially copy more bytes than it should. Replace with the value 
sizeof() or lower}}
+  strlcpy(dest, "aaa", 10); // no-warning
 }
 
 void testStrlcat(const char *src) {
@@ -51,4 +52,5 @@
   strlcat(dest, src, ulen);
   strlcpy(dest, src, 5);
   strlcat(dest + 5, src, badlen); // expected-warning {{The third argument 
allows to potentially copy more bytes than it should. Replace with the value 
sizeof() or lower}}
+  strlcat(dest, "aaa", 10); // no-warning
 }
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -153,8 +153,6 @@
 bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
   if (CE->getNumArgs() != 3)
 return false;
-  const FunctionDecl *FD = CE->getDirectCallee();
-  bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
   const Expr *DstArg = CE->getArg(0);
   const Expr *LenArg = CE->getArg(2);
 
@@ -194,13 +192,8 @@
 ASTContext  = BR.getContext();
 uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
 auto RemainingBufferLen = BufferLen - DstOff;
-if (Append) {
-  if (RemainingBufferLen <= ILRawVal)
-return true;
-} else {
-  if (RemainingBufferLen < ILRawVal)
-return true;
-}
+if (RemainingBufferLen < ILRawVal)
+  return true;
   }
 }
   }


Index: cfe/trunk/test/Analysis/cstring-syntax.c
===
--- cfe/trunk/test/Analysis/cstring-syntax.c
+++ cfe/trunk/test/Analysis/cstring-syntax.c
@@ -33,6 +33,7 @@
   strlcpy(dest, src, ulen);
   strlcpy(dest + 5, src, 5);
   strlcpy(dest + 5, src, 10); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof() or lower}}
+  strlcpy(dest, "aaa", 10); // no-warning
 }
 
 void testStrlcat(const char *src) {
@@ -51,4 +52,5 @@
   strlcat(dest, src, ulen);
   strlcpy(dest, src, 5);
   strlcat(dest + 5, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof() or lower}}
+  strlcat(dest, "aaa", 10); // no-warning
 }
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -153,8 +153,6 @@
 bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
   if (CE->getNumArgs() != 3)
 return false;
-  const FunctionDecl *FD = CE->getDirectCallee();
-  bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
   const Expr *DstArg = CE->getArg(0);
   const Expr *LenArg = CE->getArg(2);
 
@@ -194,13 +192,8 @@
 ASTContext  = BR.getContext();
 uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
 auto RemainingBufferLen = BufferLen - DstOff;
-if (Append) {
-  if (RemainingBufferLen <= ILRawVal)
-return true;
-} else {
-  if (RemainingBufferLen < ILRawVal)
-return true;
-}
+if (RemainingBufferLen < ILRawVal)
+  return true;
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin updated this revision to Diff 186063.
aheejin added a comment.

- Fix some bugs
- Make the driver error out when explicitly given options don't match, such as 
`-no-pthread` and `-matomics` are given at the same time


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  test/Driver/wasm-toolchain.c

Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -38,3 +38,35 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+
+// Thread-related command line tests.
+// - '-matomics' sets '-mthread-model posix'
+// - '-mthread-model' sets '-matomics'
+// - '-phread' sets both '-matomics' and '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics 2>&1 | FileCheck -check-prefix=ATOMICS %s
+// ATOMICS: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mthread-model posix 2>&1 | FileCheck -check-prefix=POSIX %s
+// POSIX: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread 2>&1 | FileCheck -check-prefix=PTHREAD %s
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR0 %s
+// THREAD_OPT_ERROR0: invalid argument '-matomics' not allowed with '-mthread-model single'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -matomics -no-pthread 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR1 %s
+// THREAD_OPT_ERROR1: invalid argument '-matomics' not allowed with '-no-pthread'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mno-atomics -mthread-model posix 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR2 %s
+// THREAD_OPT_ERROR2: invalid argument '-mno-atomics' not allowed with '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mno-atomics -pthread 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR3 %s
+// THREAD_OPT_ERROR3: invalid argument '-mno-atomics' not allowed with '-pthread'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR4 %s
+// THREAD_OPT_ERROR4: invalid argument '-pthread' not allowed with '-mthread-model single'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -no-pthread -mthread-model posix 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR5 %s
+// THREAD_OPT_ERROR5: invalid argument '-no-pthread' not allowed with '-mthread-model posix'
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -64,7 +64,7 @@
   llvm::opt::ArgStringList ) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
-  std::string getThreadModel() const override;
+  std::string getThreadModel(const llvm::opt::ArgList &) const override;
 
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -20,6 +20,65 @@
 using namespace clang;
 using namespace llvm::opt;
 
+void parseThreadArgs(const Driver , const ArgList ,
+ bool , bool , StringRef ,
+ bool CheckForErrors = true) {
+  // Default value for -matomics / -pthread / -mthread-model options, each being
+  // false / false / "single".
+  Atomics = DriverArgs.hasFlag(clang::driver::options::OPT_matomics,
+   clang::driver::options::OPT_mno_atomics, false);
+  Pthread = DriverArgs.hasFlag(clang::driver::options::OPT_pthread,
+   clang::driver::options::OPT_no_pthread, false);
+  ThreadModel = DriverArgs.getLastArgValue(
+  clang::driver::options::OPT_mthread_model, 

r353583 - [analyzer] CStringSyntaxChecks: Fix an off-by-one error in the strlcat() check.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri Feb  8 15:59:52 2019
New Revision: 353583

URL: http://llvm.org/viewvc/llvm-project?rev=353583=rev
Log:
[analyzer] CStringSyntaxChecks: Fix an off-by-one error in the strlcat() check.

oth strlcat and strlcpy cut off their safe bound for the argument value
at sizeof(destination). There's no need to subtract 1 in only one
of these cases.

Differential Revision: https://reviews.llvm.org/D57981

rdar://problem/47873212

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
cfe/trunk/test/Analysis/cstring-syntax.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp?rev=353583=353582=353583=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp Fri Feb  8 
15:59:52 2019
@@ -153,8 +153,6 @@ bool WalkAST::containsBadStrncatPattern(
 bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
   if (CE->getNumArgs() != 3)
 return false;
-  const FunctionDecl *FD = CE->getDirectCallee();
-  bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
   const Expr *DstArg = CE->getArg(0);
   const Expr *LenArg = CE->getArg(2);
 
@@ -194,13 +192,8 @@ bool WalkAST::containsBadStrlcpyStrlcatP
 ASTContext  = BR.getContext();
 uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
 auto RemainingBufferLen = BufferLen - DstOff;
-if (Append) {
-  if (RemainingBufferLen <= ILRawVal)
-return true;
-} else {
-  if (RemainingBufferLen < ILRawVal)
-return true;
-}
+if (RemainingBufferLen < ILRawVal)
+  return true;
   }
 }
   }

Modified: cfe/trunk/test/Analysis/cstring-syntax.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cstring-syntax.c?rev=353583=353582=353583=diff
==
--- cfe/trunk/test/Analysis/cstring-syntax.c (original)
+++ cfe/trunk/test/Analysis/cstring-syntax.c Fri Feb  8 15:59:52 2019
@@ -33,6 +33,7 @@ void testStrlcpy(const char *src) {
   strlcpy(dest, src, ulen);
   strlcpy(dest + 5, src, 5);
   strlcpy(dest + 5, src, 10); // expected-warning {{The third argument allows 
to potentially copy more bytes than it should. Replace with the value 
sizeof() or lower}}
+  strlcpy(dest, "aaa", 10); // no-warning
 }
 
 void testStrlcat(const char *src) {
@@ -51,4 +52,5 @@ void testStrlcat(const char *src) {
   strlcat(dest, src, ulen);
   strlcpy(dest, src, 5);
   strlcat(dest + 5, src, badlen); // expected-warning {{The third argument 
allows to potentially copy more bytes than it should. Replace with the value 
sizeof() or lower}}
+  strlcat(dest, "aaa", 10); // no-warning
 }


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

RE: r353569 - [Sema] Make string literal init an rvalue.

[via cfe-commits]

Hi Eli,

Your commit is causing a failure on the PS4 linux bot, can you please take a 
look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/43625

FAIL: Clang Tools :: clang-tidy/bugprone-string-constructor.cpp (14325 of 46835)
 TEST 'Clang Tools :: 
clang-tidy/bugprone-string-constructor.cpp' FAILED 
Script:
--
: 'RUN: at line 1';   /usr/bin/python2.7 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/../test/clang-tidy/check_clang_tidy.py
 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/bugprone-string-constructor.cpp
 bugprone-string-constructor 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/bugprone-string-constructor.cpp.tmp
--
Exit Code: 1

Command Output (stdout):
--
Running ['clang-tidy', 
'/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/bugprone-string-constructor.cpp.tmp.cpp',
 '-fix', '--checks=-*,bugprone-string-constructor', '--', '--std=c++11', 
'-nostdinc++']...
clang-tidy failed:
clang-tidy: 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/lib/AST/ExprConstant.cpp:3374:
 bool handleLValueToRValueConversion((anonymous namespace)::EvalInfo &, const 
clang::Expr *, clang::QualType, const (anonymous namespace)::LValue &, 
clang::APValue &): Assertion `LVal.Designator.Entries.size() == 1 && "Can only 
read characters from string literals"' failed.
 #0 0x004ad2c4 (clang-tidy+0x4ad2c4)
 #1 0x004ab03c (clang-tidy+0x4ab03c)
 #2 0x004ad878 (clang-tidy+0x4ad878)
 #3 0x7f034560b890 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
 #4 0x7f03442d1e97 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x3ee97)
 #5 0x7f03442d3801 abort (/lib/x86_64-linux-gnu/libc.so.6+0x40801)
 #6 0x7f03442c339a (/lib/x86_64-linux-gnu/libc.so.6+0x3039a)
 #7 0x7f03442c3412 (/lib/x86_64-linux-gnu/libc.so.6+0x30412)
 #8 0x01941c9d (clang-tidy+0x1941c9d)
 #9 0x0192b797 (clang-tidy+0x192b797)
#10 0x019266be (clang-tidy+0x19266be)
#11 0x005771f9 (clang-tidy+0x5771f9)
#12 0x01769d11 (clang-tidy+0x1769d11)
#13 0x01782d4b (clang-tidy+0x1782d4b)
#14 0x01769497 (clang-tidy+0x1769497)
#15 0x01774613 (clang-tidy+0x1774613)
#16 0x0177161f (clang-tidy+0x177161f)
#17 0x01782ad2 (clang-tidy+0x1782ad2)
#18 0x0176b3e6 (clang-tidy+0x176b3e6)
#19 0x0177f17d (clang-tidy+0x177f17d)
#20 0x0177161f (clang-tidy+0x177161f)
#21 0x017829f3 (clang-tidy+0x17829f3)
#22 0x0176b0bd (clang-tidy+0x176b0bd)
#23 0x0176e8b7 (clang-tidy+0x176e8b7)
#24 0x0176cfae (clang-tidy+0x176cfae)
#25 0x0174981f (clang-tidy+0x174981f)
#26 0x00c5020c (clang-tidy+0xc5020c)
#27 0x00d98873 (clang-tidy+0xd98873)
#28 0x00c381c0 (clang-tidy+0xc381c0)
#29 0x00bdcd31 (clang-tidy+0xbdcd31)
#30 0x00834386 (clang-tidy+0x834386)
#31 0x004ccba5 (clang-tidy+0x4ccba5)
#32 0x008340f6 (clang-tidy+0x8340f6)
#33 0x00833997 (clang-tidy+0x833997)
#34 0x0083579c (clang-tidy+0x83579c)
#35 0x004c9505 (clang-tidy+0x4c9505)
#36 0x0041d427 (clang-tidy+0x41d427)
#37 0x7f03442b4b97 __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x21b97)
#38 0x0041b2fa (clang-tidy+0x41b2fa)


--
Command Output (stderr):
--
Traceback (most recent call last):
  File 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/../test/clang-tidy/check_clang_tidy.py",
 line 207, in 
main()
  File 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/../test/clang-tidy/check_clang_tidy.py",
 line 147, in main
subprocess.check_output(args, stderr=subprocess.STDOUT).decode()
  File "/usr/lib/python2.7/subprocess.py", line 223, in check_output
raise CalledProcessError(retcode, cmd, output=output)
subprocess.CalledProcessError: Command '['clang-tidy', 
'/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/bugprone-string-constructor.cpp.tmp.cpp',
 '-fix', '--checks=-*,bugprone-string-constructor', '--', '--std=c++11', 
'-nostdinc++']' returned non-zero exit status -6

Douglas Yung

-Original Message-
From: cfe-commits  On Behalf Of Eli 
Friedman via cfe-commits
Sent: Friday, February 8, 2019 13:19
To: cfe-commits@lists.llvm.org
Subject: r353569 - [Sema] Make string literal init an rvalue.

Author: efriedma
Date: Fri Feb  8 13:18:46 2019
New Revision: 353569

URL: 

[PATCH] D57977: [HIP] compile option code-object-v3 propagate to llc

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added a comment.

https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/AMDGPU.cpp#L52


Repository:
  rC Clang

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

https://reviews.llvm.org/D57977



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

[PATCH] D57977: [HIP] compile option code-object-v3 propagate to llc

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added a comment.

Can you handle all features as follows instead of checking a particular option?

‘handleTargetFeaturesGroup(

  Args, Features, options::OPT_m_amdgpu_Features_Group);’ ?

There is code in AMDGPU tool chain you can use as a reference.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57977



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

[PATCH] D57981: [analyzer] strlcat() syntax check: Fix an off-by-one error.

[Devin Coughlin via Phabricator via cfe-commits]

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

Looks good to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57981



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

[PATCH] D57976: -gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353578: -gmodules: Dont emit incomplete breadcrumbs 
pointing to nonexistant PCM files. (authored by adrian, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57976?vs=186041=186056#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57976

Files:
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/test/Modules/DebugInfo-fmodule-name.c


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -2296,7 +2296,14 @@
   }
 
   bool IsRootModule = M ? !M->Parent : true;
-  if (CreateSkeletonCU && IsRootModule) {
+  // When a module name is specified as -fmodule-name, that module gets a
+  // clang::Module object, but it won't actually be built or imported; it will
+  // be textual.
+  if (CreateSkeletonCU && IsRootModule && Mod.getASTFile().empty())
+assert((!M || (M->Name == CGM.getLangOpts().ModuleName)) &&
+   "clang module without ASTFile must be specified by -fmodule-name");
+
+  if (CreateSkeletonCU && IsRootModule && !Mod.getASTFile().empty()) {
 // PCH files don't have a signature field in the control block,
 // but LLVM detects skeleton CUs by looking for a non-zero DWO id.
 // We use the lower 64 bits for debug info.
@@ -2313,6 +2320,7 @@
   Signature);
 DIB.finalize();
   }
+
   llvm::DIModule *Parent =
   IsRootModule ? nullptr
: getOrCreateModuleRef(
Index: cfe/trunk/test/Modules/DebugInfo-fmodule-name.c
===
--- cfe/trunk/test/Modules/DebugInfo-fmodule-name.c
+++ cfe/trunk/test/Modules/DebugInfo-fmodule-name.c
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodule-format=obj -fmodule-name=F \
+// RUN: -debug-info-kind=limited -dwarf-ext-refs \
+// RUN: -fimplicit-module-maps -x c -fmodules-cache-path=%t -F %S/Inputs \
+// RUN: %s -S -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s
+
+#include "F/F.h"
+
+// CHECK: !DICompileUnit
+// CHECK-NOT: dwoId:
+
+// We still want the import, but no skeleton CU, since no PCM was built.
+
+// CHECK: !DIModule({{.*}}, name: "F"
+// CHECK-NOT: !DICompileUnit
+// CHECK-NOT: dwoId:


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -2296,7 +2296,14 @@
   }
 
   bool IsRootModule = M ? !M->Parent : true;
-  if (CreateSkeletonCU && IsRootModule) {
+  // When a module name is specified as -fmodule-name, that module gets a
+  // clang::Module object, but it won't actually be built or imported; it will
+  // be textual.
+  if (CreateSkeletonCU && IsRootModule && Mod.getASTFile().empty())
+assert((!M || (M->Name == CGM.getLangOpts().ModuleName)) &&
+   "clang module without ASTFile must be specified by -fmodule-name");
+
+  if (CreateSkeletonCU && IsRootModule && !Mod.getASTFile().empty()) {
 // PCH files don't have a signature field in the control block,
 // but LLVM detects skeleton CUs by looking for a non-zero DWO id.
 // We use the lower 64 bits for debug info.
@@ -2313,6 +2320,7 @@
   Signature);
 DIB.finalize();
   }
+
   llvm::DIModule *Parent =
   IsRootModule ? nullptr
: getOrCreateModuleRef(
Index: cfe/trunk/test/Modules/DebugInfo-fmodule-name.c
===
--- cfe/trunk/test/Modules/DebugInfo-fmodule-name.c
+++ cfe/trunk/test/Modules/DebugInfo-fmodule-name.c
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodule-format=obj -fmodule-name=F \
+// RUN: -debug-info-kind=limited -dwarf-ext-refs \
+// RUN: -fimplicit-module-maps -x c -fmodules-cache-path=%t -F %S/Inputs \
+// RUN: %s -S -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s
+
+#include "F/F.h"
+
+// CHECK: !DICompileUnit
+// CHECK-NOT: dwoId:
+
+// We still want the import, but no skeleton CU, since no PCM was built.
+
+// CHECK: !DIModule({{.*}}, name: "F"
+// CHECK-NOT: !DICompileUnit
+// CHECK-NOT: dwoId:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r353578 - -gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Fri Feb  8 15:15:42 2019
New Revision: 353578

URL: http://llvm.org/viewvc/llvm-project?rev=353578=rev
Log:
-gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

When a module name is specified as -fmodule-name, that module gets a
clang::Module object, but it won't actually be built or imported; it
will be textual. CGDebugInfo wouldn't detect this and them emit a
DICompileUnit that had a hash but no name and that confused both
dsymutil, LLDB, and myself.

rdar://problem/47926508

Differential Revision: https://reviews.llvm.org/D57976

Added:
cfe/trunk/test/Modules/DebugInfo-fmodule-name.c
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=353578=353577=353578=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Feb  8 15:15:42 2019
@@ -2296,7 +2296,14 @@ CGDebugInfo::getOrCreateModuleRef(Extern
   }
 
   bool IsRootModule = M ? !M->Parent : true;
-  if (CreateSkeletonCU && IsRootModule) {
+  // When a module name is specified as -fmodule-name, that module gets a
+  // clang::Module object, but it won't actually be built or imported; it will
+  // be textual.
+  if (CreateSkeletonCU && IsRootModule && Mod.getASTFile().empty())
+assert((!M || (M->Name == CGM.getLangOpts().ModuleName)) &&
+   "clang module without ASTFile must be specified by -fmodule-name");
+
+  if (CreateSkeletonCU && IsRootModule && !Mod.getASTFile().empty()) {
 // PCH files don't have a signature field in the control block,
 // but LLVM detects skeleton CUs by looking for a non-zero DWO id.
 // We use the lower 64 bits for debug info.
@@ -2313,6 +2320,7 @@ CGDebugInfo::getOrCreateModuleRef(Extern
   Signature);
 DIB.finalize();
   }
+
   llvm::DIModule *Parent =
   IsRootModule ? nullptr
: getOrCreateModuleRef(

Added: cfe/trunk/test/Modules/DebugInfo-fmodule-name.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/DebugInfo-fmodule-name.c?rev=353578=auto
==
--- cfe/trunk/test/Modules/DebugInfo-fmodule-name.c (added)
+++ cfe/trunk/test/Modules/DebugInfo-fmodule-name.c Fri Feb  8 15:15:42 2019
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodule-format=obj -fmodule-name=F \
+// RUN: -debug-info-kind=limited -dwarf-ext-refs \
+// RUN: -fimplicit-module-maps -x c -fmodules-cache-path=%t -F %S/Inputs \
+// RUN: %s -S -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s
+
+#include "F/F.h"
+
+// CHECK: !DICompileUnit
+// CHECK-NOT: dwoId:
+
+// We still want the import, but no skeleton CU, since no PCM was built.
+
+// CHECK: !DIModule({{.*}}, name: "F"
+// CHECK-NOT: !DICompileUnit
+// CHECK-NOT: dwoId:


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

[PATCH] D57976: -gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

[Jonas Devlieghere via Phabricator via cfe-commits]

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

This makes sense to me, just one nit.




Comment at: lib/CodeGen/CGDebugInfo.cpp:2304
+assert((!M || (M->Name == CGM.getLangOpts().ModuleName)) &&
+   "clang module without ASTFile was not specified by -fmodule-name");
+

Maybe it's just me but I had to reread this sentence a few times to be sure 
what you meant by the double negation. Maybe `clang module without ASTFile must 
be specified by -fmodule-name` is easier to grok.


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

https://reviews.llvm.org/D57976



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

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353577: [CodeGen][ObjC] Fix assert on calling 
`__builtin_constant_p` with ObjC objects. (authored by vsapsai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57427?vs=184334=186054#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57427

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/test/CodeGenObjC/builtin-constant-p.m


Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -1982,6 +1982,12 @@
   return RValue::get(ConstantInt::get(ResultType, 0));
 
 Value *ArgValue = EmitScalarExpr(Arg);
+if (ArgType->isObjCObjectPointerType()) {
+  // Convert Objective-C objects to id because we cannot distinguish 
between
+  // LLVM types for Obj-C classes as they are opaque.
+  ArgType = CGM.getContext().getObjCIdType();
+  ArgValue = Builder.CreateBitCast(ArgValue, ConvertType(ArgType));
+}
 Function *F =
 CGM.getIntrinsic(Intrinsic::is_constant, ConvertType(ArgType));
 Value *Result = Builder.CreateCall(F, ArgValue);
Index: cfe/trunk/test/CodeGenObjC/builtin-constant-p.m
===
--- cfe/trunk/test/CodeGenObjC/builtin-constant-p.m
+++ cfe/trunk/test/CodeGenObjC/builtin-constant-p.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -O3 
-disable-llvm-passes -o - %s | FileCheck %s
+
+// Test that can call `__builtin_constant_p` with instances of different
+// Objective-C classes.
+// rdar://problem/47499250
+@class Foo;
+@class Bar;
+
+extern void callee(void);
+
+// CHECK-LABEL: define void @test(%0* %foo, %1* %bar)
+void test(Foo *foo, Bar *bar) {
+  // CHECK: [[ADDR_FOO:%.*]] = bitcast %0* %{{.*}} to i8*
+  // CHECK-NEXT: call i1 @llvm.is.constant.p0i8(i8* [[ADDR_FOO]])
+  // CHECK: [[ADDR_BAR:%.*]] = bitcast %1* %{{.*}} to i8*
+  // CHECK-NEXT: call i1 @llvm.is.constant.p0i8(i8* [[ADDR_BAR]])
+  if (__builtin_constant_p(foo) && __builtin_constant_p(bar))
+callee();
+}
+
+// Test other Objective-C types.
+// CHECK-LABEL: define void @test_more(i8* %object, i8* %klass)
+void test_more(id object, Class klass) {
+  // CHECK: call i1 @llvm.is.constant.p0i8(i8* %{{.*}})
+  // CHECK: call i1 @llvm.is.constant.p0i8(i8* %{{.*}})
+  if (__builtin_constant_p(object) && __builtin_constant_p(klass))
+callee();
+}


Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -1982,6 +1982,12 @@
   return RValue::get(ConstantInt::get(ResultType, 0));
 
 Value *ArgValue = EmitScalarExpr(Arg);
+if (ArgType->isObjCObjectPointerType()) {
+  // Convert Objective-C objects to id because we cannot distinguish between
+  // LLVM types for Obj-C classes as they are opaque.
+  ArgType = CGM.getContext().getObjCIdType();
+  ArgValue = Builder.CreateBitCast(ArgValue, ConvertType(ArgType));
+}
 Function *F =
 CGM.getIntrinsic(Intrinsic::is_constant, ConvertType(ArgType));
 Value *Result = Builder.CreateCall(F, ArgValue);
Index: cfe/trunk/test/CodeGenObjC/builtin-constant-p.m
===
--- cfe/trunk/test/CodeGenObjC/builtin-constant-p.m
+++ cfe/trunk/test/CodeGenObjC/builtin-constant-p.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -O3 -disable-llvm-passes -o - %s | FileCheck %s
+
+// Test that can call `__builtin_constant_p` with instances of different
+// Objective-C classes.
+// rdar://problem/47499250
+@class Foo;
+@class Bar;
+
+extern void callee(void);
+
+// CHECK-LABEL: define void @test(%0* %foo, %1* %bar)
+void test(Foo *foo, Bar *bar) {
+  // CHECK: [[ADDR_FOO:%.*]] = bitcast %0* %{{.*}} to i8*
+  // CHECK-NEXT: call i1 @llvm.is.constant.p0i8(i8* [[ADDR_FOO]])
+  // CHECK: [[ADDR_BAR:%.*]] = bitcast %1* %{{.*}} to i8*
+  // CHECK-NEXT: call i1 @llvm.is.constant.p0i8(i8* [[ADDR_BAR]])
+  if (__builtin_constant_p(foo) && __builtin_constant_p(bar))
+callee();
+}
+
+// Test other Objective-C types.
+// CHECK-LABEL: define void @test_more(i8* %object, i8* %klass)
+void test_more(id object, Class klass) {
+  // CHECK: call i1 @llvm.is.constant.p0i8(i8* %{{.*}})
+  // CHECK: call i1 @llvm.is.constant.p0i8(i8* %{{.*}})
+  if (__builtin_constant_p(object) && __builtin_constant_p(klass))
+callee();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r353577 - [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Fri Feb  8 15:02:13 2019
New Revision: 353577

URL: http://llvm.org/viewvc/llvm-project?rev=353577=rev
Log:
[CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

When we are calling `__builtin_constant_p` with ObjC objects of
different classes, we hit the assertion

> Assertion failed: (isa(Val) && "cast() argument of incompatible 
> type!"), function cast, file include/llvm/Support/Casting.h, line 254.

It happens because LLVM types for `ObjCInterfaceType` are opaque and
have no name (see `CodeGenTypes::ConvertType`). As the result, for
different ObjC classes we have different `is_constant` intrinsics with
the same name `llvm.is.constant.p0s_s`. When we try to reuse an
intrinsic with the same name, we fail because of type mismatch.

Fix by bitcasting `ObjCObjectPointerType` to `id` prior to passing as an
argument to `__builtin_constant_p`. This results in using intrinsic
`llvm.is.constant.p0i8` and correct types.

rdar://problem/47499250

Reviewers: rjmccall, ahatanak, void

Reviewed By: void, ahatanak

Subscribers: ddunbar, jkorous, hans, dexonsmith, cfe-commits

Differential Revision: https://reviews.llvm.org/D57427


Added:
cfe/trunk/test/CodeGenObjC/builtin-constant-p.m
Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=353577=353576=353577=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Feb  8 15:02:13 2019
@@ -1982,6 +1982,12 @@ RValue CodeGenFunction::EmitBuiltinExpr(
   return RValue::get(ConstantInt::get(ResultType, 0));
 
 Value *ArgValue = EmitScalarExpr(Arg);
+if (ArgType->isObjCObjectPointerType()) {
+  // Convert Objective-C objects to id because we cannot distinguish 
between
+  // LLVM types for Obj-C classes as they are opaque.
+  ArgType = CGM.getContext().getObjCIdType();
+  ArgValue = Builder.CreateBitCast(ArgValue, ConvertType(ArgType));
+}
 Function *F =
 CGM.getIntrinsic(Intrinsic::is_constant, ConvertType(ArgType));
 Value *Result = Builder.CreateCall(F, ArgValue);

Added: cfe/trunk/test/CodeGenObjC/builtin-constant-p.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/builtin-constant-p.m?rev=353577=auto
==
--- cfe/trunk/test/CodeGenObjC/builtin-constant-p.m (added)
+++ cfe/trunk/test/CodeGenObjC/builtin-constant-p.m Fri Feb  8 15:02:13 2019
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -O3 
-disable-llvm-passes -o - %s | FileCheck %s
+
+// Test that can call `__builtin_constant_p` with instances of different
+// Objective-C classes.
+// rdar://problem/47499250
+@class Foo;
+@class Bar;
+
+extern void callee(void);
+
+// CHECK-LABEL: define void @test(%0* %foo, %1* %bar)
+void test(Foo *foo, Bar *bar) {
+  // CHECK: [[ADDR_FOO:%.*]] = bitcast %0* %{{.*}} to i8*
+  // CHECK-NEXT: call i1 @llvm.is.constant.p0i8(i8* [[ADDR_FOO]])
+  // CHECK: [[ADDR_BAR:%.*]] = bitcast %1* %{{.*}} to i8*
+  // CHECK-NEXT: call i1 @llvm.is.constant.p0i8(i8* [[ADDR_BAR]])
+  if (__builtin_constant_p(foo) && __builtin_constant_p(bar))
+callee();
+}
+
+// Test other Objective-C types.
+// CHECK-LABEL: define void @test_more(i8* %object, i8* %klass)
+void test_more(id object, Class klass) {
+  // CHECK: call i1 @llvm.is.constant.p0i8(i8* %{{.*}})
+  // CHECK: call i1 @llvm.is.constant.p0i8(i8* %{{.*}})
+  if (__builtin_constant_p(object) && __builtin_constant_p(klass))
+callee();
+}


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

[PATCH] D57981: [analyzer] strlcat() syntax check: Fix an off-by-one error.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, george.karpenkov, xazax.hun, a_sidorin, 
rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware, devnexen.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.
Herald added a project: clang.

That's a fix on top of D49722 .

Both `strlcat` and `strlcpy` cut off their safe bound argument value at 
`sizeof(destination)`. There's no need to subtract 1 in one of these cases.


Repository:
  rC Clang

https://reviews.llvm.org/D57981

Files:
  lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
  test/Analysis/cstring-syntax.c


Index: test/Analysis/cstring-syntax.c
===
--- test/Analysis/cstring-syntax.c
+++ test/Analysis/cstring-syntax.c
@@ -33,6 +33,7 @@
   strlcpy(dest, src, ulen);
   strlcpy(dest + 5, src, 5);
   strlcpy(dest + 5, src, 10); // expected-warning {{The third argument allows 
to potentially copy more bytes than it should. Replace with the value 
sizeof() or lower}}
+  strlcpy(dest, "aaa", 10); // no-warning
 }
 
 void testStrlcat(const char *src) {
@@ -51,4 +52,5 @@
   strlcat(dest, src, ulen);
   strlcpy(dest, src, 5);
   strlcat(dest + 5, src, badlen); // expected-warning {{The third argument 
allows to potentially copy more bytes than it should. Replace with the value 
sizeof() or lower}}
+  strlcat(dest, "aaa", 10); // no-warning
 }
Index: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -153,8 +153,6 @@
 bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
   if (CE->getNumArgs() != 3)
 return false;
-  const FunctionDecl *FD = CE->getDirectCallee();
-  bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
   const Expr *DstArg = CE->getArg(0);
   const Expr *LenArg = CE->getArg(2);
 
@@ -194,13 +192,8 @@
 ASTContext  = BR.getContext();
 uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
 auto RemainingBufferLen = BufferLen - DstOff;
-if (Append) {
-  if (RemainingBufferLen <= ILRawVal)
-return true;
-} else {
-  if (RemainingBufferLen < ILRawVal)
-return true;
-}
+if (RemainingBufferLen < ILRawVal)
+  return true;
   }
 }
   }


Index: test/Analysis/cstring-syntax.c
===
--- test/Analysis/cstring-syntax.c
+++ test/Analysis/cstring-syntax.c
@@ -33,6 +33,7 @@
   strlcpy(dest, src, ulen);
   strlcpy(dest + 5, src, 5);
   strlcpy(dest + 5, src, 10); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof() or lower}}
+  strlcpy(dest, "aaa", 10); // no-warning
 }
 
 void testStrlcat(const char *src) {
@@ -51,4 +52,5 @@
   strlcat(dest, src, ulen);
   strlcpy(dest, src, 5);
   strlcat(dest + 5, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof() or lower}}
+  strlcat(dest, "aaa", 10); // no-warning
 }
Index: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -153,8 +153,6 @@
 bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
   if (CE->getNumArgs() != 3)
 return false;
-  const FunctionDecl *FD = CE->getDirectCallee();
-  bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
   const Expr *DstArg = CE->getArg(0);
   const Expr *LenArg = CE->getArg(2);
 
@@ -194,13 +192,8 @@
 ASTContext  = BR.getContext();
 uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
 auto RemainingBufferLen = BufferLen - DstOff;
-if (Append) {
-  if (RemainingBufferLen <= ILRawVal)
-return true;
-} else {
-  if (RemainingBufferLen < ILRawVal)
-return true;
-}
+if (RemainingBufferLen < ILRawVal)
+  return true;
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur created this revision.
Meinersbur added reviewers: hfinkel, aaron.ballman, hsaito, dmgreen, anemet, 
rjmccall, Anastasia, pekka.jaaskelainen, meheff, tyler.nowicki.
Herald added a subscriber: zzheng.
Herald added a project: clang.

Before this patch, CGLoop would dump all transformations for a loop into a 
single LoopID without encoding any order in which to apply them. rL348944 
 added the possibility to encode a 
transformation order using followup-attributes.

When a loop has more than one transformation, use the follow-up attribute 
define the order in which they are applied. The emitted order is the defacto 
order as defined by the current LLVM pass pipeline, which is:

1. LoopFullUnrollPass
2. LoopDistributePass
3. LoopVectorizePass
4. LoopUnrollAndJamPass
5. LoopUnrollPass
6. MachinePipeliner

This patch should therefore not change the assembly output, assuming that all 
explicit transformations can be applied, and no implicit transformations 
in-between. In the former case, WarnMissedTransformationsPass should emit a 
warning (except for MachinePipeliner which is not implemented yet). The latter 
could be avoided by adding 'llvm.loop.disable_nonforced' attributes.

Because LoopUnrollAndJamPass processes a loop nest, generation of the MDNode is 
delayed to after the inner loop metadata have been processed. A temporary 
LoopID is therefore used to annotate instructions and RAUW'ed by the actual 
LoopID later.


Repository:
  rC Clang

https://reviews.llvm.org/D57978

Files:
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  test/CodeGenCXX/pragma-loop-safety-imperfectly_nested.cpp
  test/CodeGenCXX/pragma-loop-safety-nested.cpp
  test/CodeGenCXX/pragma-loop-safety-outer.cpp
  test/CodeGenCXX/pragma-loop-safety.cpp
  test/CodeGenCXX/pragma-loop.cpp
  test/CodeGenCXX/pragma-unroll-and-jam.cpp
  test/OpenMP/simd_metadata.c

Index: test/OpenMP/simd_metadata.c
===
--- test/OpenMP/simd_metadata.c
+++ test/OpenMP/simd_metadata.c
@@ -147,16 +147,16 @@
 // CHECK: [[LOOP_H1_HEADER:![0-9]+]] = distinct !{[[LOOP_H1_HEADER]], [[LOOP_WIDTH_8:![0-9]+]], [[LOOP_VEC_ENABLE]]}
 // CHECK: [[LOOP_WIDTH_8]] = !{!"llvm.loop.vectorize.width", i32 8}
 // CHECK: ![[ACCESS_GROUP_7]] = distinct !{}
-// CHECK: [[LOOP_H1_HEADER:![0-9]+]] = distinct !{[[LOOP_H1_HEADER]], [[LOOP_WIDTH_8]], [[LOOP_VEC_ENABLE]], ![[PARALLEL_ACCESSES_9:[0-9]+]]}
+// CHECK: [[LOOP_H1_HEADER:![0-9]+]] = distinct !{[[LOOP_H1_HEADER]], ![[PARALLEL_ACCESSES_9:[0-9]+]], [[LOOP_WIDTH_8]], [[LOOP_VEC_ENABLE]]}
 // CHECK: ![[PARALLEL_ACCESSES_9]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_7]]}
 //
 // Metadata for h2:
 // CHECK: ![[ACCESS_GROUP_10]] = distinct !{}
-// CHECK: [[LOOP_H2_HEADER]] = distinct !{[[LOOP_H2_HEADER]], [[LOOP_VEC_ENABLE]], ![[PARALLEL_ACCESSES_12:[0-9]+]]}
+// CHECK: [[LOOP_H2_HEADER]] = distinct !{[[LOOP_H2_HEADER]], ![[PARALLEL_ACCESSES_12:[0-9]+]], [[LOOP_VEC_ENABLE]]}
 // CHECK: ![[PARALLEL_ACCESSES_12]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_10]]}
 //
 // Metadata for h3:
 // CHECK: ![[ACCESS_GROUP_13]] = distinct !{}
-// CHECK: [[LOOP_H3_HEADER]] = distinct !{[[LOOP_H3_HEADER]], [[LOOP_VEC_ENABLE]], ![[PARALLEL_ACCESSES_15:[0-9]+]]}
+// CHECK: [[LOOP_H3_HEADER]] = distinct !{[[LOOP_H3_HEADER]], ![[PARALLEL_ACCESSES_15:[0-9]+]], [[LOOP_VEC_ENABLE]]}
 // CHECK: ![[PARALLEL_ACCESSES_15]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_13]]}
 //
Index: test/CodeGenCXX/pragma-unroll-and-jam.cpp
===
--- test/CodeGenCXX/pragma-unroll-and-jam.cpp
+++ test/CodeGenCXX/pragma-unroll-and-jam.cpp
@@ -51,5 +51,5 @@
 // CHECK: ![[UNJ_4]] = !{!"llvm.loop.unroll_and_jam.count", i32 4}
 // CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[UNJ_DISABLE:.*]]}
 // CHECK: ![[UNJ_DISABLE]] = !{!"llvm.loop.unroll_and_jam.disable"}
-// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[UNROLL_4:.*]], ![[UNJ_DISABLE:.*]]}
+// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[UNJ_DISABLE:.*]], ![[UNROLL_4:.*]]}
 // CHECK: ![[UNROLL_4]] = !{!"llvm.loop.unroll.count", i32 4}
Index: test/CodeGenCXX/pragma-loop.cpp
===
--- test/CodeGenCXX/pragma-loop.cpp
+++ test/CodeGenCXX/pragma-loop.cpp
@@ -158,37 +158,60 @@
   for_template_constant_expression_test(List, Length);
 }
 
-// CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[WIDTH_4:.*]], ![[INTERLEAVE_4:.*]], ![[INTENABLE_1:.*]], ![[UNROLL_FULL:.*]], ![[DISTRIBUTE_ENABLE:.*]]}
-// CHECK: ![[WIDTH_4]] = !{!"llvm.loop.vectorize.width", i32 4}
-// CHECK: ![[INTERLEAVE_4]] = !{!"llvm.loop.interleave.count", i32 4}
-// CHECK: ![[INTENABLE_1]] = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[UNROLL_FULL:.*]]}
 // CHECK: ![[UNROLL_FULL]] = !{!"llvm.loop.unroll.full"}
-// CHECK: ![[DISTRIBUTE_ENABLE]] = 

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak accepted this revision.
ahatanak added a comment.

LGTM


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

https://reviews.llvm.org/D57427



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

[PATCH] D57977: [HIP] compile option code-object-v3 propagate to llc

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 created this revision.
ashi1 added reviewers: yaxunl, kzhuravl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch allows the compile option -mcode-object-v3 to be set by default, 
unless the -mno-code-object-v3 is added to the compile options.


Repository:
  rC Clang

https://reviews.llvm.org/D57977

Files:
  lib/Driver/ToolChains/HIP.cpp


Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -157,12 +157,20 @@
 Compilation , const JobAction , const InputInfoList ,
 const llvm::opt::ArgList , llvm::StringRef SubArchName,
 llvm::StringRef OutputFilePrefix, const char *InputFileName) const {
+
+  llvm::StringRef CodeObjectVersion;
+  // Add Code Object Version
+  if (Args.hasArg(options::OPT_mno_code_object_v3))
+CodeObjectVersion = "-mattr=-code-object-v3";
+  else
+CodeObjectVersion = "-mattr=+code-object-v3";
+
   // Construct llc command.
   // FIXME: -disable-promote-alloca-to-lds is a workaround for issues in
   // AMDGPUPromoteAlloca pass which cause invalid memory access in PyTorch.
   // Remove this once the issue is fixed.
   ArgStringList LlcArgs{InputFileName, "-mtriple=amdgcn-amd-amdhsa",
-"-filetype=obj", "-mattr=-code-object-v3",
+"-filetype=obj", Args.MakeArgString(CodeObjectVersion),
 "-disable-promote-alloca-to-lds",
 Args.MakeArgString("-mcpu=" + SubArchName), "-o"};
   std::string LlcOutputFileName =


Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -157,12 +157,20 @@
 Compilation , const JobAction , const InputInfoList ,
 const llvm::opt::ArgList , llvm::StringRef SubArchName,
 llvm::StringRef OutputFilePrefix, const char *InputFileName) const {
+
+  llvm::StringRef CodeObjectVersion;
+  // Add Code Object Version
+  if (Args.hasArg(options::OPT_mno_code_object_v3))
+CodeObjectVersion = "-mattr=-code-object-v3";
+  else
+CodeObjectVersion = "-mattr=+code-object-v3";
+
   // Construct llc command.
   // FIXME: -disable-promote-alloca-to-lds is a workaround for issues in
   // AMDGPUPromoteAlloca pass which cause invalid memory access in PyTorch.
   // Remove this once the issue is fixed.
   ArgStringList LlcArgs{InputFileName, "-mtriple=amdgcn-amd-amdhsa",
-"-filetype=obj", "-mattr=-code-object-v3",
+"-filetype=obj", Args.MakeArgString(CodeObjectVersion),
 "-disable-promote-alloca-to-lds",
 Args.MakeArgString("-mcpu=" + SubArchName), "-o"};
   std::string LlcOutputFileName =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57976: -gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: JDevlieghere, bruno.

When a module name is specified as -fmodule-name, that module gets a
clang::Module object, but it won't actually be built or imported; it
will be textual. CGDebugInfo wouldn't detect this and them emit a
DICompileUnit that had a hash but no name and that confused both
dsymutil, LLDB, and myself.

rdar://problem/47926508


https://reviews.llvm.org/D57976

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/Modules/DebugInfo-fmodule-name.c


Index: test/Modules/DebugInfo-fmodule-name.c
===
--- /dev/null
+++ test/Modules/DebugInfo-fmodule-name.c
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodule-format=obj -fmodule-name=F \
+// RUN: -debug-info-kind=limited -dwarf-ext-refs \
+// RUN: -fimplicit-module-maps -x c -fmodules-cache-path=%t -F %S/Inputs \
+// RUN: %s -S -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s
+
+#include "F/F.h"
+
+// CHECK: !DICompileUnit
+// CHECK-NOT: dwoId:
+
+// We still want the import, but no skeleton CU, since no PCM was built.
+
+// CHECK: !DIModule({{.*}}, name: "F"
+// CHECK-NOT: !DICompileUnit
+// CHECK-NOT: dwoId:
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2296,7 +2296,14 @@
   }
 
   bool IsRootModule = M ? !M->Parent : true;
-  if (CreateSkeletonCU && IsRootModule) {
+  // When a module name is specified as -fmodule-name, that module gets a
+  // clang::Module object, but it won't actually be built or imported; it will
+  // be textual.
+  if (CreateSkeletonCU && IsRootModule && Mod.getASTFile().empty())
+assert((!M || (M->Name == CGM.getLangOpts().ModuleName)) &&
+   "clang module without ASTFile was not specified by -fmodule-name");
+
+  if (CreateSkeletonCU && IsRootModule && !Mod.getASTFile().empty()) {
 // PCH files don't have a signature field in the control block,
 // but LLVM detects skeleton CUs by looking for a non-zero DWO id.
 // We use the lower 64 bits for debug info.
@@ -2313,6 +2320,7 @@
   Signature);
 DIB.finalize();
   }
+
   llvm::DIModule *Parent =
   IsRootModule ? nullptr
: getOrCreateModuleRef(


Index: test/Modules/DebugInfo-fmodule-name.c
===
--- /dev/null
+++ test/Modules/DebugInfo-fmodule-name.c
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodule-format=obj -fmodule-name=F \
+// RUN: -debug-info-kind=limited -dwarf-ext-refs \
+// RUN: -fimplicit-module-maps -x c -fmodules-cache-path=%t -F %S/Inputs \
+// RUN: %s -S -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s
+
+#include "F/F.h"
+
+// CHECK: !DICompileUnit
+// CHECK-NOT: dwoId:
+
+// We still want the import, but no skeleton CU, since no PCM was built.
+
+// CHECK: !DIModule({{.*}}, name: "F"
+// CHECK-NOT: !DICompileUnit
+// CHECK-NOT: dwoId:
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2296,7 +2296,14 @@
   }
 
   bool IsRootModule = M ? !M->Parent : true;
-  if (CreateSkeletonCU && IsRootModule) {
+  // When a module name is specified as -fmodule-name, that module gets a
+  // clang::Module object, but it won't actually be built or imported; it will
+  // be textual.
+  if (CreateSkeletonCU && IsRootModule && Mod.getASTFile().empty())
+assert((!M || (M->Name == CGM.getLangOpts().ModuleName)) &&
+   "clang module without ASTFile was not specified by -fmodule-name");
+
+  if (CreateSkeletonCU && IsRootModule && !Mod.getASTFile().empty()) {
 // PCH files don't have a signature field in the control block,
 // but LLVM detects skeleton CUs by looking for a non-zero DWO id.
 // We use the lower 64 bits for debug info.
@@ -2313,6 +2320,7 @@
   Signature);
 DIB.finalize();
   }
+
   llvm::DIModule *Parent =
   IsRootModule ? nullptr
: getOrCreateModuleRef(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57966: [clang-tidy] add camelBackOrCase casing style to readability-identifier-naming to support change to variable naming policy (if adopted)

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/readability-identifier-naming.rst:30
 
+``camelBackOrCase`` allows for both `camelBack` or `CamelCase`  cased
+identifiers to be used to match prior LLVM code style

Please fix double space.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57966



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D57874#1390014 , @sbc100 wrote:

> In D57874#1389981 , @sunfish wrote:
>
> > > - `-matomics` means `-mthread-model posix`
> >
> > The others sound reasonable, though this one seems a little surprising -- a 
> > user might have -matomics enabled because they're targeting a VM that has 
> > atomics, but still not want to use -mthread-model posix because their code 
> > doesn't actually using threads.
>
>
> If you look at what "-mthread-model posix" actually means in the codebase it 
> basically means "don't lower away atomics ops".
>
> Its only used by WebAssembly and ARMTargetMachine to select the 
> `createLowerAtomicPass`.   Given that these are the only uses it seems like 
> it should be removed or renamed.


FWIW, I plan to use the thread-model argument to determine what kind of 
initialization flags data segments should use as well. Will share a short doc 
detailing this soon.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D57427#1386781 , @hans wrote:

> What's the status of this patch? Is it something we should merge onto the 8.0 
> release branch?


Waiting on a one more review iteration from @ahatanak and hope that afterwards 
the patch should be good to go.

As for suitability for 8.0 release branch, I would ask @rjmccall for his 
opinion. As for me, I'm not entirely comfortable with CodeGen changes late in 
the release cycle. Also looks like in practice it shouldn't be a serious 
problem, `__builtin_constant_p` can be optimized out. For example, see 
https://godbolt.org/z/e1b4dB


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

https://reviews.llvm.org/D57427



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

r353571 - [CodeGen][NFC] Update comments in CGExprConstant.cpp.

[Eli Friedman via cfe-commits]

Author: efriedma
Date: Fri Feb  8 13:36:04 2019
New Revision: 353571

URL: http://llvm.org/viewvc/llvm-project?rev=353571=rev
Log:
[CodeGen][NFC] Update comments in CGExprConstant.cpp.


Modified:
cfe/trunk/lib/CodeGen/CGExprConstant.cpp

Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=353571=353570=353571=diff
==
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Fri Feb  8 13:36:04 2019
@@ -700,10 +700,12 @@ EmitArrayConstant(CodeGenModule , co
   return llvm::ConstantStruct::get(SType, Elements);
 }
 
-/// This class only needs to handle two cases:
-/// 1) Literals (this is used by APValue emission to emit literals).
-/// 2) Arrays, structs and unions (outside C++11 mode, we don't currently
-///constant fold these types).
+// This class only needs to handle arrays, structs and unions. Outside C++11
+// mode, we don't currently constant fold those types.  All other types are
+// handled by constant folding.
+//
+// Constant folding is currently missing support for a few features supported
+// here: CK_ToUnion, CK_ReinterpretMemberPointer, and DesignatedInitUpdateExpr.
 class ConstExprEmitter :
   public StmtVisitor {
   CodeGenModule 
@@ -1076,6 +1078,7 @@ public:
   }
 
   llvm::Constant *VisitStringLiteral(StringLiteral *E, QualType T) {
+// This is a string literal initializing an array in an initializer.
 return CGM.GetConstantArrayFromStringLiteral(E);
   }
 
@@ -1649,7 +1652,7 @@ private:
 llvm::Constant *ConstantLValueEmitter::tryEmit() {
   const APValue::LValueBase  = Value.getLValueBase();
 
-  // Otherwise, the destination type should be a pointer or reference
+  // The destination type should be a pointer or reference
   // type, but it might also be a cast thereof.
   //
   // FIXME: the chain of casts required should be reflected in the APValue.


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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D57874#1389953 , @aheejin wrote:

> Anyway, moved all logic to the driver layer and did this:
>
> - `-matomics` means `-mthread-model posix`
> - `-mthread-model posix` means `-matomics`
> - `-pthread` means both `-matomics` and `-mthread-model posix`


If you replace "-matomics" with "-mbulk-memory," I plan to duplicate the logic 
for items 2 and 3 above, but not 1. For bulk memory even more than atomics, 
there is a legitimate usecase for enabling it even in single threaded contexts 
(namely getting to use memory.copy and memory.fill). I wonder if consistency 
with how bulk memory works is a strong enough argument for dropping item 1 for 
atomics as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

@ahatanak, can you please take a look again?


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

https://reviews.llvm.org/D57427



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

[PATCH] D57935: [Sema] Make string literal init an rvalue.

[Eli Friedman via Phabricator via cfe-commits]

efriedma closed this revision.
efriedma added a comment.

https://reviews.llvm.org/rC353569


Repository:
  rC Clang

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

https://reviews.llvm.org/D57935



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D57874#1391116 , @tlively wrote:

> In D57874#1389953 , @aheejin wrote:
>
> > - `-matomics` means `-mthread-model posix`
>
>
> Why should this be the case? Atomic instructions are necessary for 
> multithreading, but I wouldn't think multithreading would be necessary for 
> atomic instructions. Certainly atomics are not very useful in a single 
> threaded context, but that doesn't seem like a strong enough reason to 
> implicitly opt in to the rest of multithreading when -matomics is provided by 
> itself.


Sorry, missed previous discussion.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D57874#1389953 , @aheejin wrote:

> - `-matomics` means `-mthread-model posix`


Why should this be the case? Atomic instructions are necessary for 
multithreading, but I wouldn't think multithreading would be necessary for 
atomic instructions. Certainly atomics are not very useful in a single threaded 
context, but that doesn't seem like a strong enough reason to implicitly opt in 
to the rest of multithreading when -matomics is provided by itself.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

r353569 - [Sema] Make string literal init an rvalue.

[Eli Friedman via cfe-commits]

Author: efriedma
Date: Fri Feb  8 13:18:46 2019
New Revision: 353569

URL: http://llvm.org/viewvc/llvm-project?rev=353569=rev
Log:
[Sema] Make string literal init an rvalue.

This allows substantially simplifying the expression evaluation code,
because we don't have to special-case lvalues which are actually string
literal initialization.

This currently throws away an optimization where we would avoid creating
an array APValue for string literal initialization.  If we really want
to optimize this case, we should fix APValue so it can store simple
arrays more efficiently, like llvm::ConstantDataArray.  This shouldn't
affect the memory usage for other string literals.  (Not sure if this is
a blocker; I don't think string literal init is common enough for this
to be a serious issue, but I could be wrong.)

The change to test/CodeGenObjC/encode-test.m is a weird side-effect of
these changes: we currently don't constant-evaluate arrays in C, so the
strlen call shouldn't be folded, but lvalue string init managed to get
around that check.  I this this is fine.

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


Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/AST/ast-dump-wchar.cpp
cfe/trunk/test/CodeGenObjC/encode-test.m
cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=353569=353568=353569=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Feb  8 13:18:46 2019
@@ -2688,9 +2688,11 @@ static APSInt extractStringLiteralCharac
 }
 
 // Expand a string literal into an array of characters.
-static void expandStringLiteral(EvalInfo , const Expr *Lit,
+//
+// FIXME: This is inefficient; we should probably introduce something similar
+// to the LLVM ConstantDataArray to make this cheaper.
+static void expandStringLiteral(EvalInfo , const StringLiteral *S,
 APValue ) {
-  const StringLiteral *S = cast(Lit);
   const ConstantArrayType *CAT =
   Info.Ctx.getAsConstantArrayType(S->getType());
   assert(CAT && "string literal isn't an array");
@@ -2884,18 +2886,6 @@ findSubobject(EvalInfo , const Expr
 
   ObjType = CAT->getElementType();
 
-  // An array object is represented as either an Array APValue or as an
-  // LValue which refers to a string literal.
-  if (O->isLValue()) {
-assert(I == N - 1 && "extracting subobject of character?");
-assert(!O->hasLValuePath() || O->getLValuePath().empty());
-if (handler.AccessKind != AK_Read)
-  expandStringLiteral(Info, O->getLValueBase().get(),
-  *O);
-else
-  return handler.foundString(*O, ObjType, Index);
-  }
-
   if (O->getArrayInitializedElts() > Index)
 O = >getArrayInitializedElt(Index);
   else if (handler.AccessKind != AK_Read) {
@@ -3008,11 +2998,6 @@ struct ExtractSubobjectHandler {
 Result = APValue(Value);
 return true;
   }
-  bool foundString(APValue , QualType SubobjType, uint64_t Character) {
-Result = APValue(extractStringLiteralCharacter(
-Info, Subobj.getLValueBase().get(), Character));
-return true;
-  }
 };
 } // end anonymous namespace
 
@@ -3070,9 +3055,6 @@ struct ModifySubobjectHandler {
 Value = NewVal.getFloat();
 return true;
   }
-  bool foundString(APValue , QualType SubobjType, uint64_t Character) {
-llvm_unreachable("shouldn't encounter string elements with ExpandArrays");
-  }
 };
 } // end anonymous namespace
 
@@ -3386,12 +3368,20 @@ static bool handleLValueToRValueConversi
   CompleteObject LitObj(, Base->getType(), false);
   return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal);
 } else if (isa(Base) || isa(Base)) {
-  // We represent a string literal array as an lvalue pointing at the
-  // corresponding expression, rather than building an array of chars.
-  // FIXME: Support ObjCEncodeExpr, MakeStringConstant
-  APValue Str(Base, CharUnits::Zero(), APValue::NoLValuePath(), 0);
-  CompleteObject StrObj(, Base->getType(), false);
-  return extractSubobject(Info, Conv, StrObj, LVal.Designator, RVal);
+  // Special-case character extraction so we don't have to construct an
+  // APValue for the whole string.
+  assert(LVal.Designator.Entries.size() == 1 &&
+ "Can only read characters from string literals");
+  if (LVal.Designator.isOnePastTheEnd()) {
+if (Info.getLangOpts().CPlusPlus11)
+  Info.FFDiag(Conv, diag::note_constexpr_access_past_end) << AK_Read;
+else
+  Info.FFDiag(Conv);
+return false;
+  }
+  uint64_t CharIndex = LVal.Designator.Entries[0].ArrayIndex;
+  RVal 

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Thomas Lively via Phabricator via cfe-commits]

tlively added inline comments.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:66
+if (Args.hasFlag(clang::driver::options::OPT_pthread,
+ clang::driver::options::OPT_no_pthread),
+false)

sbc100 wrote:
> aheejin wrote:
> > This code is not strictly related, but `hasFlag` is better than `hasArg` 
> > when there are both positive and negative versions of an option exist.
> Hmm.. there are currently no other references to OPT_no_pthread in the whole 
> codebase.   Maybe better to simply remove the option?
> 
> I wouldn't want to commit this as that first use of the option as it might 
> make it hard to remove :)
I think commands generally come in pairs to make it possible to override 
previous settings by appending args to command lines. Counting uses of 
OPT_no_pthread without including uses of OP_pthread doesn't make much sense.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

r353568 - Fix typo

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Fri Feb  8 13:13:25 2019
New Revision: 353568

URL: http://llvm.org/viewvc/llvm-project?rev=353568=rev
Log:
Fix typo

Modified:
cfe/trunk/include/clang/Basic/LangOptions.h

Modified: cfe/trunk/include/clang/Basic/LangOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=353568=353567=353568=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.h (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.h Fri Feb  8 13:13:25 2019
@@ -215,7 +215,7 @@ public:
   /// If none is specified, abort (GCC-compatible behaviour).
   std::string OverflowHandler;
 
-  /// The module currently being compiled as speficied by -fmodule-name.
+  /// The module currently being compiled as specified by -fmodule-name.
   std::string ModuleName;
 
   /// The name of the current module, of which the main source file


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

[PATCH] D57270: [ObjC] Fix non-canonical types preventing type arguments substitution.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


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

https://reviews.llvm.org/D57270



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

[PATCH] D57628: [index] Improve indexing support for MSPropertyDecl.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


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

https://reviews.llvm.org/D57628



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

[PATCH] D57261: [analyzer] [WIP] Opt-in C Style Cast Checker for OSObject pointers

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353566: [analyzer] Opt-in C Style Cast Checker for OSObject 
pointers (authored by george.karpenkov, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57261?vs=183630=186035#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57261

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
  test/Analysis/osobjectcstylecastchecker_test.cpp

Index: test/Analysis/osobjectcstylecastchecker_test.cpp
===
--- test/Analysis/osobjectcstylecastchecker_test.cpp
+++ test/Analysis/osobjectcstylecastchecker_test.cpp
@@ -0,0 +1,39 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.osx.OSObjectCStyleCast %s -verify
+#include "os_object_base.h"
+
+struct OSArray : public OSObject {
+  unsigned getCount();
+};
+
+struct A {
+  int x;
+};
+struct B : public A {
+  unsigned getCount();
+};
+
+unsigned warn_on_explicit_downcast(OSObject * obj) {
+  OSArray *a = (OSArray *) obj; // expected-warning{{C-style cast of OSObject. Use OSDynamicCast instead}}
+  return a->getCount();
+}
+
+void no_warn_on_upcast(OSArray *arr) {
+  OSObject *obj = (OSObject *) arr;
+  obj->retain();
+  obj->release();
+}
+
+unsigned no_warn_on_dynamic_cast(OSObject *obj) {
+  OSArray *a = OSDynamicCast(OSArray, obj);
+  return a->getCount();
+}
+
+unsigned long no_warn_on_primitive_conversion(OSArray *arr) {
+  return (unsigned long) arr;
+}
+
+unsigned no_warn_on_other_type_cast(A *a) {
+  B *b = (B *) a;
+  return b->getCount();
+}
+
Index: lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
===
--- lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
+++ lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
@@ -0,0 +1,90 @@
+//===- OSObjectCStyleCast.cpp *- C++ -*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines OSObjectCStyleCast checker, which checks for C-style casts
+// of OSObjects. Such casts almost always indicate a code smell,
+// as an explicit static or dynamic cast should be used instead.
+//===--===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "llvm/Support/Debug.h"
+
+using namespace clang;
+using namespace ento;
+using namespace ast_matchers;
+
+namespace {
+
+const char *WarnAtNode = "OSObjCast";
+
+class OSObjectCStyleCastChecker : public Checker {
+public:
+  void checkASTCodeBody(const Decl *D,
+AnalysisManager ,
+BugReporter ) const;
+};
+
+static void emitDiagnostics(const BoundNodes ,
+BugReporter ,
+AnalysisDeclContext *ADC,
+const OSObjectCStyleCastChecker *Checker) {
+  const auto *CE = Nodes.getNodeAs(WarnAtNode);
+  assert(CE);
+
+  std::string Diagnostics;
+  llvm::raw_string_ostream OS(Diagnostics);
+  OS << "C-style cast of OSObject. Use OSDynamicCast instead.";
+
+  BR.EmitBasicReport(
+ADC->getDecl(),
+Checker,
+/*Name=*/"OSObject C-Style Cast",
+/*Category=*/"Security",
+OS.str(),
+PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), ADC),
+CE->getSourceRange());
+}
+
+static auto hasTypePointingTo(DeclarationMatcher DeclM)
+-> decltype(hasType(pointerType())) {
+  return hasType(pointerType(pointee(hasDeclaration(DeclM;
+}
+
+void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, AnalysisManager ,
+ BugReporter ) const {
+
+  AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D);
+
+  auto DynamicCastM = callExpr(callee(functionDecl(hasName("safeMetaCast";
+
+  auto OSObjTypeM = hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase")));
+  auto OSObjSubclassM = hasTypePointingTo(
+cxxRecordDecl(isDerivedFrom("OSObject")));
+
+  auto CastM = cStyleCastExpr(
+  allOf(hasSourceExpression(allOf(OSObjTypeM, unless(DynamicCastM))),
+  OSObjSubclassM)).bind(WarnAtNode);
+
+  auto Matches = 

r353566 - [analyzer] Opt-in C Style Cast Checker for OSObject pointers

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Feb  8 13:09:00 2019
New Revision: 353566

URL: http://llvm.org/viewvc/llvm-project?rev=353566=rev
Log:
[analyzer] Opt-in C Style Cast Checker for OSObject pointers

Differential Revision: https://reviews.llvm.org/D57261

Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
cfe/trunk/test/Analysis/osobjectcstylecastchecker_test.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=353566=353565=353566=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Fri Feb  8 
13:09:00 2019
@@ -812,6 +812,14 @@ def GCDAntipattern : Checker<"GCDAntipat
   Documentation;
 } // end "optin.performance"
 
+let ParentPackage = OSXOptIn in {
+
+def OSObjectCStyleCast : Checker<"OSObjectCStyleCast">,
+  HelpText<"Checker for C-style casts of OSObjects">,
+  Documentation;
+
+} // end "optin.osx"
+
 let ParentPackage = CocoaAlpha in {
 
 def IvarInvalidationModeling : Checker<"IvarInvalidationModeling">,

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=353566=353565=353566=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Fri Feb  8 13:09:00 
2019
@@ -71,6 +71,7 @@ add_clang_library(clangStaticAnalyzerChe
   ObjCSelfInitChecker.cpp
   ObjCSuperDeallocChecker.cpp
   ObjCUnusedIVarsChecker.cpp
+  OSObjectCStyleCast.cpp
   PaddingChecker.cpp
   PointerArithChecker.cpp
   PointerSubChecker.cpp

Added: cfe/trunk/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp?rev=353566=auto
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp Fri Feb  8 
13:09:00 2019
@@ -0,0 +1,90 @@
+//===- OSObjectCStyleCast.cpp *- C++ 
-*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines OSObjectCStyleCast checker, which checks for C-style casts
+// of OSObjects. Such casts almost always indicate a code smell,
+// as an explicit static or dynamic cast should be used instead.
+//===--===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "llvm/Support/Debug.h"
+
+using namespace clang;
+using namespace ento;
+using namespace ast_matchers;
+
+namespace {
+
+const char *WarnAtNode = "OSObjCast";
+
+class OSObjectCStyleCastChecker : public Checker {
+public:
+  void checkASTCodeBody(const Decl *D,
+AnalysisManager ,
+BugReporter ) const;
+};
+
+static void emitDiagnostics(const BoundNodes ,
+BugReporter ,
+AnalysisDeclContext *ADC,
+const OSObjectCStyleCastChecker *Checker) {
+  const auto *CE = Nodes.getNodeAs(WarnAtNode);
+  assert(CE);
+
+  std::string Diagnostics;
+  llvm::raw_string_ostream OS(Diagnostics);
+  OS << "C-style cast of OSObject. Use OSDynamicCast instead.";
+
+  BR.EmitBasicReport(
+ADC->getDecl(),
+Checker,
+/*Name=*/"OSObject C-Style Cast",
+/*Category=*/"Security",
+OS.str(),
+PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), ADC),
+CE->getSourceRange());
+}
+
+static auto hasTypePointingTo(DeclarationMatcher DeclM)
+-> decltype(hasType(pointerType())) {
+  return hasType(pointerType(pointee(hasDeclaration(DeclM;
+}
+
+void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, 
AnalysisManager ,
+ BugReporter ) const {
+
+  AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D);
+
+  auto DynamicCastM = 

[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

> FWIW, I was rather disappointed in a recent review to learn that 
> IgnoreParens() means "ignore parens... and a whole bunch of other stuff like 
> generic selection expressions".

+1 Indeed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57267



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

[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D57267#1391101 , @riccibruno wrote:

> > I don't think there was an explicit reason beyond "I didn't need to do it 
> > at the time". So probably just an oversight on my part. I don't know the 
> > code nearly as well as @rnk, so I could be wrong, but I think the existing 
> > tests should tell you if something went haywire if you skip FullExprs.
>
> I see. I am a bit weary about relying on test coverage to validate a change. 
> It is certainly a necessary condition but I am not sure that it is a 
> sufficient condition.
>
> > Yes, @rsmith asked me to skip all FullExprs, but that change did not pass 
> > tests, so I only made IgnoreParens ignore ConstantExpr to preserve existing 
> > behavior. There is no good design reason for it to be that way, and if you 
> > can adjust the callers to account for the new behavior, I think making them 
> > consistent would be good.
>
> I agree, but for now what I would like to do is just make sure that 
> `IgnoreParenImpCasts` skips, among other things, every node skipped by 
> `IgnoreImpCasts`. Not doing so is highly misleading given the names. (An 
> other fun one is that you would think that `IgnoreParenImpCasts()` = 
> `IgnoreParens()` + `IgnoreImpCasts()` but sadly that's not the case since 
> `IgnoreParenImpCasts()` skips additionally `MaterializeTemporaryExpr` and 
> `SubstNonTypeTemplateParmExpr`, even though `IgnoreParenCasts()` = 
> `IgnoreParens()` + `IgnoreCasts()`).


FWIW, I was rather disappointed in a recent review to learn that 
`IgnoreParens()` means "ignore parens... and a whole bunch of other stuff like 
generic selection expressions".


Repository:
  rC Clang

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

https://reviews.llvm.org/D57267



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

[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

> I don't think there was an explicit reason beyond "I didn't need to do it at 
> the time". So probably just an oversight on my part. I don't know the code 
> nearly as well as @rnk, so I could be wrong, but I think the existing tests 
> should tell you if something went haywire if you skip FullExprs.

I see. I am a bit weary about relying on test coverage to validate a change. It 
is certainly a necessary condition but I am not sure that it is a sufficient 
condition.

> Yes, @rsmith asked me to skip all FullExprs, but that change did not pass 
> tests, so I only made IgnoreParens ignore ConstantExpr to preserve existing 
> behavior. There is no good design reason for it to be that way, and if you 
> can adjust the callers to account for the new behavior, I think making them 
> consistent would be good.

I agree, but for now what I would like to do is just make sure that 
`IgnoreParenImpCasts` skips, among other things, every node skipped by 
`IgnoreImpCasts`. Not doing so is highly misleading given the names. (An other 
fun one is that you would think that `IgnoreParenImpCasts()` = `IgnoreParens()` 
+ `IgnoreImpCasts()` but sadly that's not the case since 
`IgnoreParenImpCasts()` skips additionally `MaterializeTemporaryExpr` and 
`SubstNonTypeTemplateParmExpr`, even though `IgnoreParenCasts()` = 
`IgnoreParens()` + `IgnoreCasts()`).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57267



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

[PATCH] D57935: [Sema] Make string literal init an rvalue.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

Thanks, this is a much cleaner model than having two different `APValue` 
representations for arrays.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57935



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

[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D57267#1390484 , @riccibruno wrote:

> @void @rnk Perhaps you can comment on this: currently `Expr::IgnoreImpCasts` 
> skips `FullExpr`s, but `Expr::IgnoreParenImpCasts` only skips (via 
> `IgnoreParens`) `ConstantExpr`s. Is there any reason for this inconsistency ? 
> I would like to add `FullExpr` to the nodes skipped by `IgnoreParenImpCasts` 
> for consistency but I am worried about unexpected issues even though all 
> tests pass.


Yes, @rsmith asked me to skip all FullExprs, but that change did not pass 
tests, so I only made IgnoreParens ignore ConstantExpr to preserve existing 
behavior. There is no good design reason for it to be that way, and if you can 
adjust the callers to account for the new behavior, I think making them 
consistent would be good.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57267



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

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:987
+
+Only a specific set of standard library functions are supported:
+  - memcpy

aaron.ballman wrote:
> And we don't have Annex K versions for any of these, like memset_s()?
No, it doesn't look like clang even recognizes these yet, and our c library 
doesn't define _chk functions for them. Whenever that happens, it would good to 
support them though I guess.


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

https://reviews.llvm.org/D57918



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

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 186028.
erik.pilkington marked 2 inline comments as done.
erik.pilkington added a comment.

Describe what the arguments do in the docs.


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

https://reviews.llvm.org/D57918

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Builtins.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Builtins.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/fortify-std-lib.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/fortify-std-lib.c

Index: clang/test/Sema/fortify-std-lib.c
===
--- /dev/null
+++ clang/test/Sema/fortify-std-lib.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only %s -verify
+
+typedef unsigned long size_t;
+
+__attribute__((fortify_stdlib(0, 0)))
+int not_anything_special(); // expected-error {{'fortify_stdlib' attribute applied to an unknown function}}
+
+__attribute__((fortify_stdlib(4, 0))) // expected-error {{'fortify_stdlib' attribute requires integer constant between 0 and 3 inclusive}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib())) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(1, 2, 3))) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(-1, 2))) // expected-error {{'fortify_stdlib' attribute requires a non-negative integral compile time constant expression}}
+int sprintf(char *, const char *, ...);
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -52,6 +52,7 @@
 // CHECK-NEXT: ExternalSourceSymbol ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum)
 // CHECK-NEXT: Flatten (SubjectMatchRule_function)
+// CHECK-NEXT: FortifyStdLib (SubjectMatchRule_function)
 // CHECK-NEXT: GNUInline (SubjectMatchRule_function)
 // CHECK-NEXT: Hot (SubjectMatchRule_function)
 // CHECK-NEXT: IBAction (SubjectMatchRule_objc_method_is_instance)
Index: clang/test/CodeGen/fortify-std-lib.c
===
--- /dev/null
+++ clang/test/CodeGen/fortify-std-lib.c
@@ -0,0 +1,220 @@
+// RUN: %clang_cc1   -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+
+#ifdef __cplusplus
+#define EXTERN extern "C"
+#else
+#define EXTERN
+#endif
+
+#define FSL(x,y) __attribute__((fortify_stdlib(x,y)))
+typedef unsigned long size_t;
+
+FSL(0, 0) EXTERN
+void *memcpy(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memcpy(void *dst, const void *src, size_t sz) {
+  memcpy(dst, src, sz);
+  // CHECK-LABEL: define void @call_memcpy
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memcpy_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memmove(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memmove(void *dst, const void *src, size_t sz) {
+  memmove(dst, src, sz);
+  // CHECK-LABEL: define void @call_memmove
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memmove_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memset(void *dst, int c, size_t sz);
+
+EXTERN
+void call_memset(void *dst, int c, size_t sz) {
+  memset(dst, c, sz);
+  // CHECK-LABEL: define void @call_memset
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memset_chk(i8* {{.*}}, i32 {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+char *stpcpy(char* dst, const char *src);
+
+EXTERN
+void call_stpcpy(char *dst, const char *src) {
+  stpcpy(dst, src);
+  // CHECK-LABEL: define void @call_stpcpy
+  // CHECK: [[REG:%[0-9]+]] = call i64 

[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D57267#1390484 , @riccibruno wrote:

> @void @rnk Perhaps you can comment on this: currently `Expr::IgnoreImpCasts` 
> skips `FullExpr`s, but `Expr::IgnoreParenImpCasts` only skips (via 
> `IgnoreParens`) `ConstantExpr`s. Is there any reason for this inconsistency ? 
> I would like to add `FullExpr` to the nodes skipped by `IgnoreParenImpCasts` 
> for consistency but I am worried about unexpected issues even though all 
> tests pass.


I don't think there was an explicit reason beyond "I didn't need to do it at 
the time". So probably just an oversight on my part. I don't know the code 
nearly as well as @rnk, so I could be wrong, but I think the existing tests 
should tell you if something went haywire if you skip `FullExpr`s.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57267



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

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1572
+  let Spellings = [Clang<"fortify_stdlib">];
+  let Args = [IntArgument<"Type">, IntArgument<"Flag">];
+  let Subjects = SubjectList<[Function]>;

Nothing in the docs describes these two arguments.



Comment at: clang/include/clang/Basic/AttrDocs.td:987
+
+Only a specific set of standard library functions are supported:
+  - memcpy

And we don't have Annex K versions for any of these, like memset_s()?


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

https://reviews.llvm.org/D57918



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

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

[Rong Xu via Phabricator via cfe-commits]

xur updated this revision to Diff 186023.
xur added a comment.

Updated the patch to sync with D54175 


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

https://reviews.llvm.org/D54176

Files:
  include/clang/Basic/CodeGenOptions.h
  include/clang/Driver/Options.td
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/Inputs/pgotestir.proftext
  test/CodeGen/Inputs/pgotestir_cs.proftext
  test/CodeGen/cspgo-instrumentation.c
  test/CodeGen/cspgo-instrumentation_lto.c
  test/CodeGen/cspgo-instrumentation_thinlto.c

Index: test/CodeGen/cspgo-instrumentation_thinlto.c
===
--- test/CodeGen/cspgo-instrumentation_thinlto.c
+++ test/CodeGen/cspgo-instrumentation_thinlto.c
@@ -0,0 +1,52 @@
+// Test if CSPGO instrumentation and use pass are invoked in thinlto.
+//
+// RUN: rm -rf %t && mkdir %t
+// RUN: llvm-profdata merge -o %t/noncs.profdata %S/Inputs/pgotestir.proftext
+//
+// Ensure Pass PGOInstrumentationGenPass is not invoked in PreLink.
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata -fprofile-instrument=csllvm %s -fprofile-instrument-path=default.profraw -flto=thin -mllvm -debug-pass=Structure -emit-llvm-bc -o %t/foo_fe.bc 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata -fprofile-instrument=csllvm %s -fprofile-instrument-path=default.profraw  -flto=thin -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm-bc -o %t/foo_fe_pm.bc 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE-NEWPM
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE: PGOInstrumentationUsePass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE: PGOInstrumentationGenCreateVarPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE-NOT: PGOInstrumentationGenPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE-NEWPM: Running pass: PGOInstrumentationUse
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE-NEWPM: Running pass: PGOInstrumentationGenCreateVar
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE-NEWPM-NOT: Running pass: PGOInstrumentationGen on
+//
+// RUN: llvm-lto -thinlto -o %t/foo %t/foo_fe.bc
+// RUN: llvm-lto -thinlto -o %t/foo_pm %t/foo_fe_pm.bc
+// Ensure Pass PGOInstrumentationGenPass is invoked in PostLink.
+// RUN: %clang_cc1 -O2 -x ir %t/foo_fe.bc -fthinlto-index=%t/foo.thinlto.bc -fprofile-instrument=csllvm -fprofile-instrument-path=default.profraw  -flto=thin -emit-llvm -mllvm -debug-pass=Structure -o - |& FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST
+// RUN: %clang_cc1 -O2 -x ir %t/foo_fe_pm.bc -fthinlto-index=%t/foo_pm.thinlto.bc -fexperimental-new-pass-manager -fdebug-pass-manager  -fprofile-instrument=csllvm -fprofile-instrument-path=default.profraw  -flto=thin -emit-llvm -o - |& FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NEWPM
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NOT: PGOInstrumentationUsePass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NOT: PGOInstrumentationGenCreateVarPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST: PGOInstrumentationGenPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NEWPM-NOT: Running pass: PGOInstrumentationUse
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NEWPM-NOT: Running pass: PGOInstrumentationGenCreateVar
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NEWPM: Running pass: PGOInstrumentationGen on
+//
+// RUN: llvm-profdata merge -o %t/cs.profdata %S/Inputs/pgotestir_cs.proftext
+//
+// Ensure Pass PGOInstrumentationUsePass is invoked Once in PreLink.
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/cs.profdata %s -flto=thin -mllvm -debug-pass=Structure -emit-llvm-bc -o %t/foo_fe.bc 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/cs.profdata %s -flto=thin -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm-bc -o %t/foo_fe_pm.bc 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-NEWPM
+// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE: PGOInstrumentationUsePass
+// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-NOT: PGOInstrumentationUsePass
+// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-NEWPM: Running pass: PGOInstrumentationUse
+// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-NEWPM-NOT: Running pass: PGOInstrumentationUse
+//
+// RUN: llvm-lto -thinlto -o %t/foo %t/foo_fe.bc
+// RUN: llvm-lto -thinlto -o %t/foo_pm %t/foo_fe_pm.bc
+// Ensure Pass PGOInstrumentationUSEPass is invoked in PostLink.
+// RUN: %clang_cc1 -O2 -x ir %t/foo_fe.bc -fthinlto-index=%t/foo.thinlto.bc -fprofile-instrument-use-path=%t/cs.profdata -flto=thin -emit-llvm -mllvm -debug-pass=Structure -o - 2>&1 | FileCheck %s 

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[John McCall via Phabricator via cfe-commits]

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

LGTM, thanks!




Comment at: clang/include/clang/Basic/AttrDocs.td:986
+  }
+  }];
+}

erik.pilkington wrote:
> rjmccall wrote:
> > Probably worth clarifying somewhere in here that only a specific set of 
> > stdlib functions are supported.  I don't know that it's important to spell 
> > that set out.
> I just copied a list in, there really aren't that many, so may as well just 
> be explicit.
WFM


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

https://reviews.llvm.org/D57918



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

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 186013.
erik.pilkington marked 5 inline comments as done.
erik.pilkington added a comment.

Address John's comments.


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

https://reviews.llvm.org/D57918

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Builtins.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Builtins.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/fortify-std-lib.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/fortify-std-lib.c

Index: clang/test/Sema/fortify-std-lib.c
===
--- /dev/null
+++ clang/test/Sema/fortify-std-lib.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only %s -verify
+
+typedef unsigned long size_t;
+
+__attribute__((fortify_stdlib(0, 0)))
+int not_anything_special(); // expected-error {{'fortify_stdlib' attribute applied to an unknown function}}
+
+__attribute__((fortify_stdlib(4, 0))) // expected-error {{'fortify_stdlib' attribute requires integer constant between 0 and 3 inclusive}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib())) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(1, 2, 3))) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(-1, 2))) // expected-error {{'fortify_stdlib' attribute requires a non-negative integral compile time constant expression}}
+int sprintf(char *, const char *, ...);
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -52,6 +52,7 @@
 // CHECK-NEXT: ExternalSourceSymbol ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum)
 // CHECK-NEXT: Flatten (SubjectMatchRule_function)
+// CHECK-NEXT: FortifyStdLib (SubjectMatchRule_function)
 // CHECK-NEXT: GNUInline (SubjectMatchRule_function)
 // CHECK-NEXT: Hot (SubjectMatchRule_function)
 // CHECK-NEXT: IBAction (SubjectMatchRule_objc_method_is_instance)
Index: clang/test/CodeGen/fortify-std-lib.c
===
--- /dev/null
+++ clang/test/CodeGen/fortify-std-lib.c
@@ -0,0 +1,220 @@
+// RUN: %clang_cc1   -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+
+#ifdef __cplusplus
+#define EXTERN extern "C"
+#else
+#define EXTERN
+#endif
+
+#define FSL(x,y) __attribute__((fortify_stdlib(x,y)))
+typedef unsigned long size_t;
+
+FSL(0, 0) EXTERN
+void *memcpy(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memcpy(void *dst, const void *src, size_t sz) {
+  memcpy(dst, src, sz);
+  // CHECK-LABEL: define void @call_memcpy
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memcpy_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memmove(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memmove(void *dst, const void *src, size_t sz) {
+  memmove(dst, src, sz);
+  // CHECK-LABEL: define void @call_memmove
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memmove_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memset(void *dst, int c, size_t sz);
+
+EXTERN
+void call_memset(void *dst, int c, size_t sz) {
+  memset(dst, c, sz);
+  // CHECK-LABEL: define void @call_memset
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memset_chk(i8* {{.*}}, i32 {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+char *stpcpy(char* dst, const char *src);
+
+EXTERN
+void call_stpcpy(char *dst, const char *src) {
+  stpcpy(dst, src);
+  // CHECK-LABEL: define void @call_stpcpy
+  // CHECK: [[REG:%[0-9]+]] = call i64 

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:986
+  }
+  }];
+}

rjmccall wrote:
> Probably worth clarifying somewhere in here that only a specific set of 
> stdlib functions are supported.  I don't know that it's important to spell 
> that set out.
I just copied a list in, there really aren't that many, so may as well just be 
explicit.


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

https://reviews.llvm.org/D57918



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

r353556 - Pass the base element type of an array type to the visit method instead

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Fri Feb  8 11:46:53 2019
New Revision: 353556

URL: http://llvm.org/viewvc/llvm-project?rev=353556=rev
Log:
Pass the base element type of an array type to the visit method instead
of the array type itself.

This fixes a bug found by inspection that was introduced in r353459. I
don't have a test case for this since we don't yet have types that would
make the containing C struct non-trivial to copy/move but wouldn't make
it non-trivial to default-initialize or destruct.

Modified:
cfe/trunk/lib/AST/Type.cpp

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=353556=353555=353556=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Fri Feb  8 11:46:53 2019
@@ -2258,7 +2258,7 @@ struct IsNonTrivialCopyMoveVisitor
 
   bool visitWithKind(QualType::PrimitiveCopyKind PCK, QualType QT) {
 if (const auto *AT = this->Ctx.getAsArrayType(QT))
-  return this->asDerived().visit(QualType(AT, 0));
+  return this->asDerived().visit(Ctx.getBaseElementType(AT));
 return Super::visitWithKind(PCK, QT);
   }
 


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

[PATCH] D57961: [X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching of MSVC behavior with #pragma pack.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353555: [X86] Add explicit alignment to 
__m128/__m128i/__m128d/etc. to allow matching… (authored by ctopper, committed 
by ).

Changed prior to commit:
  https://reviews.llvm.org/D57961?vs=185996=186015#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57961

Files:
  lib/Headers/avx512bwintrin.h
  lib/Headers/avx512fintrin.h
  lib/Headers/avx512vlbwintrin.h
  lib/Headers/avx512vlintrin.h
  lib/Headers/avxintrin.h
  lib/Headers/emmintrin.h
  lib/Headers/mmintrin.h
  lib/Headers/xmmintrin.h
  test/CodeGen/x86-vec-struct-packing.c

Index: test/CodeGen/x86-vec-struct-packing.c
===
--- test/CodeGen/x86-vec-struct-packing.c
+++ test/CodeGen/x86-vec-struct-packing.c
@@ -0,0 +1,227 @@
+// RUN: %clang_cc1 -ffreestanding -emit-llvm-only  -triple x86_64-windows-coff -fdump-record-layouts %s | FileCheck %s --check-prefix=CHECK-MS
+// RUN: %clang_cc1 -ffreestanding -emit-llvm-only  -triple x86_64-apple-darwin -fdump-record-layouts %s | FileCheck %s --check-prefix=CHECK-NOTMS
+#include 
+
+#pragma pack(1)
+
+struct s_m64 {
+  int a;
+  __m64 b;
+};
+typedef struct s_m64 m64;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m64) == 16) - 1];
+#else
+static int a1[(sizeof(m64) == 12) - 1];
+#endif
+
+struct s_m128 {
+  int a;
+  __m128 b;
+};
+typedef struct s_m128 m128;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128) == 32) - 1];
+#else
+static int a1[(sizeof(m128) == 20) - 1];
+#endif
+
+struct s_m128i {
+  int a;
+  __m128i b;
+};
+typedef struct s_m128i m128i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128i) == 32) - 1];
+#else
+static int a1[(sizeof(m128i) == 20) - 1];
+#endif
+
+struct s_m128d {
+  int a;
+  __m128d b;
+};
+typedef struct s_m128d m128d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128d) == 32) - 1];
+#else
+static int a1[(sizeof(m128d) == 20) - 1];
+#endif
+
+struct s_m256 {
+  int a;
+  __m256 b;
+};
+typedef struct s_m256 m256;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256) == 64) - 1];
+#else
+static int a1[(sizeof(m256) == 36) - 1];
+#endif
+
+struct s_m256i {
+  int a;
+  __m256i b;
+};
+typedef struct s_m256i m256i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256i) == 64) - 1];
+#else
+static int a1[(sizeof(m256i) == 36) - 1];
+#endif
+
+struct s_m256d {
+  int a;
+  __m256d b;
+};
+typedef struct s_m256d m256d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256d) == 64) - 1];
+#else
+static int a1[(sizeof(m256d) == 36) - 1];
+#endif
+
+struct s_m512 {
+  int a;
+  __m512 b;
+};
+typedef struct s_m512 m512;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512) == 128) - 1];
+#else
+static int a1[(sizeof(m512) == 68) - 1];
+#endif
+
+struct s_m512i {
+  int a;
+  __m512i b;
+};
+typedef struct s_m512i m512i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512i) == 128) - 1];
+#else
+static int a1[(sizeof(m512i) == 68) - 1];
+#endif
+
+struct s_m512d {
+  int a;
+  __m512d b;
+};
+typedef struct s_m512d m512d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512d) == 128) - 1];
+#else
+static int a1[(sizeof(m512d) == 68) - 1];
+#endif
+
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m64
+// CHECK-MS:  0 |   int a
+// CHECK-MS:  8 |   __m64 b
+// CHECK-MS:| [sizeof=16, align=8]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128 b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128i
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128i b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128d
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128d b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256 b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256i
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256i b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256d
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256d b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m512
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 64 |   __m512 b
+// CHECK-MS:| [sizeof=128, align=64]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m512i
+// 

r353555 - [X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching of MSVC behavior with #pragma pack.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Fri Feb  8 11:45:08 2019
New Revision: 353555

URL: http://llvm.org/viewvc/llvm-project?rev=353555=rev
Log:
[X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching 
of MSVC behavior with #pragma pack.

Summary:
With MSVC, #pragma pack is ignored when there is explicit alignment. This 
differs from gcc. Clang emulates this difference when compiling for Windows.

It appears that MSVC and its headers consider the __m128/__m128i/__m128d/etc. 
types to be explicitly aligned and ignores #pragma pack for them. Since we 
don't have explicit alignment on them in our headers, we don't match the MSVC 
behavior here.

This patch adds explicit alignment to match this behavior. I'm hoping this 
won't cause any problems when we're not emulating MSVC. But if someone knows of 
something that would be different we can swith to conditionally adding the 
alignment based on _MSC_VER.

I had to add explicitly unaligned types as well so we could use them in the 
loadu/storeu intrinsics which use __attribute__(__packed__). Using the now 
explicitly aligned types wouldn't produce align 1 accesses when targeting 
Windows.

Reviewers: rnk, erichkeane, spatel, RKSimon

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D57961

Added:
cfe/trunk/test/CodeGen/x86-vec-struct-packing.c
Modified:
cfe/trunk/lib/Headers/avx512bwintrin.h
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/lib/Headers/avx512vlbwintrin.h
cfe/trunk/lib/Headers/avx512vlintrin.h
cfe/trunk/lib/Headers/avxintrin.h
cfe/trunk/lib/Headers/emmintrin.h
cfe/trunk/lib/Headers/mmintrin.h
cfe/trunk/lib/Headers/xmmintrin.h

Modified: cfe/trunk/lib/Headers/avx512bwintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512bwintrin.h?rev=353555=353554=353555=diff
==
--- cfe/trunk/lib/Headers/avx512bwintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512bwintrin.h Fri Feb  8 11:45:08 2019
@@ -1751,7 +1751,7 @@ static __inline __m512i __DEFAULT_FN_ATT
 _mm512_loadu_epi16 (void const *__P)
 {
   struct __loadu_epi16 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   return ((struct __loadu_epi16*)__P)->__v;
 }
@@ -1777,7 +1777,7 @@ static __inline __m512i __DEFAULT_FN_ATT
 _mm512_loadu_epi8 (void const *__P)
 {
   struct __loadu_epi8 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   return ((struct __loadu_epi8*)__P)->__v;
 }
@@ -1803,7 +1803,7 @@ static __inline void __DEFAULT_FN_ATTRS5
 _mm512_storeu_epi16 (void *__P, __m512i __A)
 {
   struct __storeu_epi16 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   ((struct __storeu_epi16*)__P)->__v = __A;
 }
@@ -1820,7 +1820,7 @@ static __inline void __DEFAULT_FN_ATTRS5
 _mm512_storeu_epi8 (void *__P, __m512i __A)
 {
   struct __storeu_epi8 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   ((struct __storeu_epi8*)__P)->__v = __A;
 }

Modified: cfe/trunk/lib/Headers/avx512fintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512fintrin.h?rev=353555=353554=353555=diff
==
--- cfe/trunk/lib/Headers/avx512fintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512fintrin.h Fri Feb  8 11:45:08 2019
@@ -40,9 +40,13 @@ typedef unsigned short __v32hu __attribu
 typedef unsigned long long __v8du __attribute__((__vector_size__(64)));
 typedef unsigned int __v16su __attribute__((__vector_size__(64)));
 
-typedef float __m512 __attribute__((__vector_size__(64)));
-typedef double __m512d __attribute__((__vector_size__(64)));
-typedef long long __m512i __attribute__((__vector_size__(64)));
+typedef float __m512 __attribute__((__vector_size__(64), __aligned__(64)));
+typedef double __m512d __attribute__((__vector_size__(64), __aligned__(64)));
+typedef long long __m512i __attribute__((__vector_size__(64), 
__aligned__(64)));
+
+typedef float __m512_u __attribute__((__vector_size__(64), __aligned__(1)));
+typedef double __m512d_u __attribute__((__vector_size__(64), __aligned__(1)));
+typedef long long __m512i_u __attribute__((__vector_size__(64), 
__aligned__(1)));
 
 typedef unsigned char __mmask8;
 typedef unsigned short __mmask16;
@@ -4324,7 +4328,7 @@ static __inline __m512i __DEFAULT_FN_ATT
 _mm512_loadu_si512 (void const *__P)
 {
   struct __loadu_si512 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   return ((struct __loadu_si512*)__P)->__v;
 }
@@ -4333,7 +4337,7 @@ static __inline __m512i __DEFAULT_FN_ATT
 _mm512_loadu_epi32 (void const *__P)
 {
   struct __loadu_epi32 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   return ((struct __loadu_epi32*)__P)->__v;
 }
@@ -4360,7 +4364,7 @@ static __inline 

[clang-tools-extra] r353554 - [clang-tidy] Don't use assignment for value-initialized enums

[Malcolm Parsons via cfe-commits]

Author: malcolm.parsons
Date: Fri Feb  8 11:44:42 2019
New Revision: 353554

URL: http://llvm.org/viewvc/llvm-project?rev=353554=rev
Log:
[clang-tidy] Don't use assignment for value-initialized enums

Summary:
The modernize-use-default-member-init check crashes when trying to
create an assignment value for a value-initialized enum because it isn't a
BuiltinType.
An enum cannot be initialized by assigning 0 to it unless a cast is added.
It could be initialized with an enumerator with the value 0, but there might not
be one.
Avoid these issues by ignoring the UseAssignment setting for value-initialized
enums.

Fixes PR35050.

Reviewers: aaron.ballman, alexfh, JonasToth

Reviewed By: JonasToth

Subscribers: xazax.hun, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D57852

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-assignment.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp?rev=353554=353553=353554=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp 
Fri Feb  8 11:44:42 2019
@@ -255,17 +255,20 @@ void UseDefaultMemberInitCheck::checkDef
   CharSourceRange InitRange =
   CharSourceRange::getCharRange(LParenEnd, Init->getRParenLoc());
 
+  bool ValueInit = isa(Init->getInit());
+  bool CanAssign = UseAssignment && (!ValueInit || 
!Init->getInit()->getType()->isEnumeralType());
+
   auto Diag =
   diag(Field->getLocation(), "use default member initializer for %0")
   << Field
-  << FixItHint::CreateInsertion(FieldEnd, UseAssignment ? " = " : "{")
+  << FixItHint::CreateInsertion(FieldEnd, CanAssign ? " = " : "{")
   << FixItHint::CreateInsertionFromRange(FieldEnd, InitRange);
 
-  if (UseAssignment && isa(Init->getInit()))
+  if (CanAssign && ValueInit)
 Diag << FixItHint::CreateInsertion(
 FieldEnd, getValueOfValueInit(Init->getInit()->getType()));
 
-  if (!UseAssignment)
+  if (!CanAssign)
 Diag << FixItHint::CreateInsertion(FieldEnd, "}");
 
   Diag << FixItHint::CreateRemoval(Init->getSourceRange());

Modified: 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-assignment.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-assignment.cpp?rev=353554=353553=353554=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-assignment.cpp
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-assignment.cpp
 Fri Feb  8 11:44:42 2019
@@ -166,6 +166,14 @@ struct PositiveEnum {
   // CHECK-FIXES: Enum e = Foo;
 };
 
+struct PositiveValueEnum {
+  PositiveValueEnum() : e() {}
+  // CHECK-FIXES: PositiveValueEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer 
for 'e'
+  // CHECK-FIXES: Enum e{};
+};
+
 struct PositiveString {
   PositiveString() : s("foo") {}
   // CHECK-FIXES: PositiveString()  {}

Modified: 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp?rev=353554=353553=353554=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp 
Fri Feb  8 11:44:42 2019
@@ -165,6 +165,14 @@ struct PositiveEnum {
   // CHECK-FIXES: Enum e{Foo};
 };
 
+struct PositiveValueEnum {
+  PositiveValueEnum() : e() {}
+  // CHECK-FIXES: PositiveValueEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer 
for 'e'
+  // CHECK-FIXES: Enum e{};
+};
+
 struct PositiveString {
   PositiveString() : s("foo") {}
   // CHECK-FIXES: PositiveString()  {}


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

[PATCH] D57852: [clang-tidy] Don't use assignment for value-initialized enums

[Malcolm Parsons via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE353554: [clang-tidy] Dont use assignment for 
value-initialized enums (authored by malcolm.parsons, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57852?vs=185649=186014#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57852

Files:
  clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  test/clang-tidy/modernize-use-default-member-init-assignment.cpp
  test/clang-tidy/modernize-use-default-member-init.cpp


Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -255,17 +255,20 @@
   CharSourceRange InitRange =
   CharSourceRange::getCharRange(LParenEnd, Init->getRParenLoc());
 
+  bool ValueInit = isa(Init->getInit());
+  bool CanAssign = UseAssignment && (!ValueInit || 
!Init->getInit()->getType()->isEnumeralType());
+
   auto Diag =
   diag(Field->getLocation(), "use default member initializer for %0")
   << Field
-  << FixItHint::CreateInsertion(FieldEnd, UseAssignment ? " = " : "{")
+  << FixItHint::CreateInsertion(FieldEnd, CanAssign ? " = " : "{")
   << FixItHint::CreateInsertionFromRange(FieldEnd, InitRange);
 
-  if (UseAssignment && isa(Init->getInit()))
+  if (CanAssign && ValueInit)
 Diag << FixItHint::CreateInsertion(
 FieldEnd, getValueOfValueInit(Init->getInit()->getType()));
 
-  if (!UseAssignment)
+  if (!CanAssign)
 Diag << FixItHint::CreateInsertion(FieldEnd, "}");
 
   Diag << FixItHint::CreateRemoval(Init->getSourceRange());
Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -165,6 +165,14 @@
   // CHECK-FIXES: Enum e{Foo};
 };
 
+struct PositiveValueEnum {
+  PositiveValueEnum() : e() {}
+  // CHECK-FIXES: PositiveValueEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer 
for 'e'
+  // CHECK-FIXES: Enum e{};
+};
+
 struct PositiveString {
   PositiveString() : s("foo") {}
   // CHECK-FIXES: PositiveString()  {}
Index: test/clang-tidy/modernize-use-default-member-init-assignment.cpp
===
--- test/clang-tidy/modernize-use-default-member-init-assignment.cpp
+++ test/clang-tidy/modernize-use-default-member-init-assignment.cpp
@@ -166,6 +166,14 @@
   // CHECK-FIXES: Enum e = Foo;
 };
 
+struct PositiveValueEnum {
+  PositiveValueEnum() : e() {}
+  // CHECK-FIXES: PositiveValueEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer 
for 'e'
+  // CHECK-FIXES: Enum e{};
+};
+
 struct PositiveString {
   PositiveString() : s("foo") {}
   // CHECK-FIXES: PositiveString()  {}


Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -255,17 +255,20 @@
   CharSourceRange InitRange =
   CharSourceRange::getCharRange(LParenEnd, Init->getRParenLoc());
 
+  bool ValueInit = isa(Init->getInit());
+  bool CanAssign = UseAssignment && (!ValueInit || !Init->getInit()->getType()->isEnumeralType());
+
   auto Diag =
   diag(Field->getLocation(), "use default member initializer for %0")
   << Field
-  << FixItHint::CreateInsertion(FieldEnd, UseAssignment ? " = " : "{")
+  << FixItHint::CreateInsertion(FieldEnd, CanAssign ? " = " : "{")
   << FixItHint::CreateInsertionFromRange(FieldEnd, InitRange);
 
-  if (UseAssignment && isa(Init->getInit()))
+  if (CanAssign && ValueInit)
 Diag << FixItHint::CreateInsertion(
 FieldEnd, getValueOfValueInit(Init->getInit()->getType()));
 
-  if (!UseAssignment)
+  if (!CanAssign)
 Diag << FixItHint::CreateInsertion(FieldEnd, "}");
 
   Diag << FixItHint::CreateRemoval(Init->getSourceRange());
Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -165,6 +165,14 @@
   // CHECK-FIXES: Enum e{Foo};
 };
 
+struct PositiveValueEnum {
+  PositiveValueEnum() : e() {}
+  // CHECK-FIXES: PositiveValueEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer for 'e'
+  // CHECK-FIXES: Enum e{};
+};
+
 struct PositiveString {
   PositiveString() : s("foo") {}
   // CHECK-FIXES: PositiveString()  {}
Index: 

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

You'll also need to update `TreeTransform::TransformCompoundStmt`. (And please 
add some tests for template instantiation.)


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

https://reviews.llvm.org/D57086



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

[PATCH] D57966: [clang-tidy] add camelBackOrCase casing style to readability-identifier-naming to support change to variable naming policy (if adopted)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: michaelplatings, aaron.ballman, JonasToth, 
hokein.
MyDeveloperDay added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

Introduction of a new variable policy (as outlined in D57896: Variable names 
rule ) could lead to clang tidy complaining 
about 1000's of variables

Consider add a new casing style to `readability-identifier-naming` to support 
allowing either `camelBack` and `camelCase` styles which could be applied to 
the VariableCase option in the LLVM .clang-tidy files

Users wishing to convert their files to new policy would simply change this 
setting to `camelBack` and run with -fix

Depends on D57896 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57966

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tidy/readability/IdentifierNamingCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/readability-identifier-naming.rst
  test/clang-tidy/readability-identifier-naming-camelback-or-case.cpp

Index: test/clang-tidy/readability-identifier-naming-camelback-or-case.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-identifier-naming-camelback-or-case.cpp
@@ -0,0 +1,117 @@
+// Remove UNSUPPORTED for powerpc64le when the problem introduced by
+// r288563 is resolved.
+// UNSUPPORTED: powerpc64le
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.AbstractClassCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ClassConstantCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ClassMethodCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ConstantCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.ConstexprFunctionCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.ConstexprMethodCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.ConstexprVariableCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.EnumCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.EnumConstantCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.GlobalConstantCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.InlineNamespaceCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.LocalConstantCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.LocalVariableCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.MemberCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.PublicMemberCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.MethodCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.NamespaceCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ConstantParameterCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ParameterPackCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.PureFunctionCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.PureMethodCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.StaticConstantCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.StaticVariableCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.StructCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.TemplateParameterCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.TemplateTemplateParameterCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.TemplateUsingCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.TypeTemplateParameterCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: 

[PATCH] D57965: Clean up ObjCPropertyDecl printing

[David Goldman via Phabricator via cfe-commits]

dgoldman created this revision.
Herald added subscribers: cfe-commits, jfb, arphaman.
Herald added a project: clang.

- `@property(attr, attr2)` instead of `@property ( attr,attr2 )`.
- Change priority of attributes (see code/comments inline).
- Support for printing weak and unsafe_unretained attributes.


Repository:
  rC Clang

https://reviews.llvm.org/D57965

Files:
  lib/AST/DeclPrinter.cpp
  test/Index/comment-objc-decls.m
  test/Index/comment-unqualified-objc-pointer.m
  test/PCH/chain-remap-types.m

Index: test/PCH/chain-remap-types.m
===
--- test/PCH/chain-remap-types.m
+++ test/PCH/chain-remap-types.m
@@ -6,7 +6,7 @@
 
 // CHECK: @class X;
 // CHECK: struct Y 
-// CHECK: @property ( assign,readwrite,atomic ) X * prop
+// CHECK: @property(atomic, assign, unsafe_unretained, readwrite) X * prop
 // CHECK: void h(X *);
 // CHECK: @interface X(Blah)
 // CHECK: void g(X *);
Index: test/Index/comment-unqualified-objc-pointer.m
===
--- test/Index/comment-unqualified-objc-pointer.m
+++ test/Index/comment-unqualified-objc-pointer.m
@@ -19,7 +19,7 @@
 
 //! This is a property to get the Name.
 @property (copy) NSString *Name;
-// CHECK: @property(readwrite, copy, atomic) NSString *Name;
+// CHECK: @property(atomic, copy, readwrite) NSString *Name;
 @end
 
 @implementation NSMutableArray
Index: test/Index/comment-objc-decls.m
===
--- test/Index/comment-objc-decls.m
+++ test/Index/comment-objc-decls.m
@@ -32,7 +32,7 @@
 @end
 // CHECK: @protocol MyProto\n@end
 // CHECK: - (unsigned int)MethodMyProto:(nullable id)anObject inRange:(unsigned int)range;
-// CHECK: @optional\n@property(readwrite, copy, atomic, nonnull) id PropertyMyProto;
+// CHECK: @optional\n@property(atomic, copy, readwrite, nonnull) id PropertyMyProto;
 // CHECK: + (id)ClassMethodMyProto;
 
 /**
@@ -77,7 +77,7 @@
 // CHECK: id IvarMyClass
 // CHECK: - (id)MethodMyClass;
 // CHECK: + (id)ClassMethodMyClass;
-// CHECK: @property(readwrite, copy, atomic) id PropertyMyClass;@property(atomic, copy, readwrite) id PropertyMyClass;@interface MyClass (Category)\n@end
 // CHECK: - (void)MethodMyClassCategory;
-// CHECK: @property(readwrite, copy, atomic) id PropertyMyClassCategory;
+// CHECK: @property(atomic, copy, readwrite) id PropertyMyClassCategory;
 // CHECK: - (id)PropertyMyClassCategory;
 // CHECK: - (void)setPropertyMyClassCategory:(id)arg;
 
Index: lib/AST/DeclPrinter.cpp
===
--- lib/AST/DeclPrinter.cpp
+++ lib/AST/DeclPrinter.cpp
@@ -1389,6 +1389,13 @@
 
 /// PrintObjCPropertyDecl - print a property declaration.
 ///
+/// Print attributes in the following order:
+/// - class
+/// - nonatomic | atomic
+/// - assign | retain | strong | copy | weak | unsafe_unretained
+/// - readwrite | readonly
+/// - getter & setter
+/// - nullability
 void DeclPrinter::VisitObjCPropertyDecl(ObjCPropertyDecl *PDecl) {
   if (PDecl->getPropertyImplementation() == ObjCPropertyDecl::Required)
 Out << "@required\n";
@@ -1400,58 +1407,69 @@
   Out << "@property";
   if (PDecl->getPropertyAttributes() != ObjCPropertyDecl::OBJC_PR_noattr) {
 bool first = true;
-Out << " (";
-if (PDecl->getPropertyAttributes() &
-ObjCPropertyDecl::OBJC_PR_readonly) {
-  Out << (first ? ' ' : ',') << "readonly";
+Out << "(";
+if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_class) {
+  Out << (first ? "" : ", ") << "class";
   first = false;
 }
 
-if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_getter) {
-  Out << (first ? ' ' : ',') << "getter = ";
-  PDecl->getGetterName().print(Out);
+if (PDecl->getPropertyAttributes() &
+ObjCPropertyDecl::OBJC_PR_nonatomic) {
+  Out << (first ? "" : ", ") << "nonatomic";
   first = false;
 }
-if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_setter) {
-  Out << (first ? ' ' : ',') << "setter = ";
-  PDecl->getSetterName().print(Out);
+if (PDecl->getPropertyAttributes() &
+ObjCPropertyDecl::OBJC_PR_atomic) {
+  Out << (first ? "" : ", ") << "atomic";
   first = false;
 }
 
 if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_assign) {
-  Out << (first ? ' ' : ',') << "assign";
-  first = false;
-}
-
-if (PDecl->getPropertyAttributes() &
-ObjCPropertyDecl::OBJC_PR_readwrite) {
-  Out << (first ? ' ' : ',') << "readwrite";
+  Out << (first ? "" : ", ") << "assign";
   first = false;
 }
-
 if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_retain) {
-  Out << (first ? ' ' : ',') << "retain";
+  Out << (first ? "" : ", ") << "retain";
   first = false;
 }
 
 if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_strong) {
-  Out << (first ? ' 

[PATCH] D54978: Move the SMT API to LLVM

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

> There is at least one other conflicting commit rL353465 
>  on top of this code already.

Sorry about that. I think it would be fine to revert both or to replay the 
other commit on top of reverted version of this one.

@mikhail.ramalho could you take a look?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Brian Rzycki via Phabricator via cfe-commits]

brzycki added a comment.

In D54978#1390698 , @thakis wrote:

> Thanks for the analysis. I think it's fine if you revert, given that.


I'm running in to conflict dependency issues when attempting to revert rL353373 
. There is at least one other conflicting 
commit rL353465  on top of this code already.

I don't feel comfortable reverting 2+ patches in a sub-section of the code I 
know little about on a Friday afternoon. :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D57961: [X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching of MSVC behavior with #pragma pack.

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a subscriber: rjmccall.
rnk added a comment.

Sounds good.

This change reminded me of D46042 , 
https://crbug.com/849251, and rL333791 , 
which I had to revert, and I don't think it relanded. I think your change to 
add the *_u typedefs fixes the issues encountered there, and perhaps @rjmccall 
will be able to reland his change after this lands.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57961



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

Re: r328173 - Improve -Winfinite-recursion

[Steven Wu via cfe-commits]

Hi Robert

I ping'ed this commit on Phabricator with an example of false positive 
introduced by this patch. Can you take a look?

Thanks

Steven

> On Mar 21, 2018, at 8:16 PM, Robert Widmann via cfe-commits 
>  wrote:
> 
> Author: codafi
> Date: Wed Mar 21 20:16:23 2018
> New Revision: 328173
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=328173=rev
> Log:
> Improve -Winfinite-recursion
> 
> Summary: Rewrites -Winfinite-recursion to remove the state dictionary and 
> explore paths in loops - especially infinite loops.  The new check now 
> detects recursion in loop bodies dominated by a recursive call.
> 
> Reviewers: rsmith, rtrieu
> 
> Reviewed By: rtrieu
> 
> Subscribers: lebedev.ri, cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D43737
> 
> Modified:
>cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
>cfe/trunk/test/SemaCXX/warn-infinite-recursion.cpp
> 
> Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=328173=328172=328173=diff
> ==
> --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
> +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Wed Mar 21 20:16:23 2018
> @@ -200,60 +200,41 @@ static bool hasRecursiveCallInPath(const
>   return false;
> }
> 
> -// All blocks are in one of three states.  States are ordered so that blocks
> -// can only move to higher states.
> -enum RecursiveState {
> -  FoundNoPath,
> -  FoundPath,
> -  FoundPathWithNoRecursiveCall
> -};
> -
> -// Returns true if there exists a path to the exit block and every path
> -// to the exit block passes through a call to FD.
> +// Returns true if every path from the entry block passes through a call to 
> FD.
> static bool checkForRecursiveFunctionCall(const FunctionDecl *FD, CFG *cfg) {
> +  llvm::SmallPtrSet Visited;
> +  llvm::SmallVector WorkList;
> +  // Keep track of whether we found at least one recursive path.
> +  bool foundRecursion = false;
> 
>   const unsigned ExitID = cfg->getExit().getBlockID();
> 
> -  // Mark all nodes as FoundNoPath, then set the status of the entry block.
> -  SmallVector States(cfg->getNumBlockIDs(), FoundNoPath);
> -  States[cfg->getEntry().getBlockID()] = FoundPathWithNoRecursiveCall;
> -
> -  // Make the processing stack and seed it with the entry block.
> -  SmallVector Stack;
> -  Stack.push_back(>getEntry());
> -
> -  while (!Stack.empty()) {
> -CFGBlock *CurBlock = Stack.back();
> -Stack.pop_back();
> -
> -unsigned ID = CurBlock->getBlockID();
> -RecursiveState CurState = States[ID];
> -
> -if (CurState == FoundPathWithNoRecursiveCall) {
> -  // Found a path to the exit node without a recursive call.
> -  if (ExitID == ID)
> -return false;
> -
> -  // Only change state if the block has a recursive call.
> -  if (hasRecursiveCallInPath(FD, *CurBlock))
> -CurState = FoundPath;
> -}
> +  // Seed the work list with the entry block.
> +  WorkList.push_back(>getEntry());
> 
> -// Loop over successor blocks and add them to the Stack if their state
> -// changes.
> -for (auto I = CurBlock->succ_begin(), E = CurBlock->succ_end(); I != E; 
> ++I)
> -  if (*I) {
> -unsigned next_ID = (*I)->getBlockID();
> -if (States[next_ID] < CurState) {
> -  States[next_ID] = CurState;
> -  Stack.push_back(*I);
> +  while (!WorkList.empty()) {
> +CFGBlock *Block = WorkList.pop_back_val();
> +
> +for (auto I = Block->succ_begin(), E = Block->succ_end(); I != E; ++I) {
> +  if (CFGBlock *SuccBlock = *I) {
> +if (!Visited.insert(SuccBlock).second)
> +  continue;
> +
> +// Found a path to the exit node without a recursive call.
> +if (ExitID == SuccBlock->getBlockID())
> +  return false;
> +
> +// If the successor block contains a recursive call, end analysis 
> there.
> +if (hasRecursiveCallInPath(FD, *SuccBlock)) {
> +  foundRecursion = true;
> +  continue;
> }
> +
> +WorkList.push_back(SuccBlock);
>   }
> +}
>   }
> -
> -  // Return true if the exit node is reachable, and only reachable through
> -  // a recursive call.
> -  return States[ExitID] == FoundPath;
> +  return foundRecursion;
> }
> 
> static void checkRecursiveFunction(Sema , const FunctionDecl *FD,
> @@ -269,10 +250,6 @@ static void checkRecursiveFunction(Sema
>   CFG *cfg = AC.getCFG();
>   if (!cfg) return;
> 
> -  // If the exit block is unreachable, skip processing the function.
> -  if (cfg->getExit().pred_empty())
> -return;
> -
>   // Emit diagnostic if a recursive function call is detected for all paths.
>   if (checkForRecursiveFunctionCall(FD, cfg))
> S.Diag(Body->getLocStart(), diag::warn_infinite_recursive_function);
> 
> Modified: cfe/trunk/test/SemaCXX/warn-infinite-recursion.cpp
> URL: 
>