r342801 - update the links to use https

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Sat Sep 22 00:39:44 2018
New Revision: 342801

URL: http://llvm.org/viewvc/llvm-project?rev=342801&view=rev
Log:
update the links to use https

Modified:
cfe/trunk/www/get_started.html

Modified: cfe/trunk/www/get_started.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/get_started.html?rev=342801&r1=342800&r2=342801&view=diff
==
--- cfe/trunk/www/get_started.html (original)
+++ cfe/trunk/www/get_started.html Sat Sep 22 00:39:44 2018
@@ -19,18 +19,18 @@
 options.  This should get you up and running with the minimum of muss and fuss.
 If you like what you see, please consider getting
 involved with the Clang community.  If you run into problems, please file
-bugs in http://llvm.org/bugs/";>LLVM Bugzilla.
+bugs in https://bugs.llvm.org/";>LLVM Bugzilla.
 
 Release Clang Versions
 
-Clang is released as part of regular LLVM releases. You can download the 
release versions from http://llvm.org/releases/";>http://llvm.org/releases/.
+Clang is released as part of regular LLVM releases. You can download the 
release versions from https://llvm.org/releases/";>https://llvm.org/releases/.
 Clang is also provided in all major BSD or GNU/Linux distributions as part 
of their respective packaging systems. From Xcode 4.2, Clang is the default 
compiler for Mac OS X.
 
 Building Clang and Working with the Code
 
 On Unix-like Systems
 
-Note: as an experimental setup, you can use a single checkout with 
all the projects, and an easy CMake invocation, see the LLVM Doc "http://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo";>For
 developers to work with a git monorepo"
+Note: as an experimental setup, you can use a single checkout with 
all the projects, and an easy CMake invocation, see the LLVM Doc "https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo";>For
 developers to work with a git monorepo"
 
 If you would like to check out and build Clang, the current procedure is as
 follows:
@@ -39,7 +39,7 @@ follows:
   Get the required tools.
   
 See
-  http://llvm.org/docs/GettingStarted.html#requirements";>
+  https://llvm.org/docs/GettingStarted.html#requirements";>
   Getting Started with the LLVM System - Requirements.
 Note also that Python is needed for running the test suite.
   Get it at: http://www.python.org/download";>
@@ -98,7 +98,7 @@ follows:
 CMake allows you to generate project files for several IDEs: Xcode,
 Eclipse CDT4, CodeBlocks, Qt-Creator (use the CodeBlocks generator),
 KDevelop3. For more details see
-http://llvm.org/docs/CMake.html";>Building LLVM with CMake
+https://llvm.org/docs/CMake.html";>Building LLVM with CMake
 page.
   
   
@@ -200,7 +200,7 @@ Visual Studio:
  32-bit toolset. If you are developing on a 64-bit version of Windows and
  want to use the 64-bit toolset, pass the ``-Thost=x64`` flag when
  generating the Visual Studio solution. This requires CMake 3.8.0 or 
later.
-See the http://www.llvm.org/docs/CMake.html";>LLVM CMake 
guide for
+See the https://www.llvm.org/docs/CMake.html";>LLVM CMake 
guide for
 more information on other configuration options for CMake.
 The above, if successful, will have created an LLVM.sln file in the
build directory.


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

r342802 - use the current url for bugzilla

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Sat Sep 22 00:41:09 2018
New Revision: 342802

URL: http://llvm.org/viewvc/llvm-project?rev=342802&view=rev
Log:
use the current url for bugzilla

Modified:
cfe/trunk/www/cxx_status.html
cfe/trunk/www/get_involved.html
cfe/trunk/www/menu.html.incl

Modified: cfe/trunk/www/cxx_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=342802&r1=342801&r2=342802&view=diff
==
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Sat Sep 22 00:41:09 2018
@@ -43,7 +43,7 @@ as they become available.
 Specifications that will help drive the future of the C++ programming
 language.
 
-The http://llvm.org/bugs/";>LLVM bug tracker contains Clang
+The https://bugs.llvm.org/";>LLVM bug tracker contains Clang
 C++ components that track known bugs with Clang's language conformance in
 each language mode.
 

Modified: cfe/trunk/www/get_involved.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/get_involved.html?rev=342802&r1=342801&r2=342802&view=diff
==
--- cfe/trunk/www/get_involved.html (original)
+++ cfe/trunk/www/get_involved.html Sat Sep 22 00:41:09 2018
@@ -65,7 +65,7 @@ Clang developers, contributors and the s
 
 If you're looking for something to work on, check out our Open Projects page or look through the http://llvm.org/bugs/";>Bugzilla bug database.
+href="https://bugs.llvm.org/";>Bugzilla bug database.
 
 Contributing Extensions to Clang
 

Modified: cfe/trunk/www/menu.html.incl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/menu.html.incl?rev=342802&r1=342801&r2=342802&view=diff
==
--- cfe/trunk/www/menu.html.incl (original)
+++ cfe/trunk/www/menu.html.incl Sat Sep 22 00:41:09 2018
@@ -36,7 +36,7 @@
 http://lists.llvm.org/mailman/listinfo/cfe-users";>cfe-users 
List
 http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev List
 http://lists.llvm.org/mailman/listinfo/cfe-commits";>cfe-commits 
List
-http://llvm.org/bugs/";>Bug Reports
+https://bugs.llvm.org/";>Bug Reports
 http://planet.clang.org/";>Planet Clang
 IRC: irc.oftc.net#llvm
   


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

[PATCH] D52390: [analyzer] StackSizeChecker

[Máté Tóth via Phabricator via cfe-commits]

mate1214 created this revision.
mate1214 added reviewers: NoQ, george.karpenkov, dcoughlin.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, a.sidorin, 
szepet, mgorny.

Add StackSizeChecker to StaticAnalyzer

  
  This checker can be used to warn about potential stack overflows
  or be used to estimate the size of used stack space.
  Both a clang static analyzer and a clang-tidy version exist.
  The clang-tidy version references the StackUsageMeasuringVisitor
  class included in this commit so it can only come after this.
  Both versions work by examining function bodies via the AST,
  the static analyzer version can follow and summarize the space
  used by call chains, while the calng-tidy version can analyze
  function bodies faster, one by one.
  This version emits warnings if the estimated stack size surpasses
  the StackUsageLimit parameter, which is measured in bytes and
  defaults to 10. Setting it to some small value can be used
  to turn the checker into a statistical tool since the calculated
  values are included in the warning messages.
  
  The calculations used for the size estimations do not take into
  account potential compile-time optimizations and contain some
  minor simplifications. Only the information stored in the AST
  is used and variable lifetime rules are respected. The
  calculations in a particular function body are carried out by the
  StackUsageMeasuringVisitor class, which gives a composite result
  about a piece of the AST containing the maximal estimated space,
  the space that remain in use after the execution of those lines
  and flags about encountered variable length arrays or special
  nodes that satisfy a given predicate (e.g.: templates).
  
  The current version takes no special actions upon encountering
  variable length arrays, the tidy version has a simple extra logic
  for them. The tests are divided between the two versions, the
  static analyzer ones are about the ability to calculate the size
  of a complete call stack and the clang-tidy ones focus on
  particular statements and expression types, this is referenced
  in one of the comments of the test files for those who want
  to use the test cases to understand the checker a little bit more.
  
  The code favors readability and maintainability over performance
  in some places.


Repository:
  rC Clang

https://reviews.llvm.org/D52390

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp
  lib/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.cpp
  test/Analysis/stack-size-callchain.cpp
  test/Analysis/stack-size.cpp

Index: test/Analysis/stack-size.cpp
===
--- /dev/null
+++ test/Analysis/stack-size.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core.StackSize -analyzer-config alpha.core.StackSize:StackUsageLimit=1 -std=c++11 -triple x86_64-unknown-linux-gnu -verify %s
+
+// In this test an integer is 4 bytes, a bool is 1 byte.
+// The analysis uses the same method of calculation as the misc-stack-size
+// checker in clang-tidy.
+// (see clang-tools-extra/test/clang-tidy/misc-stack-size.cpp for examples)
+
+// If a call happens in a function body, the stack space used by the function
+// will be registered as the space used up until that point. Each time the
+// execution of a function body is completely finished, the maximal amount
+// of space used is estimated and checked against the limit.
+
+// This function gets called multiple times. The space used is varying each time.
+int f(int X) { return X; } // expected-warning {{Estimated stack usage is 8 bytes}}
+   // expected-warning@-1 {{Estimated stack usage is 12 bytes}}
+
+// This function illustrates the partial summation ability.
+// Before each call, the stack usage at that point is registered,
+// so it can be added to the statistics of the called function.
+void linear() {
+  int X = 1;
+  f(X); // expected-warning {{Estimated stack usage is 4 bytes}}
+  int Y = 2;
+  f(Y); // expected-warning {{Estimated stack usage is 8 bytes}}
+} // expected-warning {{Estimated stack usage is 20 bytes}}
+
+// This function shows the ability of this checker to calculate with recursion
+// far as the analyzer follows it.
+void recursion(int N) {
+  int X;
+
+  if (N <= 0)
+return; // expected-warning {{Estimated stack usage is 44 bytes}}
+
+  recursion(N - 1); // expected-warning {{Estimated stack usage is 8 bytes}}
+// expected-warning@-1 {{Estimated stack usage is 16 bytes}}
+// expected-warning@-2 {{Estimated stack usage is 24 bytes}}
+// expected-warning@-3 {{Estimated stack usage is 20 bytes}}
+// expected-warning@-4 {{Estimated stack usage is 28 bytes}}
+  

[PATCH] D52334: [clang-tidy] Build it even without static analyzer

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

@JonasToth Sorry, I don't know what's unclear. I'm so surprised by your 
question that I think maybe I'm missing something. I thought the commit message 
and the patch itself are clear. Am I missing something?

Currently you can only build clang-tidy if you build the static analyzer.

This patch fixes that so that even if you do not build the static analyzer, you 
can build clang-tidy. That is the purpose of this patch, as in the title.

Am I missing something? Or did I misunderstand your comment?




Comment at: clang-tidy/tool/CMakeLists.txt:32
   clangTidyModernizeModule
-  clangTidyMPIModule
   clangTidyObjCModule

JonasToth wrote:
> Is the MPI module remove from the list of available checks (`clang-tidy 
> -list-checks`) with that?
Yes.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52334



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

[PATCH] D52334: [clang-tidy] Build it even without static analyzer

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

`#ifdef` hell is usually messy and is a source of problems.
May i ask what is the motivation for this change?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52334



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

[PATCH] D52390: [analyzer] StackSizeChecker

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

Hi!

Always great to see a new checker! I've started working in this project little 
over half a year ago, so I don't claim to be an expert, read my remarks as 
such! It'll be some time before I go through the entire code, but so far here 
are the things that caught my eye.

I think you should move the checker related files to a new folder, maybe 
`StackOverflow/`?




Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:143-146
+def StackSizeChecker : Checker<"StackSize">,
+  HelpText<"Warn if estimated stack usage exceeds the StackUsageLimit 
parameter (measured in bytes, defaults to 10)">,
+  DescFile<"StackSizeChecker.cpp">;
+

Alphabetical sorting?
(get it? //alpha//betical? I'm sorry.)



Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:16
+//===--===//
+
+#include "llvm/ADT/DenseMap.h"

Missing header guard.



Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:25
+#include 
+#include 
+

You included `map` but I don't see `std::map` anywhere?



Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:28
+namespace clang {
+namespace stack {
+

Use `ento` after `clang`. Btw, does anybody know what `ento` refers to? Never 
got to know it :D
```
namespace clang {
namespace ento {
namespace stack {



Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:57-60
+  explicit StackUsageMeasuringVisitor(ASTContext &Context)
+  : Context(Context), ContinueTraversing(true), ExitFlag(false),
+HardExit(false), ForceTemporariesToConstant(false),
+ExitPredicate(nullptr) {}

Since this is the only constructor of this visitor, I think C++11 in-class 
member initialization would be nicer.



Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:62
+
+  Usage collected() const;
+

It isn't obvious to me what this function does.



Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:64
+
+  // The visitor should traver post-order, because each node needs
+  // precalculated information in their children.

travel



Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:94
+private:
+  void clear();
+  bool hasUsageStored(const Stmt *S) const {

Are you sure this function belongs in a visitor?



Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:143-144
+
+} // stack
+} // clang

`// end of namespace .*`



Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:19
+#include "clang/AST/StmtCXX.h"
+#include "clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"

I bet you can't just `#include` this without getting some nasty buildbut errors 
down the line, just `#include "StackUsageMeasuringVisitor.h"`.



Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:25
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include 
+

Don't include standard stream libraries: 
https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden



Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:34
+  int Used;
+  bool operator==(const StackInt &Other) const { return Used == Other.Used; }
+  void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(Used); }

If you have a conversion operator to int, do you need `operator==`? 



Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:41
+: public Checker {
+  mutable std::unique_ptr StackOverflowBugType;
+

I think you don't need to make this `mutable` anymore, you can just initialize 
it in the constructor.



Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:68-77
+int length(const llvm::ImmutableList& L) {
+  int N = 0;
+  auto I = L.begin();
+  auto E = L.end();
+  while(I != E){
+++N;
+++I;

```
size_t length(const llvm::ImmutableList &L) {
  int Length = 0;
  for (auto It = L.begin(), E = L.end(); It != E; ++It)
++Length;
  return Length;
}



Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:78
+}
+}
+

`// end of anonymous namespace`



Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:80
+
+REGISTER_LIST_WITH_PROGRAMSTATE(StackSizeList, StackInt)
+

Would look better right below the definition of `StackInt`.



Comment at: lib/StaticAnalyzer/Checke

[PATCH] D52334: [clang-tidy] Build it even without static analyzer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D52334#1242811, @steveire wrote:

> @JonasToth Sorry, I don't know what's unclear. I'm so surprised by your 
> question that I think maybe I'm missing something. I thought the commit 
> message and the patch itself are clear. Am I missing something?
>
> Currently you can only build clang-tidy if you build the static analyzer.
>
> This patch fixes that so that even if you do not build the static analyzer, 
> you can build clang-tidy. That is the purpose of this patch, as in the title.
>
> Am I missing something? Or did I misunderstand your comment?


Sorry for formulating it unclear. I was curious if you could currently break 
the clang-tidy builds if you deactivate building CSA and that it might result 
in some dangling stuff or build errors.
But that clang-tidy is deactivated totally makes that impossible :)

I agree with @lebedev.ri that `#ifdef` in code might be a problem because its 
not constantly checked if the build still works if the analyzer is turned off 
and you might get bitrot.
Some of the clang-tidy stuff relies on `Analysis/*` from clang as well, e.g. 
the CFG class. Is this still included in builds with CSA off?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52334



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

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:156
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+  AST = tooling::buildASTFromCode(

shuaiwang wrote:
> JonasToth wrote:
> > JonasToth wrote:
> > > I feel that there a multiple tests missing:
> > > 
> > > - multiple levels of pointers `int ***`, `int * const *`
> > > - pointers to references `int &*`
> > > - references to pointers `int *&`
> > > - ensure that having a const pointer does no influence the pointee 
> > > analysis `int * const p = &i; *p = 42;`
> > > - a class with `operator*` + `operator->` const/non-const and the 
> > > analysis for pointers to that class
> > > - pointer returned from a function
> > > - non-const reference returned 
> > > ```
> > > int& foo(int *p) {
> > >   return *p;
> > > }
> > > ```
> > > 
> > for the multi-level pointer mutation: it would be enough to test, that the 
> > second layer is analyzed properly, and that the `int * >const< *` would be 
> > detected.
> Added except for:
> - Anything that requires following a dereference, we need 
> `findPointeeDerefMutation` for that.
> - Pointer to a class with `operator*` + `operator->`, I think those two 
> operators doesn't matter, there's no way to accidentally invoke them from a 
> pointer.
> 
But we want to analyze smart pointers in the future as well, not? It would be 
good to already prepare that in the testing department.
Or would the nature of `operator*` already make that happen magically?


Repository:
  rC Clang

https://reviews.llvm.org/D52219



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

[PATCH] D52334: [clang-tidy] Build it even without static analyzer

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

> But that clang-tidy is deactivated totally makes that impossible :)

Yes. That should be clear by reading the patch.

> Some of the clang-tidy stuff relies on Analysis/* from clang as well, e.g. 
> the CFG class. Is this still included in builds with CSA off?

The `Analysis` includes are ifdef'd out in the patch. Have you read the patch?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52334



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

[PATCH] D52334: [clang-tidy] Build it even without static analyzer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

>> Some of the clang-tidy stuff relies on Analysis/* from clang as well, e.g. 
>> the CFG class. Is this still included in builds with CSA off?
> 
> The `Analysis` includes are ifdef'd out in the patch. Have you read the patch?

Yes I did read the patch. `bugprone-use-after-move` utilizes `CFG`, there is no 
ifdefing in `bugprone` module. Thats why I am curious if the `CFG` is still 
part of a clang-build if the CSA is deactivated.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52334



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

[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check

[Wojtek Gumuła via Phabricator via cfe-commits]

wgml marked 6 inline comments as done.
wgml added inline comments.



Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:52
+const NamespaceContextVec &Namespaces) {
+  std::ostringstream Result;
+  bool First = true;

aaron.ballman wrote:
> wgml wrote:
> > aaron.ballman wrote:
> > > Can this be rewritten with `llvm::for_each()` and a `Twine` so that we 
> > > don't have to use `ostringstream` (which is a big hammer for this).
> > The main advantage of `stringstream` was that it is concise.
> > 
> > I don't think I can effectively use `Twine` to build a result in a loop. If 
> > I'm wrong, correct me, please.
> > 
> > I reworked `concatNamespaces` to use `SmallString` with another educated 
> > guess of `40` for capacity.
> The `Twine` idea I was thinking of is not too far off from what you have with 
> `SmallString`, but perhaps is too clever:
> ```
> return std::accumulate(Namespaces.begin(), Namespaces.end(), llvm::Twine(), 
> [](llvm::Twine Ret, const NamespaceDecl *ND) {
>   return Ret + "::" + ND->getName();
> }).str();
> ```
Yeah, I tried that, but Twine has it's `operator=` deleted.



Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:31
+static bool singleNamedNamespaceChild(const NamespaceDecl &ND) {
+  const NamespaceDecl::decl_range Decls = ND.decls();
+  if (std::distance(Decls.begin(), Decls.end()) != 1)

aaron.ballman wrote:
> We usually only const-qualify local declarations when they're 
> pointers/references, so you can drop the `const` here (and in several other 
> places). It's not a hard and fast rule, but the clutter is not useful in such 
> small functions.
From my perspective, `const` is a easy way of declaring that my intention is 
only to name given declaration only for reading and to improve code 
readability. 



Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:50
+
+auto ConcatNestedNamespacesCheck::concatNamespaces() -> NamespaceString {
+  NamespaceString Result("namespace ");

aaron.ballman wrote:
> I'm not keen on the trailing return type used here. It's reasonable, but 
> clever in ways we don't use elsewhere.
Ok. Wanted to avoid line break and the...

```
ConcatNestedNamespacesCheck::NamespaceString
ConcatNestedNamespacesCheck::concatNamespaces() {...}
```

but I'll adjust.


https://reviews.llvm.org/D52136



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

[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check

[Wojtek Gumuła via Phabricator via cfe-commits]

wgml updated this revision to Diff 166602.
wgml marked an inline comment as done.
wgml added a comment.

Updated signature of `concatNamespaces`.


https://reviews.llvm.org/D52136

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
  clang-tidy/modernize/ConcatNestedNamespacesCheck.h
  clang-tidy/modernize/ModernizeTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
  test/clang-tidy/modernize-concat-nested-namespaces.cpp

Index: test/clang-tidy/modernize-concat-nested-namespaces.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-concat-nested-namespaces.cpp
@@ -0,0 +1,161 @@
+// RUN: %check_clang_tidy %s modernize-concat-nested-namespaces %t -- -- -std=c++17
+
+namespace n1 {}
+
+namespace n2 {
+namespace n3 {
+void t();
+}
+namespace n4 {
+void t();
+}
+} // namespace n2
+
+namespace n5 {
+inline namespace n6 {
+void t();
+}
+} // namespace n5
+
+namespace n7 {
+void t();
+
+namespace n8 {
+void t();
+}
+} // namespace n7
+
+namespace n9 {
+namespace n10 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n9::n10
+void t();
+} // namespace n10
+} // namespace n9
+// CHECK-FIXES: }
+
+namespace n11 {
+namespace n12 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n11::n12
+namespace n13 {
+void t();
+}
+namespace n14 {
+void t();
+}
+} // namespace n12
+} // namespace n11
+// CHECK-FIXES: }
+
+namespace n15 {
+namespace n16 {
+void t();
+}
+
+inline namespace n17 {
+void t();
+}
+
+namespace n18 {
+namespace n19 {
+namespace n20 {
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n18::n19::n20
+void t();
+} // namespace n20
+} // namespace n19
+} // namespace n18
+// CHECK-FIXES: }
+
+namespace n21 {
+void t();
+}
+} // namespace n15
+
+namespace n22 {
+namespace {
+void t();
+}
+} // namespace n22
+
+namespace n23 {
+namespace {
+namespace n24 {
+namespace n25 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n24::n25
+void t();
+} // namespace n25
+} // namespace n24
+// CHECK-FIXES: }
+} // namespace
+} // namespace n23
+
+namespace n26::n27 {
+namespace n28 {
+namespace n29::n30 {
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n26::n27::n28::n29::n30
+void t() {}
+} // namespace n29::n30
+} // namespace n28
+} // namespace n26::n27
+// CHECK-FIXES: }
+
+namespace n31 {
+namespace n32 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+} // namespace n31
+// CHECK-FIXES-EMPTY
+
+namespace n33 {
+namespace n34 {
+namespace n35 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+} // namespace n34
+// CHECK-FIXES-EMPTY
+namespace n36 {
+void t();
+}
+} // namespace n33
+
+namespace n37::n38 {
+void t();
+}
+
+#define IEXIST
+namespace n39 {
+namespace n40 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n39::n40
+#ifdef IEXIST
+void t() {}
+#endif
+} // namespace n40
+} // namespace n39
+// CHECK-FIXES: }
+
+namespace n41 {
+namespace n42 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n41::n42
+#ifdef IDONTEXIST
+void t() {}
+#endif
+} // namespace n42
+} // namespace n41
+// CHECK-FIXES: }
+
+int main() {
+  n26::n27::n28::n29::n30::t();
+#ifdef IEXIST
+  n39::n40::t();
+#endif
+
+#ifdef IDONTEXIST
+  n41::n42::t();
+#endif
+
+  return 0;
+}
Index: docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - modernize-concat-nested-namespaces
+
+modernize-concat-nested-namespaces
+==
+
+Checks for use of nested namespaces in a form of ``namespace a { namespace b { ... } }``
+and offers change to syntax introduced in C++17: ``namespace a::b { ... }``.
+Inlined namespaces are not modified.
+
+For example:
+
+.. code-block:: c++
+
+  namespace n1 {
+  namespace n2 {
+  void t();
+  }
+  }
+
+  namespace n3 {
+  namespace n4 {
+  namespace n5 {
+  void t();
+  }
+  }
+  namespace n6 {
+  namespace n7 {
+  void t();
+  }
+  }
+  }
+
+Will be modified to:
+
+.. code-block:

[PATCH] D52390: [analyzer] StackSizeChecker

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added subscribers: rnkovacs, baloghadamsoftware.
Szelethus added a comment.

I think the idea for a checker like this is great! I left some inline comments, 
but most of them are minor nits. I have some general remarks to make however:

- Your code lacks comments, especially a nice documentation describing this 
problem, how you approach it, what the general algorithm is etc. For example, 
you use `Usage` extensively, but I am still a little unsure what the difference 
is between `Empty` and `EmptyFlagged`. I'd encourage you to look at a couple 
nice examples:
  1. @baloghadamsoftware's `IteratorChecker` 

 (A long code with fairly long comments to support it),
  2. @rnkovacs's `DeleteWithNonVirtualDtorChecker` 

 (A shorter code with shorter explanation),
  3. My `UninitializedObjectChecker` 

 (Very similar to your checker in terms of quantity of files etc)

- Make sure you follow the LLVM Coding Standards 
.
- As stated before, should move this to a new directory.
- For the amount of checker implementation you have, there aren't to many test 
cases. I can see that you handle `CXXTryStmt`, but there are no test cases for 
it.
- This revision is way too large for comfortable reading, you can help this by 
learning from my mistake and split it up into smaller patches. @NoQ summarized 
the reasons for this very nicely here: https://reviews.llvm.org/D45532#1083670
  1. Post a checker class with an empty callback [1]. Even in this stage, some 
discussion could be had with the general direction the implementation should 
go. This avoids the potential of investing a lot of effort into a project that 
could be wrong from the get-go. Don't get me wrong, I'm not at all saying this 
is the case here!
  2. Add implementation for checking a very easy case, such as a function with 
10 array declarations, each having an absurd size.
  3. Add each new functionality as a separate patch, and add dependencies with 
"Edit Related Revisions". For example, one for handling branches, one for 
loops, etc.

Let me emphasize that I'm a beginner myself, so I could be wrong on many of 
these ^-^

[1] This could be a nice first patch:

  //===- StackSizeChecker.cpp --*- C++ 
-*-==//
  //
  // The LLVM Compiler Infrastructure
  //
  // This file is distributed under the University of Illinois Open Source
  // License. See LICENSE.TXT for details.
  //
  
//===--===//
  //
  //  This file defines a checker that checks for suspiciously high stack use.
  //  The checker produces a warning whenever the given stack limit is exceeded.
  //  The limit is defined by the "StackUsageLimit" analyzer parameter,
  //  or defaults to 10 bytes.
  //
  
//===--===//
  
  class StackSizeChecker
  : public Checker {
mutable std::unique_ptr StackOverflowBugType;
  
  public:
int StackUsageLimit;
  
void checkPreCall(const CallEvent &Call, CheckerContext &C) const {}
void checkPostCall(const CallEvent &Call, CheckerContext &C) const {}
void checkEndFunction(CheckerContext &C) const {}
  };
  
  void clang::ento::registerStackSizeChecker(CheckerManager &Mgr) {
StackSizeChecker *Check = Mgr.registerChecker();
Check->StackUsageLimit = Mgr.getAnalyzerOptions().getOptionAsInteger(
"StackUsageLimit", /*DefaultVal*/ 10, Check);
  }




Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:30
+
+struct Usage {
+

To me, `Usage` seems to be a way too general name. Also, for a class being used 
this heavily, it has no comments explaining what it does. Is this a property of 
a variable? A function call?



Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:46
+ // its children.
+  bool ExitFlag; // Tells the visitor to cease consuming the tree.
+

Which tree? I know you refer to the //statement// tree after reading the code 
for a while, but it would be more obvious if you put some comments for `Usage`.



Comment at: 
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:94-131
+  void clear();
+  bool hasUsageStored(const Stmt *S) const {
+return StmtSubtreeUsages.find(S) != StmtSubtreeUsages.end();
+  }
+  bool hasUsageStored(const Decl *D) const {
+return DeclSubtreeUsages.find(D) != DeclSubtreeUsages.end();
+  }

Szelethus wrote:
> Are you sure this function belongs in a visitor?
Ac

[PATCH] D52334: [clang-tidy] Build it even without static analyzer

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

Thanks, that at least makes it more obvious where you are getting confused.

See `tools/clang/lib/CMakeLists.txt`. It contains:

add_subdirectory(Analysis)

...
===

if(CLANG_ENABLE_STATIC_ANALYZER)

  add_subdirectory(StaticAnalyzer)

endif()

1. That is: Analysis and StaticAnalyzer are different.

CLANG_ENABLE_STATIC_ANALYZER refers to the latter.

2. Also, something that may not have occurred to you: This patch does build. 
Obviously, if there were a problem with CFG after this patch, then it would not 
build.

Please keep both of those points in mind as you read the patch.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52334



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

[PATCH] D52390: [analyzer] StackSizeChecker

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:104
+  ProgramStateRef State = C.getState();
+  auto StackLevels = State->get();
+  if (length(StackLevels) != countPredecessors(C))

Szelethus wrote:
> It isn't obvious to me why what "StackLevels" mean. Did you mean depth? 
Ah, I I guess it's actually the height of the statement tree.


Repository:
  rC Clang

https://reviews.llvm.org/D52390



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

[PATCH] D52390: [analyzer] StackSizeChecker

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:19
+#include "clang/AST/StmtCXX.h"
+#include "clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"

Szelethus wrote:
> I bet you can't just `#include` this without getting some nasty buildbut 
> errors down the line, just `#include "StackUsageMeasuringVisitor.h"`.
Oh never mind, you had it in 
`include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h`. I bet it 
is for the clang-tidy check. Never mind.



Comment at: lib/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.cpp:17
+
+#include 
+

Szelethus wrote:
> #include "StackUsageMeasuringVisitor.h"
Never mind, but should use `""` instead of `<>`.


Repository:
  rC Clang

https://reviews.llvm.org/D52390



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

[PATCH] D52281: [clang-tidy] Add modernize check to use std::invoke in generic code

[Borsik Gábor via Phabricator via cfe-commits]

boga95 updated this revision to Diff 166604.

https://reviews.llvm.org/D52281

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp
  clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
  test/clang-tidy/modernize-replace-generic-functor-call.cpp

Index: test/clang-tidy/modernize-replace-generic-functor-call.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-replace-generic-functor-call.cpp
@@ -0,0 +1,56 @@
+// RUN: %check_clang_tidy %s modernize-replace-generic-functor-call %t -- -- -std=c++17
+
+namespace std {
+
+template 
+T max(T a, T b) {
+  return a < b ? b : a;
+}
+
+struct function {
+  void operator()();
+};
+
+} // namespace std
+
+struct Foo {
+  static void print_() {}
+  void print() const {}
+  int num_;
+};
+
+// Something that triggers the check
+template 
+void func2(T func) {
+  func(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(func, 1);
+  func();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(func);
+
+  Foo foo;
+  (foo.*func)(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(foo, func, 1);
+  (foo.*func)();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(foo, func);
+  (Foo().*func)(1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(Foo(), func, 1, 2);
+}
+
+// Something that doesn't triggers the check
+void func3() {}
+
+void g(std::function func, int (*fp)(int), void (Foo::*mfp)(int)) {
+  func3();// Regular function call
+  std::max(1, 2); // Template function call
+  Foo::print_();  // Static function call
+  []() {}();  // Lambda call
+  func(); // Call through std::function
+  fp(3);  // Call through function pointer
+  Foo foo;
+  (foo.*(mfp))(1); // Call through member function pointer
+}
Index: docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - modernize-replace-generic-functor-call
+
+modernize-replace-generic-functor-call
+==
+
+Replaces type dependent functor, function pointer and pointer to member call
+to the proper ``std::invoke()`` in C++17 or newer codebases.
+  
+Examples:
+
+.. code-block:: c++
+
+  template
+  void f(T1 functionPointer, T2 memberFunctionPointer) {
+functionPointer(2); // Replace to ::std::invoke(functionPointer, 2)
+
+Foo foo;
+(foo.*memberFunctionPointer)(1, 2); // Replace to ::std::invoke(foo, memberFunctionPointer, 1, 2)
+
+(Foo().*memberFunctionPointer)(3); // Replace to ::std::invoke(Foo(), memberFunctionPointer, 3)
+  }
+
+  // Neither of them is type dependent, no diagnose
+  void g(std::function func, int (*functionPointer)(int), void (Foo::*memberFunctionPointer)(int)) {
+freeFunction();
+
+std::max(1,2);
+
+Foo::print_();
+
+[](){}();
+
+func();
+
+functionPointer(3);
+
+Foo foo;
+(foo.*(memberFunctionPointer))(1);
+  }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -171,6 +171,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-generic-functor-call
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -142,6 +142,12 @@
 
   Checks on ``switch`` and ``if`` - ``else if`` constructs that do not cover all possible code paths.
 
+- New :doc:`modernize-replace-generic-functor-call
+  ` check.
+
+  Replaces type dependent functor, function pointer and pointer to member call
+  to the proper ``std::invoke()`` in C++17 or newer codebases.
+  
 - New :doc:`modernize-use-uncaught-exceptions
   ` check.
 
Index: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h
=

[PATCH] D52390: [analyzer] StackSizeChecker

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Without seeing the full picture (i.e. what you want to do with the clang-tidy 
check) it is hard to tell,
but are you *very* sure all this logic should be in 
`clang/lib/StaticAnalyzer/`, and not in `clang/lib/Analysis/` ?


Repository:
  rC Clang

https://reviews.llvm.org/D52390



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

[PATCH] D50130: [libc++] Fix build failures after merging

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.
Herald added a subscriber: libcxx-commits.



Comment at: lib/abi/x86_64-unknown-linux-gnu.v1.abilist:1195
 {'name': '_ZNSt3__16__clocEv', 'is_defined': True, 'type': 'FUNC'}
+{'name': '_ZNSt3__16__itoa8__u64toaEmPc', 'is_defined': True, 'type': 'FUNC'}
+{'name': '_ZNSt3__16__itoa8__u32toaEjPc', 'is_defined': True, 'type': 'FUNC'}

Whenever you add symbols to the ABI list, they need to me mentioned in the 
CHANGELOG.txt. 

Can you please add an entry to that file?


Repository:
  rCXX libc++

https://reviews.llvm.org/D50130



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

[PATCH] D45179: [libc++] Add _LIBCPP_ENABLE_NODISCARD and _LIBCPP_NODISCARD_EXT to allow pre-C++2a [[nodiscard]]

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D45179#1241721, @lebedev.ri wrote:

> In https://reviews.llvm.org/D45179#1225911, @EricWF wrote:
>
> > I don't think it ever landed. I'll try to get it in this week.
>
>
> What does this need?
>  Is there some changes missing still?
>  Or this simply needs to be committed (and bots watched?)


I think this was good to go. I was just waiting to give time for comments. 
Obviously I've waited long enough.

I'll work on this today.


https://reviews.llvm.org/D45179



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

[PATCH] D45179: [libc++] Add _LIBCPP_ENABLE_NODISCARD and _LIBCPP_NODISCARD_EXT to allow pre-C++2a [[nodiscard]]

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D45179#1242896, @EricWF wrote:

> In https://reviews.llvm.org/D45179#1241721, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D45179#1225911, @EricWF wrote:
> >
> > > I don't think it ever landed. I'll try to get it in this week.
> >
> >
> > What does this need?
> >  Is there some changes missing still?
> >  Or this simply needs to be committed (and bots watched?)
>
>
> I think this was good to go. I was just waiting to give time for comments. 
> Obviously I've waited long enough.


Ah, great.

> I'll work on this today.

If wanted, i can commandeer this back and land.


https://reviews.llvm.org/D45179



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

[PATCH] D52392: [X86] For lzcnt/tzcnt intrinsics use cttz/ctlz intrinsics with zero_undef flag set to false.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: RKSimon, spatel.
Herald added a subscriber: cfe-commits.

Previously we used a select and the zero_undef=true intrinsic. In -O2 this 
pattern will get optimized to zero_undef=false. But in -O0 this optimization 
won't happen. This results in a compare and cmov being wrapped around a 
tzcnt/lzcnt instruction.

By using the zero_undef=false intrinsic directly without the select, we can 
improve the -O0 codegen to just an lzcnt/tzcnt instruction.


Repository:
  rC Clang

https://reviews.llvm.org/D52392

Files:
  include/clang/Basic/BuiltinsX86.def
  include/clang/Basic/BuiltinsX86_64.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/bmiintrin.h
  lib/Headers/lzcntintrin.h
  test/CodeGen/bmi-builtins.c
  test/CodeGen/lzcnt-builtins.c

Index: test/CodeGen/lzcnt-builtins.c
===
--- test/CodeGen/lzcnt-builtins.c
+++ test/CodeGen/lzcnt-builtins.c
@@ -5,30 +5,30 @@
 
 unsigned short test__lzcnt16(unsigned short __X)
 {
-  // CHECK: @llvm.ctlz.i16
+  // CHECK: @llvm.ctlz.i16(i16 %{{.*}}, i1 false)
   return __lzcnt16(__X);
 }
 
 unsigned int test_lzcnt32(unsigned int __X)
 {
-  // CHECK: @llvm.ctlz.i32
+  // CHECK: @llvm.ctlz.i32(i32 %{{.*}}, i1 false)
   return __lzcnt32(__X);
 }
 
 unsigned long long test__lzcnt64(unsigned long long __X)
 {
-  // CHECK: @llvm.ctlz.i64
+  // CHECK: @llvm.ctlz.i64(i64 %{{.*}}, i1 false)
   return __lzcnt64(__X);
 }
 
 unsigned int test_lzcnt_u32(unsigned int __X)
 {
-  // CHECK: @llvm.ctlz.i32
+  // CHECK: @llvm.ctlz.i32(i32 %{{.*}}, i1 false)
   return _lzcnt_u32(__X);
 }
 
 unsigned long long test__lzcnt_u64(unsigned long long __X)
 {
-  // CHECK: @llvm.ctlz.i64
+  // CHECK: @llvm.ctlz.i64(i64 %{{.*}}, i1 false)
   return _lzcnt_u64(__X);
 }
Index: test/CodeGen/bmi-builtins.c
===
--- test/CodeGen/bmi-builtins.c
+++ test/CodeGen/bmi-builtins.c
@@ -15,9 +15,7 @@
 
 unsigned short test__tzcnt_u16(unsigned short __X) {
   // CHECK-LABEL: test__tzcnt_u16
-  // CHECK: zext i16 %{{.*}} to i32
-  // CHECK: icmp ne i32 %{{.*}}, 0
-  // CHECK: i16 @llvm.cttz.i16(i16 %{{.*}}, i1 true)
+  // CHECK: i16 @llvm.cttz.i16(i16 %{{.*}}, i1 false)
   return __tzcnt_u16(__X);
 }
 
@@ -57,15 +55,13 @@
 
 unsigned int test__tzcnt_u32(unsigned int __X) {
   // CHECK-LABEL: test__tzcnt_u32
-  // CHECK: icmp ne i32 %{{.*}}, 0
-  // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 true)
+  // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 false)
   return __tzcnt_u32(__X);
 }
 
 int test_mm_tzcnt_32(unsigned int __X) {
   // CHECK-LABEL: test_mm_tzcnt_32
-  // CHECK: icmp ne i32 %{{.*}}, 0
-  // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 true)
+  // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 false)
   return _mm_tzcnt_32(__X);
 }
 
@@ -105,25 +101,21 @@
 
 unsigned long long test__tzcnt_u64(unsigned long long __X) {
   // CHECK-LABEL: test__tzcnt_u64
-  // CHECK: icmp ne i64 %{{.*}}, 0
-  // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 true)
+  // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 false)
   return __tzcnt_u64(__X);
 }
 
 long long test_mm_tzcnt_64(unsigned long long __X) {
   // CHECK-LABEL: test_mm_tzcnt_64
-  // CHECK: icmp ne i64 %{{.*}}, 0
-  // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 true)
+  // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 false)
   return _mm_tzcnt_64(__X);
 }
 
 // Intel intrinsics
 
 unsigned short test_tzcnt_u16(unsigned short __X) {
   // CHECK-LABEL: test_tzcnt_u16
-  // CHECK: zext i16 %{{.*}} to i32
-  // CHECK: icmp ne i32 %{{.*}}, 0
-  // CHECK: i16 @llvm.cttz.i16(i16 %{{.*}}, i1 true)
+  // CHECK: i16 @llvm.cttz.i16(i16 %{{.*}}, i1 false)
   return _tzcnt_u16(__X);
 }
 
@@ -168,8 +160,7 @@
 
 unsigned int test_tzcnt_u32(unsigned int __X) {
   // CHECK-LABEL: test_tzcnt_u32
-  // CHECK: icmp ne i32 %{{.*}}, 0
-  // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 true)
+  // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 false)
   return _tzcnt_u32(__X);
 }
 
@@ -215,7 +206,6 @@
 
 unsigned long long test_tzcnt_u64(unsigned long long __X) {
   // CHECK-LABEL: test_tzcnt_u64
-  // CHECK: icmp ne i64 %{{.*}}, 0
-  // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 true)
+  // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 false)
   return _tzcnt_u64(__X);
 }
Index: lib/Headers/lzcntintrin.h
===
--- lib/Headers/lzcntintrin.h
+++ lib/Headers/lzcntintrin.h
@@ -44,7 +44,7 @@
 static __inline__ unsigned short __DEFAULT_FN_ATTRS
 __lzcnt16(unsigned short __X)
 {
-  return __X ? __builtin_clzs(__X) : 16;
+  return __builtin_ia32_lzcnt_u16(__X);
 }
 
 /// Counts the number of leading zero bits in the operand.
@@ -61,7 +61,7 @@
 static __inline__ unsigned int __DEFAULT_FN_ATTRS
 __lzcnt32(unsigned int __X)
 {
-  return __X ? __builtin_clz(__X) : 32;
+  return __builtin_ia32_lzcnt_u32(__X);
 }
 
 /// Counts the number of leading zero bits in the operand.
@@ -78,7 +78,7 @

[PATCH] D52391: Document new symbols for __u64toa and __u32toa

[Zhihao Yuan via Phabricator via cfe-commits]

lichray created this revision.
lichray added a reviewer: EricWF.
Herald added subscribers: libcxx-commits, ldionne, christof.

They are introduced in r338479; their Linux ABI changes are recorded in r338486.

TODO: Record the Mac OS X ABI changes.


Repository:
  rCXX libc++

https://reviews.llvm.org/D52391

Files:
  lib/abi/CHANGELOG.TXT


Index: lib/abi/CHANGELOG.TXT
===
--- lib/abi/CHANGELOG.TXT
+++ lib/abi/CHANGELOG.TXT
@@ -16,6 +16,18 @@
 Version 7.0
 ---
 
+* r338479 - Elementary string conversions for integral types
+
+  The change emits __u64toa and __u32toa under std::__1::__itoa.
+
+  x86_64-linux-gnu
+  
+  Symbol added: _ZNSt3__16__itoa8__u64toaEmPc
+  Symbol added: _ZNSt3__16__itoa8__u32toaEjPc
+
+  x86_64-apple-darwin16.0
+  ---
+
 * r333467 - Fix embarrasing typo in uncaught_exceptions.
 
   This bug caused __uncaught_exception to be ODR used instead of


Index: lib/abi/CHANGELOG.TXT
===
--- lib/abi/CHANGELOG.TXT
+++ lib/abi/CHANGELOG.TXT
@@ -16,6 +16,18 @@
 Version 7.0
 ---
 
+* r338479 - Elementary string conversions for integral types
+
+  The change emits __u64toa and __u32toa under std::__1::__itoa.
+
+  x86_64-linux-gnu
+  
+  Symbol added: _ZNSt3__16__itoa8__u64toaEmPc
+  Symbol added: _ZNSt3__16__itoa8__u32toaEjPc
+
+  x86_64-apple-darwin16.0
+  ---
+
 * r333467 - Fix embarrasing typo in uncaught_exceptions.
 
   This bug caused __uncaught_exception to be ODR used instead of
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45179: [libc++] Add _LIBCPP_ENABLE_NODISCARD and _LIBCPP_NODISCARD_EXT to allow pre-C++2a [[nodiscard]]

[Eric Fiselier via Phabricator via cfe-commits]

EricWF accepted this revision.
EricWF added a comment.

@lebedev.ri Sure, be my guest. It frees me up to do reviews. Thank you.

I just finished running the test suite, so it should be good to go.


https://reviews.llvm.org/D45179



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

[PATCH] D45179: [libc++] Add _LIBCPP_ENABLE_NODISCARD and _LIBCPP_NODISCARD_EXT to allow pre-C++2a [[nodiscard]]

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri commandeered this revision.
lebedev.ri removed a reviewer: lebedev.ri.
lebedev.ri added a comment.

In https://reviews.llvm.org/D45179#1242915, @EricWF wrote:

> @lebedev.ri Sure, be my guest. It frees me up to do reviews. Thank you.


Np.

> I just finished running the test suite, so it should be good to go.

Great, thank you!

Commandeering back to me, let's land this ^^


https://reviews.llvm.org/D45179



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

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang added inline comments.



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:156
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+  AST = tooling::buildASTFromCode(

JonasToth wrote:
> shuaiwang wrote:
> > JonasToth wrote:
> > > JonasToth wrote:
> > > > I feel that there a multiple tests missing:
> > > > 
> > > > - multiple levels of pointers `int ***`, `int * const *`
> > > > - pointers to references `int &*`
> > > > - references to pointers `int *&`
> > > > - ensure that having a const pointer does no influence the pointee 
> > > > analysis `int * const p = &i; *p = 42;`
> > > > - a class with `operator*` + `operator->` const/non-const and the 
> > > > analysis for pointers to that class
> > > > - pointer returned from a function
> > > > - non-const reference returned 
> > > > ```
> > > > int& foo(int *p) {
> > > >   return *p;
> > > > }
> > > > ```
> > > > 
> > > for the multi-level pointer mutation: it would be enough to test, that 
> > > the second layer is analyzed properly, and that the `int * >const< *` 
> > > would be detected.
> > Added except for:
> > - Anything that requires following a dereference, we need 
> > `findPointeeDerefMutation` for that.
> > - Pointer to a class with `operator*` + `operator->`, I think those two 
> > operators doesn't matter, there's no way to accidentally invoke them from a 
> > pointer.
> > 
> But we want to analyze smart pointers in the future as well, not? It would be 
> good to already prepare that in the testing department.
> Or would the nature of `operator*` already make that happen magically?
Yes we'll handle smart pointers, and we'll handle that in 
`findPointeeDerefMutation`, basically it'll look like:
```
if (native pointer && derefed with *) findMutation(deref expr)
if (smart pointer && called operator*) findMutation(operator call expr)
if (smart pointer && called operator->) findPointeeMutation(operator call expr)
```
I think it would be more clear if we can match the implementation progress with 
unit test cases as that shows what kind of cases starts to be supported by the 
change.


Repository:
  rC Clang

https://reviews.llvm.org/D52219



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

[PATCH] D52391: Document new symbols for __u64toa and __u32toa

[Eric Fiselier via Phabricator via cfe-commits]

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

LGTM. Thanks


Repository:
  rCXX libc++

https://reviews.llvm.org/D52391



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

[PATCH] D45179: [libc++] Add _LIBCPP_ENABLE_NODISCARD and _LIBCPP_NODISCARD_EXT to allow pre-C++2a [[nodiscard]]

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX342808: [libc++] Add _LIBCPP_ENABLE_NODISCARD and 
_LIBCPP_NODISCARD_EXT to allow pre… (authored by lebedevri, committed by ).
Herald added subscribers: libcxx-commits, ldionne.

Repository:
  rCXX libc++

https://reviews.llvm.org/D45179

Files:
  docs/UsingLibcxx.rst
  include/__config
  include/memory
  test/libcxx/diagnostics/enable_nodiscard.fail.cpp
  test/libcxx/diagnostics/enable_nodiscard_disable_after_cxx17.fail.cpp
  test/libcxx/diagnostics/enable_nodiscard_disable_nodiscard_ext.fail.cpp
  test/libcxx/diagnostics/nodiscard.fail.cpp
  test/libcxx/diagnostics/nodiscard.pass.cpp
  test/libcxx/diagnostics/nodiscard_aftercxx17.fail.cpp
  test/libcxx/diagnostics/nodiscard_aftercxx17.pass.cpp
  test/libcxx/diagnostics/nodiscard_extensions.fail.cpp
  test/libcxx/diagnostics/nodiscard_extensions.pass.cpp
  utils/libcxx/test/format.py

Index: docs/UsingLibcxx.rst
===
--- docs/UsingLibcxx.rst
+++ docs/UsingLibcxx.rst
@@ -226,6 +226,21 @@
   replacement scenarios from working, e.g. replacing `operator new` and
   expecting a non-replaced `operator new[]` to call the replaced `operator new`.
 
+**_LIBCPP_ENABLE_NODISCARD**:
+  Allow the library to add ``[[nodiscard]]`` attributes to entities not specified
+  as ``[[nodiscard]]`` by the current language dialect. This includes
+  backporting applications of ``[[nodiscard]]`` from newer dialects and
+  additional extended applications at the discretion of the library. All
+  additional applications of ``[[nodiscard]]`` are disabled by default.
+  See :ref:`Extended Applications of [[nodiscard]] ` for
+  more information.
+
+**_LIBCPP_DISABLE_NODISCARD_EXT**:
+  This macro prevents the library from applying ``[[nodiscard]]`` to entities
+  purely as an extension. See :ref:`Extended Applications of [[nodiscard]] `
+  for more information.
+
+
 C++17 Specific Configuration Macros
 ---
 **_LIBCPP_ENABLE_CXX17_REMOVED_FEATURES**:
@@ -238,3 +253,58 @@
 
 **_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR**:
   This macro is used to re-enable `std::auto_ptr` in C++17.
+
+C++2a Specific Configuration Macros:
+
+**_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17**:
+  This macro can be used to disable diagnostics emitted from functions marked
+  ``[[nodiscard]]`` in dialects after C++17.  See :ref:`Extended Applications of [[nodiscard]] `
+  for more information.
+
+
+Libc++ Extensions
+=
+
+This section documents various extensions provided by libc++, how they're
+provided, and any information regarding how to use them.
+
+.. _nodiscard extension:
+
+Extended applications of ``[[nodiscard]]``
+--
+
+The ``[[nodiscard]]`` attribute is intended to help users find bugs where
+function return values are ignored when they shouldn't be. After C++17 the
+C++ standard has started to declared such library functions as ``[[nodiscard]]``.
+However, this application is limited and applies only to dialects after C++17.
+Users who want help diagnosing misuses of STL functions may desire a more
+liberal application of ``[[nodiscard]]``.
+
+For this reason libc++ provides an extension that does just that! The
+extension must be enabled by defining ``_LIBCPP_ENABLE_NODISCARD``. The extended
+applications of ``[[nodiscard]]`` takes two forms:
+
+1. Backporting ``[[nodiscard]]`` to entities declared as such by the
+   standard in newer dialects, but not in the present one.
+
+2. Extended applications of ``[[nodiscard]]``, at the libraries discretion,
+   applied to entities never declared as such by the standard.
+
+Users may also opt-out of additional applications ``[[nodiscard]]`` using
+additional macros.
+
+Applications of the first form, which backport ``[[nodiscard]]`` from a newer
+dialect may be disabled using macros specific to the dialect it was added. For
+example ``_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17``.
+
+Applications of the second form, which are pure extensions, may be disabled
+by defining ``_LIBCPP_DISABLE_NODISCARD_EXT``.
+
+
+Entities declared with ``_LIBCPP_NODISCARD_EXT``
+
+
+This section lists all extended applications of ``[[nodiscard]]`` to entities
+which no dialect declares as such (See the second form described above).
+
+* ``get_temporary_buffer``
Index: include/memory
===
--- include/memory
+++ include/memory
@@ -2004,7 +2004,7 @@
 };
 
 template 
-_LIBCPP_NO_CFI
+_LIBCPP_NODISCARD_EXT _LIBCPP_NO_CFI
 pair<_Tp*, ptrdiff_t>
 get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT
 {
Index: include/__config
===
--- include/__config
+++ include/__config
@@ -1049,8 +1049,30 @@
 #  define _LIBCPP_CONSTEXPR_AFTER_CXX17
 #endif
 
-#if __has_cpp_

[PATCH] D45179: [libc++] Add _LIBCPP_ENABLE_NODISCARD and _LIBCPP_NODISCARD_EXT to allow pre-C++2a [[nodiscard]]

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342808: [libc++] Add _LIBCPP_ENABLE_NODISCARD and 
_LIBCPP_NODISCARD_EXT to allow pre… (authored by lebedevri, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45179?vs=154724&id=166607#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D45179

Files:
  libcxx/trunk/docs/UsingLibcxx.rst
  libcxx/trunk/include/__config
  libcxx/trunk/include/memory
  libcxx/trunk/test/libcxx/diagnostics/enable_nodiscard.fail.cpp
  
libcxx/trunk/test/libcxx/diagnostics/enable_nodiscard_disable_after_cxx17.fail.cpp
  
libcxx/trunk/test/libcxx/diagnostics/enable_nodiscard_disable_nodiscard_ext.fail.cpp
  libcxx/trunk/test/libcxx/diagnostics/nodiscard.fail.cpp
  libcxx/trunk/test/libcxx/diagnostics/nodiscard.pass.cpp
  libcxx/trunk/test/libcxx/diagnostics/nodiscard_aftercxx17.fail.cpp
  libcxx/trunk/test/libcxx/diagnostics/nodiscard_aftercxx17.pass.cpp
  libcxx/trunk/test/libcxx/diagnostics/nodiscard_extensions.fail.cpp
  libcxx/trunk/test/libcxx/diagnostics/nodiscard_extensions.pass.cpp
  libcxx/trunk/utils/libcxx/test/format.py

Index: libcxx/trunk/utils/libcxx/test/format.py
===
--- libcxx/trunk/utils/libcxx/test/format.py
+++ libcxx/trunk/utils/libcxx/test/format.py
@@ -250,9 +250,8 @@
 #
 # Therefore, we check if the test was expected to fail because of
 # nodiscard before enabling it
-test_str = "ignoring return value of function declared with " \
-  + "'nodiscard' attribute"
-if test_str in contents:
+test_str_list = ['ignoring return value', 'nodiscard', 'NODISCARD']
+if any(test_str in contents for test_str in test_str_list):
 test_cxx.flags += ['-Werror=unused-result']
 cmd, out, err, rc = test_cxx.compile(source_path, out=os.devnull)
 expected_rc = 0 if use_verify else 1
Index: libcxx/trunk/docs/UsingLibcxx.rst
===
--- libcxx/trunk/docs/UsingLibcxx.rst
+++ libcxx/trunk/docs/UsingLibcxx.rst
@@ -226,6 +226,21 @@
   replacement scenarios from working, e.g. replacing `operator new` and
   expecting a non-replaced `operator new[]` to call the replaced `operator new`.
 
+**_LIBCPP_ENABLE_NODISCARD**:
+  Allow the library to add ``[[nodiscard]]`` attributes to entities not specified
+  as ``[[nodiscard]]`` by the current language dialect. This includes
+  backporting applications of ``[[nodiscard]]`` from newer dialects and
+  additional extended applications at the discretion of the library. All
+  additional applications of ``[[nodiscard]]`` are disabled by default.
+  See :ref:`Extended Applications of [[nodiscard]] ` for
+  more information.
+
+**_LIBCPP_DISABLE_NODISCARD_EXT**:
+  This macro prevents the library from applying ``[[nodiscard]]`` to entities
+  purely as an extension. See :ref:`Extended Applications of [[nodiscard]] `
+  for more information.
+
+
 C++17 Specific Configuration Macros
 ---
 **_LIBCPP_ENABLE_CXX17_REMOVED_FEATURES**:
@@ -238,3 +253,58 @@
 
 **_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR**:
   This macro is used to re-enable `std::auto_ptr` in C++17.
+
+C++2a Specific Configuration Macros:
+
+**_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17**:
+  This macro can be used to disable diagnostics emitted from functions marked
+  ``[[nodiscard]]`` in dialects after C++17.  See :ref:`Extended Applications of [[nodiscard]] `
+  for more information.
+
+
+Libc++ Extensions
+=
+
+This section documents various extensions provided by libc++, how they're
+provided, and any information regarding how to use them.
+
+.. _nodiscard extension:
+
+Extended applications of ``[[nodiscard]]``
+--
+
+The ``[[nodiscard]]`` attribute is intended to help users find bugs where
+function return values are ignored when they shouldn't be. After C++17 the
+C++ standard has started to declared such library functions as ``[[nodiscard]]``.
+However, this application is limited and applies only to dialects after C++17.
+Users who want help diagnosing misuses of STL functions may desire a more
+liberal application of ``[[nodiscard]]``.
+
+For this reason libc++ provides an extension that does just that! The
+extension must be enabled by defining ``_LIBCPP_ENABLE_NODISCARD``. The extended
+applications of ``[[nodiscard]]`` takes two forms:
+
+1. Backporting ``[[nodiscard]]`` to entities declared as such by the
+   standard in newer dialects, but not in the present one.
+
+2. Extended applications of ``[[nodiscard]]``, at the libraries discretion,
+   applied to entities never declared as such by the standard.
+
+Users may also opt-out of additional applications ``[[nodiscard]]`` using
+additional macros.
+
+Applications of the first form, which backport

[PATCH] D52393: [libunwind][NFC] Suppress unused parameter warnings

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added a reviewer: EricWF.
Herald added subscribers: cfe-commits, dexonsmith, chrib, christof.

Repository:
  rUNW libunwind

https://reviews.llvm.org/D52393

Files:
  libunwind/src/Registers.hpp


Index: libunwind/src/Registers.hpp
===
--- libunwind/src/Registers.hpp
+++ libunwind/src/Registers.hpp
@@ -517,6 +517,7 @@
 return false;
   return true;
 #else
+  (void)regNum; // suppress unused parameter warning
   return false;
 #endif
 }
@@ -526,6 +527,7 @@
   assert(validVectorRegister(regNum));
   return _xmm[regNum - UNW_X86_64_XMM0];
 #else
+  (void)regNum; // suppress unused parameter warning
   _LIBUNWIND_ABORT("no x86_64 vector registers");
 #endif
 }
@@ -535,6 +537,7 @@
   assert(validVectorRegister(regNum));
   _xmm[regNum - UNW_X86_64_XMM0] = value;
 #else
+  (void)regNum; (void)value; // suppress unused parameter warnings
   _LIBUNWIND_ABORT("no x86_64 vector registers");
 #endif
 }
@@ -2149,7 +2152,7 @@
   memset(&_vfp_d16_d31, 0, sizeof(_vfp_d16_d31));
 #if defined(__ARM_WMMX)
   _saved_iwmmx = false;
-  _saved_iwmmx_control = false;  
+  _saved_iwmmx_control = false;
   memset(&_iwmmx, 0, sizeof(_iwmmx));
   memset(&_iwmmx_control, 0, sizeof(_iwmmx_control));
 #endif


Index: libunwind/src/Registers.hpp
===
--- libunwind/src/Registers.hpp
+++ libunwind/src/Registers.hpp
@@ -517,6 +517,7 @@
 return false;
   return true;
 #else
+  (void)regNum; // suppress unused parameter warning
   return false;
 #endif
 }
@@ -526,6 +527,7 @@
   assert(validVectorRegister(regNum));
   return _xmm[regNum - UNW_X86_64_XMM0];
 #else
+  (void)regNum; // suppress unused parameter warning
   _LIBUNWIND_ABORT("no x86_64 vector registers");
 #endif
 }
@@ -535,6 +537,7 @@
   assert(validVectorRegister(regNum));
   _xmm[regNum - UNW_X86_64_XMM0] = value;
 #else
+  (void)regNum; (void)value; // suppress unused parameter warnings
   _LIBUNWIND_ABORT("no x86_64 vector registers");
 #endif
 }
@@ -2149,7 +2152,7 @@
   memset(&_vfp_d16_d31, 0, sizeof(_vfp_d16_d31));
 #if defined(__ARM_WMMX)
   _saved_iwmmx = false;
-  _saved_iwmmx_control = false;  
+  _saved_iwmmx_control = false;
   memset(&_iwmmx, 0, sizeof(_iwmmx));
   memset(&_iwmmx_control, 0, sizeof(_iwmmx_control));
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52391: Document new symbols for __u64toa and __u32toa

[Zhihao Yuan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX342810: Document new symbols for __u64toa and __u32toa 
(authored by lichray, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52391?vs=166605&id=166610#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D52391

Files:
  lib/abi/CHANGELOG.TXT


Index: lib/abi/CHANGELOG.TXT
===
--- lib/abi/CHANGELOG.TXT
+++ lib/abi/CHANGELOG.TXT
@@ -16,6 +16,18 @@
 Version 7.0
 ---
 
+* r338479 - Elementary string conversions for integral types
+
+  The change emits __u64toa and __u32toa under std::__1::__itoa.
+
+  x86_64-linux-gnu
+  
+  Symbol added: _ZNSt3__16__itoa8__u64toaEmPc
+  Symbol added: _ZNSt3__16__itoa8__u32toaEjPc
+
+  x86_64-apple-darwin16.0
+  ---
+
 * r333467 - Fix embarrasing typo in uncaught_exceptions.
 
   This bug caused __uncaught_exception to be ODR used instead of


Index: lib/abi/CHANGELOG.TXT
===
--- lib/abi/CHANGELOG.TXT
+++ lib/abi/CHANGELOG.TXT
@@ -16,6 +16,18 @@
 Version 7.0
 ---
 
+* r338479 - Elementary string conversions for integral types
+
+  The change emits __u64toa and __u32toa under std::__1::__itoa.
+
+  x86_64-linux-gnu
+  
+  Symbol added: _ZNSt3__16__itoa8__u64toaEmPc
+  Symbol added: _ZNSt3__16__itoa8__u32toaEjPc
+
+  x86_64-apple-darwin16.0
+  ---
+
 * r333467 - Fix embarrasing typo in uncaught_exceptions.
 
   This bug caused __uncaught_exception to be ODR used instead of
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52394: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin

[Louis Dionne via Phabricator via cfe-commits]

ldionne updated this revision to Diff 166613.
ldionne added a comment.

Remove the change to the Darwin ABI list file, which was meant for a different 
commit.


Repository:
  rCXX libc++

https://reviews.llvm.org/D52394

Files:
  libcxx/lib/abi/CMakeLists.txt


Index: libcxx/lib/abi/CMakeLists.txt
===
--- libcxx/lib/abi/CMakeLists.txt
+++ libcxx/lib/abi/CMakeLists.txt
@@ -1,12 +1,16 @@
 
 if (DEFINED TARGET_TRIPLE)
-# Ignore the major, minor, and patchlevel versions of darwin targets.
-string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
-
-# Ignore the major and minor versions of freebsd targets.
-string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+if (TARGET_TRIPLE MATCHES "darwin")
+# Ignore the major, minor, and patchlevel versions of darwin targets.
+string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+elseif(TARGET_TRIPLE MATCHES "freebsd")
+# Ignore the major and minor versions of freebsd targets.
+string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+else()
+set(GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+endif()
 endif()
 
 # Detect if we are building in the same configuration used to generate


Index: libcxx/lib/abi/CMakeLists.txt
===
--- libcxx/lib/abi/CMakeLists.txt
+++ libcxx/lib/abi/CMakeLists.txt
@@ -1,12 +1,16 @@
 
 if (DEFINED TARGET_TRIPLE)
-# Ignore the major, minor, and patchlevel versions of darwin targets.
-string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
-
-# Ignore the major and minor versions of freebsd targets.
-string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+if (TARGET_TRIPLE MATCHES "darwin")
+# Ignore the major, minor, and patchlevel versions of darwin targets.
+string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+elseif(TARGET_TRIPLE MATCHES "freebsd")
+# Ignore the major and minor versions of freebsd targets.
+string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+else()
+set(GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+endif()
 endif()
 
 # Detect if we are building in the same configuration used to generate
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52394: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added reviewers: dim, EricWF.
Herald added subscribers: libcxx-commits, cfe-commits, dexonsmith, christof, 
krytarowski, mgorny, emaste.
ldionne updated this revision to Diff 166613.
ldionne added a comment.

Remove the change to the Darwin ABI list file, which was meant for a different 
commit.


r342805 added support for the check-cxx-abilist target on FreeBSD, but broke
the target on macOS in doing so. The problem is that the GENERIC_TARGET_TRIPLE
gets overwritten after replacing the FreeBSD regular expression, which
nullifies the replacement done with the darwin regular expression.


Repository:
  rCXX libc++

https://reviews.llvm.org/D52394

Files:
  libcxx/lib/abi/CMakeLists.txt


Index: libcxx/lib/abi/CMakeLists.txt
===
--- libcxx/lib/abi/CMakeLists.txt
+++ libcxx/lib/abi/CMakeLists.txt
@@ -1,12 +1,16 @@
 
 if (DEFINED TARGET_TRIPLE)
-# Ignore the major, minor, and patchlevel versions of darwin targets.
-string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
-
-# Ignore the major and minor versions of freebsd targets.
-string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+if (TARGET_TRIPLE MATCHES "darwin")
+# Ignore the major, minor, and patchlevel versions of darwin targets.
+string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+elseif(TARGET_TRIPLE MATCHES "freebsd")
+# Ignore the major and minor versions of freebsd targets.
+string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+else()
+set(GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+endif()
 endif()
 
 # Detect if we are building in the same configuration used to generate


Index: libcxx/lib/abi/CMakeLists.txt
===
--- libcxx/lib/abi/CMakeLists.txt
+++ libcxx/lib/abi/CMakeLists.txt
@@ -1,12 +1,16 @@
 
 if (DEFINED TARGET_TRIPLE)
-# Ignore the major, minor, and patchlevel versions of darwin targets.
-string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
-
-# Ignore the major and minor versions of freebsd targets.
-string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+if (TARGET_TRIPLE MATCHES "darwin")
+# Ignore the major, minor, and patchlevel versions of darwin targets.
+string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+elseif(TARGET_TRIPLE MATCHES "freebsd")
+# Ignore the major and minor versions of freebsd targets.
+string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+else()
+set(GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+endif()
 endif()
 
 # Detect if we are building in the same configuration used to generate
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] r342811 - [libunwind][NFC] Suppress unused parameter warnings

[Louis Dionne via cfe-commits]

Author: ldionne
Date: Sat Sep 22 11:18:34 2018
New Revision: 342811

URL: http://llvm.org/viewvc/llvm-project?rev=342811&view=rev
Log:
[libunwind][NFC] Suppress unused parameter warnings

Reviewers: EricWF

Subscribers: christof, chrib, dexonsmith, cfe-commits

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

Modified:
libunwind/trunk/src/Registers.hpp

Modified: libunwind/trunk/src/Registers.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Registers.hpp?rev=342811&r1=342810&r2=342811&view=diff
==
--- libunwind/trunk/src/Registers.hpp (original)
+++ libunwind/trunk/src/Registers.hpp Sat Sep 22 11:18:34 2018
@@ -517,6 +517,7 @@ inline bool Registers_x86_64::validVecto
 return false;
   return true;
 #else
+  (void)regNum; // suppress unused parameter warning
   return false;
 #endif
 }
@@ -526,6 +527,7 @@ inline v128 Registers_x86_64::getVectorR
   assert(validVectorRegister(regNum));
   return _xmm[regNum - UNW_X86_64_XMM0];
 #else
+  (void)regNum; // suppress unused parameter warning
   _LIBUNWIND_ABORT("no x86_64 vector registers");
 #endif
 }
@@ -535,6 +537,7 @@ inline void Registers_x86_64::setVectorR
   assert(validVectorRegister(regNum));
   _xmm[regNum - UNW_X86_64_XMM0] = value;
 #else
+  (void)regNum; (void)value; // suppress unused parameter warnings
   _LIBUNWIND_ABORT("no x86_64 vector registers");
 #endif
 }
@@ -2149,7 +2152,7 @@ inline Registers_arm::Registers_arm()
   memset(&_vfp_d16_d31, 0, sizeof(_vfp_d16_d31));
 #if defined(__ARM_WMMX)
   _saved_iwmmx = false;
-  _saved_iwmmx_control = false;  
+  _saved_iwmmx_control = false;
   memset(&_iwmmx, 0, sizeof(_iwmmx));
   memset(&_iwmmx_control, 0, sizeof(_iwmmx_control));
 #endif


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

[PATCH] D52393: [libunwind][NFC] Suppress unused parameter warnings

[Louis Dionne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342811: [libunwind][NFC] Suppress unused parameter warnings 
(authored by ldionne, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52393?vs=166609&id=166614#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52393

Files:
  libunwind/trunk/src/Registers.hpp


Index: libunwind/trunk/src/Registers.hpp
===
--- libunwind/trunk/src/Registers.hpp
+++ libunwind/trunk/src/Registers.hpp
@@ -517,6 +517,7 @@
 return false;
   return true;
 #else
+  (void)regNum; // suppress unused parameter warning
   return false;
 #endif
 }
@@ -526,6 +527,7 @@
   assert(validVectorRegister(regNum));
   return _xmm[regNum - UNW_X86_64_XMM0];
 #else
+  (void)regNum; // suppress unused parameter warning
   _LIBUNWIND_ABORT("no x86_64 vector registers");
 #endif
 }
@@ -535,6 +537,7 @@
   assert(validVectorRegister(regNum));
   _xmm[regNum - UNW_X86_64_XMM0] = value;
 #else
+  (void)regNum; (void)value; // suppress unused parameter warnings
   _LIBUNWIND_ABORT("no x86_64 vector registers");
 #endif
 }
@@ -2149,7 +2152,7 @@
   memset(&_vfp_d16_d31, 0, sizeof(_vfp_d16_d31));
 #if defined(__ARM_WMMX)
   _saved_iwmmx = false;
-  _saved_iwmmx_control = false;  
+  _saved_iwmmx_control = false;
   memset(&_iwmmx, 0, sizeof(_iwmmx));
   memset(&_iwmmx_control, 0, sizeof(_iwmmx_control));
 #endif


Index: libunwind/trunk/src/Registers.hpp
===
--- libunwind/trunk/src/Registers.hpp
+++ libunwind/trunk/src/Registers.hpp
@@ -517,6 +517,7 @@
 return false;
   return true;
 #else
+  (void)regNum; // suppress unused parameter warning
   return false;
 #endif
 }
@@ -526,6 +527,7 @@
   assert(validVectorRegister(regNum));
   return _xmm[regNum - UNW_X86_64_XMM0];
 #else
+  (void)regNum; // suppress unused parameter warning
   _LIBUNWIND_ABORT("no x86_64 vector registers");
 #endif
 }
@@ -535,6 +537,7 @@
   assert(validVectorRegister(regNum));
   _xmm[regNum - UNW_X86_64_XMM0] = value;
 #else
+  (void)regNum; (void)value; // suppress unused parameter warnings
   _LIBUNWIND_ABORT("no x86_64 vector registers");
 #endif
 }
@@ -2149,7 +2152,7 @@
   memset(&_vfp_d16_d31, 0, sizeof(_vfp_d16_d31));
 #if defined(__ARM_WMMX)
   _saved_iwmmx = false;
-  _saved_iwmmx_control = false;  
+  _saved_iwmmx_control = false;
   memset(&_iwmmx, 0, sizeof(_iwmmx));
   memset(&_iwmmx_control, 0, sizeof(_iwmmx_control));
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52334: [clang-tidy] Build it even without static analyzer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D52334#1242881, @steveire wrote:

> Thanks, that at least makes it more obvious where you are getting confused.
>
> See `tools/clang/lib/CMakeLists.txt`. It contains:
>
>   add_subdirectory(Analysis)
>   # ...
>   if(CLANG_ENABLE_STATIC_ANALYZER)
> add_subdirectory(StaticAnalyzer)
>   endif()
>
>
>
>
> 1. That is: Analysis and StaticAnalyzer are different.
>
>   CLANG_ENABLE_STATIC_ANALYZER refers to the latter.
> 2. Also, something that may not have occurred to you: This patch does build. 
> Obviously, if there were a problem with CFG after this patch, then it would 
> not build.
>
>   Please keep both of those points in mind as you read the patch.


I expected, that you tested a build and running the unit tests as well ;)
What i suspected, that there are other parts directly in C++ source code that 
do an `if CLANG_ENABLE_STATIC_ANALYZER` to switch something on/off. I checked 
that now and it does not seem to be the case.

@alexfh should make the decision as the code-owner, but to me it makes sense to 
have clang-tidy without CSA.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52334



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

r342812 - Fix codemodels.c test case (only test mcmodel=medium on X86).

[Caroline Tice via cfe-commits]

Author: ctice
Date: Sat Sep 22 11:25:58 2018
New Revision: 342812

URL: http://llvm.org/viewvc/llvm-project?rev=342812&view=rev
Log:
Fix codemodels.c test case (only test mcmodel=medium on X86).

aarch64 testing is broken because "medium" is not a valid
code-model on aarch64, and codemodels.c tests that.  This fixes
that problem by adding "-triple x86_64-unknown-linux-gnu" to the
test with "-mcode-model moedium".



Modified:
cfe/trunk/test/CodeGen/codemodels.c

Modified: cfe/trunk/test/CodeGen/codemodels.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/codemodels.c?rev=342812&r1=342811&r2=342812&view=diff
==
--- cfe/trunk/test/CodeGen/codemodels.c (original)
+++ cfe/trunk/test/CodeGen/codemodels.c Sat Sep 22 11:25:58 2018
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 -emit-llvm -mcode-model tiny %s -o - | FileCheck %s 
-check-prefix=CHECK-TINY
 // RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s 
-check-prefix=CHECK-SMALL
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -mcode-model 
kernel %s -o - | FileCheck %s -check-prefix=CHECK-KERNEL
-// RUN: %clang_cc1 -emit-llvm -mcode-model medium %s -o - | FileCheck %s 
-check-prefix=CHECK-MEDIUM
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -mcode-model 
medium %s -o - | FileCheck %s -check-prefix=CHECK-MEDIUM
 // RUN: %clang_cc1 -emit-llvm -mcode-model large %s -o - | FileCheck %s 
-check-prefix=CHECK-LARGE
 
 // CHECK-TINY: !llvm.module.flags = !{{{.*}}}


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

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

TDD, thats ok ;)

Am 22.09.2018 um 19:37 schrieb Shuai Wang via Phabricator:

> shuaiwang added inline comments.
> 
> 
>  Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:156
> 
>   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
> 
> +
>  +  AST = tooling::buildASTFromCode(
> 
>  
> 
> JonasToth wrote:
> 
>> shuaiwang wrote:
>> 
>>> JonasToth wrote:
>>> 
 JonasToth wrote:
 
> I feel that there a multiple tests missing:
> 
> - multiple levels of pointers `int ***`, `int * const *`
> - pointers to references `int &*`
> - references to pointers `int *&`
> - ensure that having a const pointer does no influence the pointee 
> analysis `int * const p = &i; *p = 42;`
> - a class with `operator*` + `operator->` const/non-const and the 
> analysis for pointers to that class
> - pointer returned from a function
> - non-const reference returned ``` int& foo(int *p) { return *p; } ```
 
 for the multi-level pointer mutation: it would be enough to test, that the 
 second layer is analyzed properly, and that the `int * >const< *` would be 
 detected.
>>> 
>>> Added except for:
>>> 
>>> - Anything that requires following a dereference, we need 
>>> `findPointeeDerefMutation` for that.
>>> - Pointer to a class with `operator*` + `operator->`, I think those two 
>>> operators doesn't matter, there's no way to accidentally invoke them from a 
>>> pointer.
>> 
>> But we want to analyze smart pointers in the future as well, not? It would 
>> be good to already prepare that in the testing department.
>>  Or would the nature of `operator*` already make that happen magically?
> 
> Yes we'll handle smart pointers, and we'll handle that in 
> `findPointeeDerefMutation`, basically it'll look like:
> 
>   if (native pointer && derefed with *) findMutation(deref expr)
>   if (smart pointer && called operator*) findMutation(operator call expr)
>   if (smart pointer && called operator->) findPointeeMutation(operator call 
> expr)
> 
> 
> I think it would be more clear if we can match the implementation progress 
> with unit test cases as that shows what kind of cases starts to be supported 
> by the change.
> 
> Repository:
> 
>   rC Clang
> 
> https://reviews.llvm.org/D52219


Repository:
  rC Clang

https://reviews.llvm.org/D52219



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

[PATCH] D52395: Thread safety analysis: Require exclusive lock for passing by non-const reference

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley.
Herald added a subscriber: cfe-commits.

When passing by reference, we check if the reference is const-qualified
and if it isn't, we demand an exclusive lock. Unlike checking const
qualifiers on member functions, there are probably not many false
positives here: if a function takes a non-const reference, it will
in almost all cases modify the object that we passed it.


Repository:
  rC Clang

https://reviews.llvm.org/D52395

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/ThreadSafety.cpp
  test/SemaCXX/warn-thread-safety-analysis.cpp

Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5026,19 +5026,19 @@
 
   void test1() {
 copy(foo); // expected-warning {{reading variable 'foo' requires holding mutex 'mu'}}
-write1(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-write2(10, foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+write1(foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+write2(10, foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 read1(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 read2(10, foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-destroy(mymove(foo));  // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+destroy(mymove(foo));  // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 
-mwrite1(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-mwrite2(10, foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+mwrite1(foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+mwrite2(10, foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 mread1(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 mread2(10, foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 
-smwrite1(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-smwrite2(10, foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+smwrite1(foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+smwrite2(10, foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 smread1(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 smread2(10, foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 
@@ -5053,12 +5053,13 @@
 (*this) << foo;  // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 
 copy(*foop); // expected-warning {{reading the value pointed to by 'foop' requires holding mutex 'mu'}}
-write1(*foop);   // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
-write2(10, *foop);   // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
+write1(*foop);   // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
+write2(10, *foop);   // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
 read1(*foop);// expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
 read2(10, *foop);// expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
-destroy(mymove(*foop));  // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
+destroy(mymove(*foop));  // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
 
+// TODO -- sometimes this should warn about writing.
 copy(*f

[PATCH] D52396: [libcxx] Document new symbols __u64toa and __u32toa on Darwin

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added a reviewer: EricWF.
Herald added subscribers: libcxx-commits, cfe-commits, dexonsmith, christof.

This is the counterpart for https://reviews.llvm.org/D50130 and
https://reviews.llvm.org/D52391 on Darwin.


Repository:
  rCXX libc++

https://reviews.llvm.org/D52396

Files:
  libcxx/lib/abi/CHANGELOG.TXT
  libcxx/lib/abi/x86_64-apple-darwin.v1.abilist


Index: libcxx/lib/abi/x86_64-apple-darwin.v1.abilist
===
--- libcxx/lib/abi/x86_64-apple-darwin.v1.abilist
+++ libcxx/lib/abi/x86_64-apple-darwin.v1.abilist
@@ -1282,6 +1282,8 @@
 {'type': 'OBJECT', 'is_defined': True, 'name': '__ZNSt3__15wcerrE', 'size': 0}
 {'type': 'OBJECT', 'is_defined': True, 'name': '__ZNSt3__15wclogE', 'size': 0}
 {'type': 'OBJECT', 'is_defined': True, 'name': '__ZNSt3__15wcoutE', 'size': 0}
+{'type': 'FUNC', 'is_defined': True, 'name': '__ZNSt3__16__itoa8__u32toaEjPc'}
+{'type': 'FUNC', 'is_defined': True, 'name': '__ZNSt3__16__itoa8__u64toaEyPc'}
 {'type': 'FUNC', 'is_defined': True, 'name': 
'__ZNSt3__16__sortIRNS_6__lessIaaEEPaEEvT0_S5_T_'}
 {'type': 'FUNC', 'is_defined': True, 'name': 
'__ZNSt3__16__sortIRNS_6__lessIccEEPcEEvT0_S5_T_'}
 {'type': 'FUNC', 'is_defined': True, 'name': 
'__ZNSt3__16__sortIRNS_6__lessIddEEPdEEvT0_S5_T_'}
Index: libcxx/lib/abi/CHANGELOG.TXT
===
--- libcxx/lib/abi/CHANGELOG.TXT
+++ libcxx/lib/abi/CHANGELOG.TXT
@@ -27,6 +27,9 @@
 
   x86_64-apple-darwin16.0
   ---
+  Symbol added: __ZNSt3__16__itoa8__u64toaEyPc
+  Symbol added: __ZNSt3__16__itoa8__u32toaEjPc
+
 
 * r333467 - Fix embarrasing typo in uncaught_exceptions.
 


Index: libcxx/lib/abi/x86_64-apple-darwin.v1.abilist
===
--- libcxx/lib/abi/x86_64-apple-darwin.v1.abilist
+++ libcxx/lib/abi/x86_64-apple-darwin.v1.abilist
@@ -1282,6 +1282,8 @@
 {'type': 'OBJECT', 'is_defined': True, 'name': '__ZNSt3__15wcerrE', 'size': 0}
 {'type': 'OBJECT', 'is_defined': True, 'name': '__ZNSt3__15wclogE', 'size': 0}
 {'type': 'OBJECT', 'is_defined': True, 'name': '__ZNSt3__15wcoutE', 'size': 0}
+{'type': 'FUNC', 'is_defined': True, 'name': '__ZNSt3__16__itoa8__u32toaEjPc'}
+{'type': 'FUNC', 'is_defined': True, 'name': '__ZNSt3__16__itoa8__u64toaEyPc'}
 {'type': 'FUNC', 'is_defined': True, 'name': '__ZNSt3__16__sortIRNS_6__lessIaaEEPaEEvT0_S5_T_'}
 {'type': 'FUNC', 'is_defined': True, 'name': '__ZNSt3__16__sortIRNS_6__lessIccEEPcEEvT0_S5_T_'}
 {'type': 'FUNC', 'is_defined': True, 'name': '__ZNSt3__16__sortIRNS_6__lessIddEEPdEEvT0_S5_T_'}
Index: libcxx/lib/abi/CHANGELOG.TXT
===
--- libcxx/lib/abi/CHANGELOG.TXT
+++ libcxx/lib/abi/CHANGELOG.TXT
@@ -27,6 +27,9 @@
 
   x86_64-apple-darwin16.0
   ---
+  Symbol added: __ZNSt3__16__itoa8__u64toaEyPc
+  Symbol added: __ZNSt3__16__itoa8__u32toaEjPc
+
 
 * r333467 - Fix embarrasing typo in uncaught_exceptions.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52394: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin

[Louis Dionne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342813: [libcxx] Fix the definition of the check-cxx-abilist 
target on Darwin (authored by ldionne, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52394?vs=166613&id=166617#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52394

Files:
  libcxx/trunk/lib/abi/CMakeLists.txt


Index: libcxx/trunk/lib/abi/CMakeLists.txt
===
--- libcxx/trunk/lib/abi/CMakeLists.txt
+++ libcxx/trunk/lib/abi/CMakeLists.txt
@@ -1,12 +1,16 @@
 
 if (DEFINED TARGET_TRIPLE)
-# Ignore the major, minor, and patchlevel versions of darwin targets.
-string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
-
-# Ignore the major and minor versions of freebsd targets.
-string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+if (TARGET_TRIPLE MATCHES "darwin")
+# Ignore the major, minor, and patchlevel versions of darwin targets.
+string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+elseif(TARGET_TRIPLE MATCHES "freebsd")
+# Ignore the major and minor versions of freebsd targets.
+string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+else()
+set(GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+endif()
 endif()
 
 # Detect if we are building in the same configuration used to generate


Index: libcxx/trunk/lib/abi/CMakeLists.txt
===
--- libcxx/trunk/lib/abi/CMakeLists.txt
+++ libcxx/trunk/lib/abi/CMakeLists.txt
@@ -1,12 +1,16 @@
 
 if (DEFINED TARGET_TRIPLE)
-# Ignore the major, minor, and patchlevel versions of darwin targets.
-string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
-
-# Ignore the major and minor versions of freebsd targets.
-string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
-   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+if (TARGET_TRIPLE MATCHES "darwin")
+# Ignore the major, minor, and patchlevel versions of darwin targets.
+string(REGEX REPLACE "darwin[0-9]+\\.[0-9]+\\.[0-9]+" "darwin"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+elseif(TARGET_TRIPLE MATCHES "freebsd")
+# Ignore the major and minor versions of freebsd targets.
+string(REGEX REPLACE "freebsd[0-9]+\\.[0-9]+" "freebsd"
+   GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+else()
+set(GENERIC_TARGET_TRIPLE "${TARGET_TRIPLE}")
+endif()
 endif()
 
 # Detect if we are building in the same configuration used to generate
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52281: [clang-tidy] Add modernize check to use std::invoke in generic code

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:70
+  if (MFunctor && MFunctor->isTypeDependent()) {
+const auto *Paren = static_cast(MFunctor->getCallee());
+const auto *BinOp =

Eugene.Zelenko wrote:
> I think you should use LLVM's cast instead, but I'm not sure which one. Same 
> for other places.
in this case i think `const auto *BinOp = 
dyn_cast(Paren->getSubExpr());` would match.

As a safety measure adding a `assert(BinOp && "No Binary Operator as 
subexpression");` helps spotting bugs.



Comment at: test/clang-tidy/modernize-replace-generic-functor-call.cpp:24
+template 
+void func2(T func) {
+  func(1);

Please add tests that include the usage of macros to do the function call.
In my opinions these should be excluded from the FIXITs and only the warning 
should be emitted.


https://reviews.llvm.org/D52281



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

[PATCH] D52394: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin

[Dimitry Andric via Phabricator via cfe-commits]

dim added a comment.

Ah, sorry about that! I should have realized this, but my CMake-fu is weak. :)


Repository:
  rL LLVM

https://reviews.llvm.org/D52394



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

[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

ping :)


https://reviews.llvm.org/D49722



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

[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:275
+os << "sizeof(" << DstName << ")";
+  else
+os << "sizeof()";

Why can't this `else if` case be folded into the `strlcpy` case? There are lots 
of duplication.

`strlcpy` does not check `DstName.empty()` but this one does. Is there any 
cases I am missing?


https://reviews.llvm.org/D49722



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

[PATCH] D51187: [RFC] Thread safety analysis: Track status of scoped capability

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

> Any changes should always be done by adding or removing entries from the 
> FactSet, not by mutating the underlying FactEntries.

To make that clearer in the code, I made `FactEntry`s immutable that are 
managed by `FactManager` in https://reviews.llvm.org/rC342787.

In https://reviews.llvm.org/D51187#1242620, @delesley wrote:

> It should definitely go in -Wthread-safety-beta so it won't break the build.  
> Unfortunately, the before/after checks are still in thread-safety-beta, and 
> they should really be moved out of beta before something else is moved in.  
> The good news is that Matthew O'Niel just volunteered to do that.  That is, 
> unfortunately, not a trivial operation, so it may be some weeks before he's 
> done.


That's Ok for me. It's not something that I terribly need, but it seemed to 
make things more consistent.

Does the migration of the before/after checks include changes to how it is 
handled? Because I wondered why it doesn't work on S-expressions like the rest 
of the analysis, but just `ValueDecl`s. That leads to some false positives with 
more complex expressions:

  class A {
  public:
Mutex mu1;
Mutex mu2 ACQUIRED_AFTER(mu1);
  };
  
  class B {
A a1, a2;
Mutex lm ACQUIRED_AFTER(a1.mu2);
  
  public:
void f() {
  a2.mu2.Lock();
  a1.mu1.Lock();// warns "mutex 'mu1' must be acquired before 'mu2'", 
but they are on different objects
  a1.mu1.Unlock();
  a2.mu2.Unlock();
}
  
void g() {
  lm.Lock();
  a1.mu1.Lock();
  a1.mu1.Unlock();
  a2.mu1.Lock();// Warns "mutex 'mu1' must be acquired before 'lm'", 
but lm talks only about a1.mu2.
  a2.mu1.Unlock();
  lm.Unlock();
}
  };

But maybe that's not the right place to discuss this.


Repository:
  rC Clang

https://reviews.llvm.org/D51187



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

r342823 - Eliminate some unneeded signed/unsigned conversions

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Sat Sep 22 14:56:16 2018
New Revision: 342823

URL: http://llvm.org/viewvc/llvm-project?rev=342823&view=rev
Log:
Eliminate some unneeded signed/unsigned conversions

No functional change is intended, but generally this should be a bit
more safe.

Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp

Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h?rev=342823&r1=342822&r2=342823&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h Sat Sep 22 
14:56:16 2018
@@ -1643,10 +1643,10 @@ private:
   friend class SCFG;
 
   // assign unique ids to all instructions
-  int renumberInstrs(int id);
+  unsigned renumberInstrs(unsigned id);
 
-  int topologicalSort(SimpleArray &Blocks, int ID);
-  int topologicalFinalSort(SimpleArray &Blocks, int ID);
+  unsigned topologicalSort(SimpleArray &Blocks, unsigned ID);
+  unsigned topologicalFinalSort(SimpleArray &Blocks, unsigned 
ID);
   void computeDominator();
   void computePostDominator();
 
@@ -1657,7 +1657,7 @@ private:
   SCFG *CFGPtr = nullptr;
 
   // Unique ID for this BB in the containing CFG. IDs are in topological order.
-  int BlockID : 31;
+  unsigned BlockID : 31;
 
   // Bit to determine if a block has been visited during a traversal.
   bool Visited : 1;

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=342823&r1=342822&r2=342823&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Sat Sep 22 14:56:16 2018
@@ -730,7 +730,7 @@ void LocalVariableMap::traverseCFG(CFG *
   CtxIndices.resize(CFGraph->getNumBlockIDs());
 
   for (const auto *CurrBlock : *SortedGraph) {
-int CurrBlockID = CurrBlock->getBlockID();
+unsigned CurrBlockID = CurrBlock->getBlockID();
 CFGBlockInfo *CurrBlockInfo = &BlockInfo[CurrBlockID];
 
 VisitedBlocks.insert(CurrBlock);
@@ -746,7 +746,7 @@ void LocalVariableMap::traverseCFG(CFG *
 continue;
   }
 
-  int PrevBlockID = (*PI)->getBlockID();
+  unsigned PrevBlockID = (*PI)->getBlockID();
   CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
 
   if (CtxInit) {
@@ -2302,7 +2302,7 @@ void ThreadSafetyAnalyzer::runAnalysis(A
   }
 
   for (const auto *CurrBlock : *SortedGraph) {
-int CurrBlockID = CurrBlock->getBlockID();
+unsigned CurrBlockID = CurrBlock->getBlockID();
 CFGBlockInfo *CurrBlockInfo = &BlockInfo[CurrBlockID];
 
 // Use the default initial lockset in case there are no predecessors.
@@ -2329,7 +2329,7 @@ void ThreadSafetyAnalyzer::runAnalysis(A
   if (*PI == nullptr || !VisitedBlocks.alreadySet(*PI))
 continue;
 
-  int PrevBlockID = (*PI)->getBlockID();
+  unsigned PrevBlockID = (*PI)->getBlockID();
   CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
 
   // Ignore edges from blocks that can't return.
@@ -2370,7 +2370,7 @@ void ThreadSafetyAnalyzer::runAnalysis(A
 // Process continue and break blocks. Assume that the lockset for the
 // resulting block is unaffected by any discrepancies in them.
 for (const auto *PrevBlock : SpecialBlocks) {
-  int PrevBlockID = PrevBlock->getBlockID();
+  unsigned PrevBlockID = PrevBlock->getBlockID();
   CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
 
   if (!LocksetInitialized) {

Modified: cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp?rev=342823&r1=342822&r2=342823&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp Sat Sep 22 14:56:16 2018
@@ -150,7 +150,7 @@ void til::simplifyIncompleteArg(til::Phi
 }
 
 // Renumbers the arguments and instructions to have unique, sequential IDs.
-int BasicBlock::renumberInstrs(int ID) {
+unsigned BasicBlock::renumberInstrs(unsigned ID) {
   for (auto *Arg : Args)
 Arg->setID(this, ID++);
   for (auto *Instr : Instrs)
@@ -163,7 +163,8 @@ int BasicBlock::renumberInstrs(int ID) {
 // Each block will be written into the Blocks array in order, and its BlockID
 // will be set to the index in the array.  Sorting should start from the entry
 // block, and ID should be the total number of blocks.
-int BasicBlock::topologicalSort(SimpleArray &Blocks, int ID) {
+unsigned BasicBlock::topologicalSort(SimpleArray &Blocks,
+ unsi

[PATCH] D52397: [libc++] Remove Fuchsia-specific knowledge to pick the ABI version

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added reviewers: phosek, EricWF.
Herald added subscribers: libcxx-commits, cfe-commits, dexonsmith, christof, 
mgorny.

The ABI version used by libc++ is a configuration option just like any other
configuration option. It is a knob that can be used by vendors to customize
the libc++ that they ship. As such, we should not be hardcoding vendor-specific
configuration choices in libc++.

When building libc++ for Fuchsia, Fuchsia's build scripts should simply define
the libc++ ABI version to 2 -- this will result in the _LIBCPP_ABI_VERSION
macro being defined in the __config header that is generated when libc++ is
built and installed, which is the correct way to customize libc++'s behavior
for specific vendors.


Repository:
  rCXX libc++

https://reviews.llvm.org/D52397

Files:
  libcxx/CMakeLists.txt
  libcxx/include/__config


Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -36,11 +36,7 @@
 #define _LIBCPP_VERSION 8000
 
 #ifndef _LIBCPP_ABI_VERSION
-#  ifdef __Fuchsia__
-#define _LIBCPP_ABI_VERSION 2
-#  else
-#define _LIBCPP_ABI_VERSION 1
-#  endif
+#  define _LIBCPP_ABI_VERSION 1
 #endif
 
 #ifndef _LIBCPP_STD_VER
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -111,12 +111,7 @@
 "Install libc++fs.a" ON
 "LIBCXX_ENABLE_FILESYSTEM;LIBCXX_INSTALL_LIBRARY" OFF)
 
-if (FUCHSIA)
-  set(DEFAULT_ABI_VERSION 2)
-else()
-  set(DEFAULT_ABI_VERSION 1)
-endif()
-set(LIBCXX_ABI_VERSION ${DEFAULT_ABI_VERSION} CACHE STRING "ABI version of 
libc++.")
+set(LIBCXX_ABI_VERSION "1" CACHE STRING "ABI version of libc++. Can be either 
1 or 2, where 2 is the unstable version of the ABI. Defaults to 1.")
 option(LIBCXX_ABI_UNSTABLE "Unstable ABI of libc++." OFF)
 option(LIBCXX_ABI_FORCE_ITANIUM "Ignore auto-detection and force use of the 
Itanium ABI.")
 option(LIBCXX_ABI_FORCE_MICROSOFT "Ignore auto-detection and force use of the 
Microsoft ABI.")
@@ -657,7 +652,7 @@
 endif()
 
 # Configuration file flags 
=
-if (NOT LIBCXX_ABI_VERSION EQUAL DEFAULT_ABI_VERSION)
+if (NOT LIBCXX_ABI_VERSION EQUAL 1)
   config_define(${LIBCXX_ABI_VERSION} _LIBCPP_ABI_VERSION)
 endif()
 config_define_if(LIBCXX_ABI_UNSTABLE _LIBCPP_ABI_UNSTABLE)


Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -36,11 +36,7 @@
 #define _LIBCPP_VERSION 8000
 
 #ifndef _LIBCPP_ABI_VERSION
-#  ifdef __Fuchsia__
-#define _LIBCPP_ABI_VERSION 2
-#  else
-#define _LIBCPP_ABI_VERSION 1
-#  endif
+#  define _LIBCPP_ABI_VERSION 1
 #endif
 
 #ifndef _LIBCPP_STD_VER
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -111,12 +111,7 @@
 "Install libc++fs.a" ON
 "LIBCXX_ENABLE_FILESYSTEM;LIBCXX_INSTALL_LIBRARY" OFF)
 
-if (FUCHSIA)
-  set(DEFAULT_ABI_VERSION 2)
-else()
-  set(DEFAULT_ABI_VERSION 1)
-endif()
-set(LIBCXX_ABI_VERSION ${DEFAULT_ABI_VERSION} CACHE STRING "ABI version of libc++.")
+set(LIBCXX_ABI_VERSION "1" CACHE STRING "ABI version of libc++. Can be either 1 or 2, where 2 is the unstable version of the ABI. Defaults to 1.")
 option(LIBCXX_ABI_UNSTABLE "Unstable ABI of libc++." OFF)
 option(LIBCXX_ABI_FORCE_ITANIUM "Ignore auto-detection and force use of the Itanium ABI.")
 option(LIBCXX_ABI_FORCE_MICROSOFT "Ignore auto-detection and force use of the Microsoft ABI.")
@@ -657,7 +652,7 @@
 endif()
 
 # Configuration file flags =
-if (NOT LIBCXX_ABI_VERSION EQUAL DEFAULT_ABI_VERSION)
+if (NOT LIBCXX_ABI_VERSION EQUAL 1)
   config_define(${LIBCXX_ABI_VERSION} _LIBCPP_ABI_VERSION)
 endif()
 config_define_if(LIBCXX_ABI_UNSTABLE _LIBCPP_ABI_UNSTABLE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342825 - [Index] Fix header guard naming

[Fangrui Song via cfe-commits]

Author: maskray
Date: Sat Sep 22 15:49:38 2018
New Revision: 342825

URL: http://llvm.org/viewvc/llvm-project?rev=342825&view=rev
Log:
[Index] Fix header guard naming

Modified:
cfe/trunk/include/clang/Index/USRGeneration.h

Modified: cfe/trunk/include/clang/Index/USRGeneration.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/USRGeneration.h?rev=342825&r1=342824&r2=342825&view=diff
==
--- cfe/trunk/include/clang/Index/USRGeneration.h (original)
+++ cfe/trunk/include/clang/Index/USRGeneration.h Sat Sep 22 15:49:38 2018
@@ -90,5 +90,5 @@ bool generateUSRFragmentForModuleName(St
 } // namespace index
 } // namespace clang
 
-#endif // LLVM_CLANG_IDE_USRGENERATION_H
+#endif // LLVM_CLANG_INDEX_USRGENERATION_H
 


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

[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.

[Kristina Brooks via Phabricator via cfe-commits]

kristina marked 3 inline comments as done.
kristina added a comment.

Want to see what @rnk has to say about this before landing it since he wrote 
the original code and if my understanding of `common` vs `dso_local` is 
accurate or not since I don't have much experience with the Windows specific 
metadata, so I'll likely land it on Monday if he's happy with it. I kept 
`cfstring` as is, without the dot as per comment and now ELF and PE/COFF share 
the same paths sans the `DLLImport/DLLExport` part.


Repository:
  rC Clang

https://reviews.llvm.org/D52344



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

[PATCH] D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley.
Herald added subscribers: cfe-commits, kristina.

When people are really sure they'll get the lock they sometimes use
__builtin_expect. It's also used by some assertion implementations.
Asserting that try-lock succeeded is basically the same as asserting
that the lock is not held by anyone else (and acquiring it).


Repository:
  rC Clang

https://reviews.llvm.org/D52398

Files:
  lib/Analysis/ThreadSafety.cpp
  test/SemaCXX/warn-thread-safety-analysis.cpp


Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1754,14 +1754,29 @@
 mu.Unlock();
   }
 
+  void foo2_builtin_expect() {
+if (__builtin_expect(!mu.TryLock(), false))
+  return;
+a = 2;
+mu.Unlock();
+  }
+
   void foo3() {
 bool b = mu.TryLock();
 if (b) {
   a = 3;
   mu.Unlock();
 }
   }
 
+  void foo3_builtin_expect() {
+bool b = mu.TryLock();
+if (__builtin_expect(b, true)) {
+  a = 3;
+  mu.Unlock();
+}
+  }
+
   void foo4() {
 bool b = mu.TryLock();
 if (!b) return;
Index: lib/Analysis/ThreadSafety.cpp
===
--- lib/Analysis/ThreadSafety.cpp
+++ lib/Analysis/ThreadSafety.cpp
@@ -33,6 +33,7 @@
 #include "clang/Analysis/Analyses/ThreadSafetyUtil.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
@@ -1388,8 +1389,11 @@
   if (!Cond)
 return nullptr;
 
-  if (const auto *CallExp = dyn_cast(Cond))
+  if (const auto *CallExp = dyn_cast(Cond)) {
+if (CallExp->getBuiltinCallee() == Builtin::BI__builtin_expect)
+  return getTrylockCallExpr(CallExp->getArg(0), C, Negate);
 return CallExp;
+  }
   else if (const auto *PE = dyn_cast(Cond))
 return getTrylockCallExpr(PE->getSubExpr(), C, Negate);
   else if (const auto *CE = dyn_cast(Cond))


Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1754,14 +1754,29 @@
 mu.Unlock();
   }
 
+  void foo2_builtin_expect() {
+if (__builtin_expect(!mu.TryLock(), false))
+  return;
+a = 2;
+mu.Unlock();
+  }
+
   void foo3() {
 bool b = mu.TryLock();
 if (b) {
   a = 3;
   mu.Unlock();
 }
   }
 
+  void foo3_builtin_expect() {
+bool b = mu.TryLock();
+if (__builtin_expect(b, true)) {
+  a = 3;
+  mu.Unlock();
+}
+  }
+
   void foo4() {
 bool b = mu.TryLock();
 if (!b) return;
Index: lib/Analysis/ThreadSafety.cpp
===
--- lib/Analysis/ThreadSafety.cpp
+++ lib/Analysis/ThreadSafety.cpp
@@ -33,6 +33,7 @@
 #include "clang/Analysis/Analyses/ThreadSafetyUtil.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
@@ -1388,8 +1389,11 @@
   if (!Cond)
 return nullptr;
 
-  if (const auto *CallExp = dyn_cast(Cond))
+  if (const auto *CallExp = dyn_cast(Cond)) {
+if (CallExp->getBuiltinCallee() == Builtin::BI__builtin_expect)
+  return getTrylockCallExpr(CallExp->getArg(0), C, Negate);
 return CallExp;
+  }
   else if (const auto *PE = dyn_cast(Cond))
 return getTrylockCallExpr(PE->getSubExpr(), C, Negate);
   else if (const auto *CE = dyn_cast(Cond))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In https://reviews.llvm.org/D45639#1242444, @phosek wrote:

> In https://reviews.llvm.org/D45639#1193112, @ldionne wrote:
>
> > @phosek I don't understand how you can expect code compiled with new 
> > headers to link against an old dylib, unless you're setting the target 
> > platform, in which case anything that would fail to link will instead be a 
> > compiler error. I mean, we're adding stuff to the dylib from one version to 
> > the next, so _of course_ it won't always work.
>
>
> @ldionne it's the other way round. Today, Clang driver on macOS automatically 
> uses headers that are part of the compiler distribution (i.e. 
> `-I../include/c++/v1`) but it always links against the system library 
> (`/usr/lib/libc++.dylib`) because the library path to libraries that are 
> shipped with the compiler (`-L../lib`) is never added to the link-line. Those 
> two may not necessarily be the same version, and almost certainly won't be if 
> you use Clang build from trunk.


Sorry, my comment was wrong. You're right, using new libc++ headers and linking 
against an old `libc++.dylib` is a supported use case, and in fact this is 
exactly what happens whenever you use new libc++ headers and link against the 
system-provided `libc++.dylib` on macOS. However, what is _not_ supported is 
linking against a new `libc++.dylib` and then trying to run using the system's 
`libc++.dylib`, which may be older.

If you add `-L/../lib`, will you start linking against the 
Clang-provided `libc++.dylib`? If so, and if you run the resulting application 
without setting the `DYLD_LIBRARY_PATH` to include the Clang-provided 
`libc++.dylib`, your program won't run because the system `libc++.dylib` may 
not include all symbols that the newer Clang-provided `libc++.dylib` contains.

> We hit this case again recently where developers are trying to use libc++ 
> filesystem library. Using `#include ` just works but `-lc++fs` 
> fails because that library is not shipped as part of macOS at the moment, so 
> developers need to add explicit `-L/../lib` on macOS. This 
> is not the case on other platforms such as Linux.

Not shipping filesystem on macOS is a design choice we're making because the 
filesystem library is not ABI stable. Adding `-L/../lib` 
explicitly shows that you understand you're doing something unusual (and not 
officially supported), which is good.

I assume this does not happen on Linux because Linux distributions must include 
`libc++fs.a` as part of their system -- is that really the case?

> The purpose of this patch is to make the driver behave more consistently 
> across platforms so developers who build their own Clang don't need to 
> explicitly pass `-L/../lib` on macOS.

Thanks for the good explanation -- now I understand the purpose of the patch. 
However, I think we need a larger discussion around how libc++ is shipped to 
users and what use cases we want to support. For example, one question I have 
is why we're even shipping `libc++.dylib`, `libc++abi.dylib` and 
`libunwind.dylib` in our LLVM releases for MacOS, given they are provided by 
the system (and mixing them is a recipe for disaster). Another question is 
whether the LLVM-provided Clang should instead always link to the LLVM-provided 
libraries (which would require users setting the `DYLD_LIBRARY_PATH` properly 
to avoid falling back onto the macOS-provided `libc++.dylib`).

I'm quite sympathetic to your use case (and in fact we have similar use cases), 
but I'm uncomfortable moving forward with this patch until we have a better 
understanding of some important surrounding questions. I'd like to talk with 
@dexonsmith about it and then maybe we can meet at the LLVM Dev Meeting (if you 
plan to attend) with other libc++ people to flesh those questions out?


Repository:
  rC Clang

https://reviews.llvm.org/D45639



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

[PATCH] D52397: [libc++] Remove Fuchsia-specific knowledge to pick the ABI version

[Louis Dionne via Phabricator via cfe-commits]

ldionne updated this revision to Diff 166621.
ldionne added a comment.

Clarify that version 2 of the ABI is _currently_ unstable, it is not _the_ 
unstable version.


Repository:
  rCXX libc++

https://reviews.llvm.org/D52397

Files:
  libcxx/CMakeLists.txt
  libcxx/include/__config


Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -36,11 +36,7 @@
 #define _LIBCPP_VERSION 8000
 
 #ifndef _LIBCPP_ABI_VERSION
-#  ifdef __Fuchsia__
-#define _LIBCPP_ABI_VERSION 2
-#  else
-#define _LIBCPP_ABI_VERSION 1
-#  endif
+#  define _LIBCPP_ABI_VERSION 1
 #endif
 
 #ifndef _LIBCPP_STD_VER
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -111,12 +111,7 @@
 "Install libc++fs.a" ON
 "LIBCXX_ENABLE_FILESYSTEM;LIBCXX_INSTALL_LIBRARY" OFF)
 
-if (FUCHSIA)
-  set(DEFAULT_ABI_VERSION 2)
-else()
-  set(DEFAULT_ABI_VERSION 1)
-endif()
-set(LIBCXX_ABI_VERSION ${DEFAULT_ABI_VERSION} CACHE STRING "ABI version of 
libc++.")
+set(LIBCXX_ABI_VERSION "1" CACHE STRING "ABI version of libc++. Can be either 
1 or 2, where 2 is currently not stable. Defaults to 1.")
 option(LIBCXX_ABI_UNSTABLE "Unstable ABI of libc++." OFF)
 option(LIBCXX_ABI_FORCE_ITANIUM "Ignore auto-detection and force use of the 
Itanium ABI.")
 option(LIBCXX_ABI_FORCE_MICROSOFT "Ignore auto-detection and force use of the 
Microsoft ABI.")
@@ -657,7 +652,7 @@
 endif()
 
 # Configuration file flags 
=
-if (NOT LIBCXX_ABI_VERSION EQUAL DEFAULT_ABI_VERSION)
+if (NOT LIBCXX_ABI_VERSION EQUAL 1)
   config_define(${LIBCXX_ABI_VERSION} _LIBCPP_ABI_VERSION)
 endif()
 config_define_if(LIBCXX_ABI_UNSTABLE _LIBCPP_ABI_UNSTABLE)


Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -36,11 +36,7 @@
 #define _LIBCPP_VERSION 8000
 
 #ifndef _LIBCPP_ABI_VERSION
-#  ifdef __Fuchsia__
-#define _LIBCPP_ABI_VERSION 2
-#  else
-#define _LIBCPP_ABI_VERSION 1
-#  endif
+#  define _LIBCPP_ABI_VERSION 1
 #endif
 
 #ifndef _LIBCPP_STD_VER
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -111,12 +111,7 @@
 "Install libc++fs.a" ON
 "LIBCXX_ENABLE_FILESYSTEM;LIBCXX_INSTALL_LIBRARY" OFF)
 
-if (FUCHSIA)
-  set(DEFAULT_ABI_VERSION 2)
-else()
-  set(DEFAULT_ABI_VERSION 1)
-endif()
-set(LIBCXX_ABI_VERSION ${DEFAULT_ABI_VERSION} CACHE STRING "ABI version of libc++.")
+set(LIBCXX_ABI_VERSION "1" CACHE STRING "ABI version of libc++. Can be either 1 or 2, where 2 is currently not stable. Defaults to 1.")
 option(LIBCXX_ABI_UNSTABLE "Unstable ABI of libc++." OFF)
 option(LIBCXX_ABI_FORCE_ITANIUM "Ignore auto-detection and force use of the Itanium ABI.")
 option(LIBCXX_ABI_FORCE_MICROSOFT "Ignore auto-detection and force use of the Microsoft ABI.")
@@ -657,7 +652,7 @@
 endif()
 
 # Configuration file flags =
-if (NOT LIBCXX_ABI_VERSION EQUAL DEFAULT_ABI_VERSION)
+if (NOT LIBCXX_ABI_VERSION EQUAL 1)
   config_define(${LIBCXX_ABI_VERSION} _LIBCPP_ABI_VERSION)
 endif()
 config_define_if(LIBCXX_ABI_UNSTABLE _LIBCPP_ABI_UNSTABLE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47395: [libcxx] [test] Remove nonportable locale assumption in basic.ios.members/narrow.pass.cpp

[Louis Dionne via Phabricator via cfe-commits]

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

LGTM and has been committed -- you can close this.


https://reviews.llvm.org/D47395



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

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

I think on Darwin it would **not** make sense to have `libc++fs.a` ship in 
`libc++.dylib` especially considering that it ends up in the dyld cache and 
that has a lot of other implications. It would make sense to ship it as a 
separate library, perhaps as part of the SDK, at least for now. As far as 
making it a system dylib, it's a possibility as long as no core os components 
depend on it and it's there solely for consumers.


Repository:
  rC Clang

https://reviews.llvm.org/D45639



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

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In https://reviews.llvm.org/D45639#1243017, @kristina wrote:

> I think on Darwin it would **not** make sense to have `libc++fs.a` ship in 
> `libc++.dylib` especially considering that it ends up in the dyld cache and 
> that has a lot of other implications. It would make sense to ship it as a 
> separate library, perhaps as part of the SDK, at least for now.


Indeed, this makes a lot of sense and this is exactly what I was thinking 
about. I'd like to get a more holistic plan before we do that though.

> As far as making it a system dylib, it's a possibility as long as no core os 
> components depend on it and it's there solely for consumers.

We can't do that unless that dylib is ABI-stable. Otherwise, imagine what 
happens when we update the OS (which would then contain a new version of that 
dylib): all consumers that link against that dylib will now crash at runtime 
because we'll have broken its ABI.


Repository:
  rC Clang

https://reviews.llvm.org/D45639



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

r342827 - Fix modules build with shared library.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Sat Sep 22 17:48:05 2018
New Revision: 342827

URL: http://llvm.org/viewvc/llvm-project?rev=342827&view=rev
Log:
Fix modules build with shared library.

r341994 caused clangAnalysis to pull all of the AST matchers
library into clang. Due to inline key functions in the headers,
importing the AST matchers library gives a link dependency on the
AST matchers (and thus the AST), which clang should not
have.

This patch works around the issues by excluding the offending
libclangAnalysis header in the modulemap.

Modified:
cfe/trunk/include/clang/module.modulemap

Modified: cfe/trunk/include/clang/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/module.modulemap?rev=342827&r1=342826&r2=342827&view=diff
==
--- cfe/trunk/include/clang/module.modulemap (original)
+++ cfe/trunk/include/clang/module.modulemap Sat Sep 22 17:48:05 2018
@@ -5,6 +5,12 @@ module Clang_Analysis {
   textual header "Analysis/Analyses/ThreadSafetyOps.def"
 
   module * { export * }
+
+  // FIXME: Exclude these headers to avoid pulling all of the AST matchers
+  // library into clang. Due to inline key functions in the headers,
+  // importing the AST matchers library gives a link dependency on the AST
+  // matchers (and thus the AST), which clang-format should not have.
+  exclude header "Analysis/Analyses/ExprMutationAnalyzer.h"
 }
 
 module Clang_AST {


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

[PATCH] D52399: [AArch64] Support adding X[8-15, 18] registers as CSRs.

[Tri Vo via Phabricator via cfe-commits]

trong created this revision.
trong added reviewers: srhines, nickdesaulniers.
Herald added a reviewer: javed.absar.
Herald added subscribers: jfb, kristof.beyls.

Making X[8-15,18] registers call-saved is used to support
CONFIG_ARM64_LSE_ATOMICS in Linux kernel.

Signed-off-by: Tri Vo 


Repository:
  rC Clang

https://reviews.llvm.org/D52399

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Arch/AArch64.cpp
  test/Driver/aarch64-call-saved-x-register.c

Index: test/Driver/aarch64-call-saved-x-register.c
===
--- /dev/null
+++ test/Driver/aarch64-call-saved-x-register.c
@@ -0,0 +1,58 @@
+// RUN: %clang -target aarch64-none-gnu -fcall-saved-x8 -### %s  2>&1  \
+// RUN: | FileCheck --check-prefix=CHECK-CALL-SAVED-X8 %s
+
+// RUN: %clang -target aarch64-none-gnu -fcall-saved-x9 -### %s  2>&1  \
+// RUN: | FileCheck --check-prefix=CHECK-CALL-SAVED-X9 %s
+
+// RUN: %clang -target aarch64-none-gnu -fcall-saved-x10 -### %s  2>&1  \
+// RUN: | FileCheck --check-prefix=CHECK-CALL-SAVED-X10 %s
+
+// RUN: %clang -target aarch64-none-gnu -fcall-saved-x11 -### %s  2>&1  \
+// RUN: | FileCheck --check-prefix=CHECK-CALL-SAVED-X11 %s
+
+// RUN: %clang -target aarch64-none-gnu -fcall-saved-x12 -### %s  2>&1  \
+// RUN: | FileCheck --check-prefix=CHECK-CALL-SAVED-X12 %s
+
+// RUN: %clang -target aarch64-none-gnu -fcall-saved-x13 -### %s  2>&1  \
+// RUN: | FileCheck --check-prefix=CHECK-CALL-SAVED-X13 %s
+
+// RUN: %clang -target aarch64-none-gnu -fcall-saved-x14 -### %s  2>&1  \
+// RUN: | FileCheck --check-prefix=CHECK-CALL-SAVED-X14 %s
+
+// RUN: %clang -target aarch64-none-gnu -fcall-saved-x15 -### %s  2>&1  \
+// RUN: | FileCheck --check-prefix=CHECK-CALL-SAVED-X15 %s
+
+// RUN: %clang -target aarch64-none-gnu -fcall-saved-x18 -### %s  2>&1  \
+// RUN: | FileCheck --check-prefix=CHECK-CALL-SAVED-X18 %s
+
+// Test all call-saved-x# options together.
+// RUN: %clang -target aarch64-none-gnu \
+// RUN: -fcall-saved-x8 \
+// RUN: -fcall-saved-x9 \
+// RUN: -fcall-saved-x10 \
+// RUN: -fcall-saved-x11 \
+// RUN: -fcall-saved-x12 \
+// RUN: -fcall-saved-x13 \
+// RUN: -fcall-saved-x14 \
+// RUN: -fcall-saved-x15 \
+// RUN: -fcall-saved-x18 \
+// RUN: -### %s  2>&1 | FileCheck %s \
+// RUN: --check-prefix=CHECK-CALL-SAVED-X8 \
+// RUN: --check-prefix=CHECK-CALL-SAVED-X9 \
+// RUN: --check-prefix=CHECK-CALL-SAVED-X10 \
+// RUN: --check-prefix=CHECK-CALL-SAVED-X11 \
+// RUN: --check-prefix=CHECK-CALL-SAVED-X12 \
+// RUN: --check-prefix=CHECK-CALL-SAVED-X13 \
+// RUN: --check-prefix=CHECK-CALL-SAVED-X14 \
+// RUN: --check-prefix=CHECK-CALL-SAVED-X15 \
+// RUN: --check-prefix=CHECK-CALL-SAVED-X18
+
+// CHECK-CALL-SAVED-X8: "-target-feature" "+call-saved-x8"
+// CHECK-CALL-SAVED-X9: "-target-feature" "+call-saved-x9"
+// CHECK-CALL-SAVED-X10: "-target-feature" "+call-saved-x10"
+// CHECK-CALL-SAVED-X11: "-target-feature" "+call-saved-x11"
+// CHECK-CALL-SAVED-X12: "-target-feature" "+call-saved-x12"
+// CHECK-CALL-SAVED-X13: "-target-feature" "+call-saved-x13"
+// CHECK-CALL-SAVED-X14: "-target-feature" "+call-saved-x14"
+// CHECK-CALL-SAVED-X15: "-target-feature" "+call-saved-x15"
+// CHECK-CALL-SAVED-X18: "-target-feature" "+call-saved-x18"
Index: lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- lib/Driver/ToolChains/Arch/AArch64.cpp
+++ lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -225,6 +225,33 @@
   if (Args.hasArg(options::OPT_ffixed_x20))
 Features.push_back("+reserve-x20");
 
+  if (Args.hasArg(options::OPT_fcall_saved_x8))
+Features.push_back("+call-saved-x8");
+
+  if (Args.hasArg(options::OPT_fcall_saved_x9))
+Features.push_back("+call-saved-x9");
+
+  if (Args.hasArg(options::OPT_fcall_saved_x10))
+Features.push_back("+call-saved-x10");
+
+  if (Args.hasArg(options::OPT_fcall_saved_x11))
+Features.push_back("+call-saved-x11");
+
+  if (Args.hasArg(options::OPT_fcall_saved_x12))
+Features.push_back("+call-saved-x12");
+
+  if (Args.hasArg(options::OPT_fcall_saved_x13))
+Features.push_back("+call-saved-x13");
+
+  if (Args.hasArg(options::OPT_fcall_saved_x14))
+Features.push_back("+call-saved-x14");
+
+  if (Args.hasArg(options::OPT_fcall_saved_x15))
+Features.push_back("+call-saved-x15");
+
+  if (Args.hasArg(options::OPT_fcall_saved_x18))
+Features.push_back("+call-saved-x18");
+
   if (Args.hasArg(options::OPT_mno_neg_immediates))
 Features.push_back("+no-neg-immediates");
 }
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2054,6 +2054,10 @@
   def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group,
 HelpText<"Reserve the "#i#" register (AArch64 only)">;
 
+foreach i = {8-15,18} in
+  def fcall_saved_x#i : Flag<["-"], "fcall-saved-x"#i>, Group,
+HelpText<"Make the x"#

[PATCH] D52400: Improve -Wshadow warnings with enumerators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, dblaikie.

Currently, we do not check for enumerators that shadow other enumerators as 
part of -Wshadow, but gcc does provide such a diagnostic for this case.  This 
is intended to catch shadowing issues like:

  enum E1{e1};
  void f(void) {
enum E2{e1};
  }

This patch addresses PR24718.


https://reviews.llvm.org/D52400

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-shadow.c


Index: test/Sema/warn-shadow.c
===
--- test/Sema/warn-shadow.c
+++ test/Sema/warn-shadow.c
@@ -59,3 +59,8 @@
 void test8() {
   int bob; // expected-warning {{declaration shadows a variable in the global 
scope}}
 }
+
+enum PR24718_1{pr24718}; // expected-note {{previous declaration is here}}
+void PR24718(void) {
+  enum PR24718_2{pr24718}; // expected-warning {{declaration shadows a 
variable in the global scope}}
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -16290,8 +16290,10 @@
 
   // Verify that there isn't already something declared with this name in this
   // scope.
-  NamedDecl *PrevDecl = LookupSingleName(S, Id, IdLoc, LookupOrdinaryName,
- ForVisibleRedeclaration);
+  LookupResult R(*this, Id, IdLoc, LookupOrdinaryName, 
ForVisibleRedeclaration);
+  LookupName(R, S);
+  NamedDecl *PrevDecl = R.getAsSingle();
+
   if (PrevDecl && PrevDecl->isTemplateParameter()) {
 // Maybe we will complain about the shadowed template parameter.
 DiagnoseTemplateParameterShadow(IdLoc, PrevDecl);
@@ -16314,6 +16316,9 @@
 return nullptr;
 
   if (PrevDecl) {
+// Check for other kinds of shadowing not already handled.
+CheckShadow(New, PrevDecl, R);
+
 // When in C++, we may get a TagDecl with the same name; in this case the
 // enum constant will 'hide' the tag.
 assert((getLangOpts().CPlusPlus || !isa(PrevDecl)) &&


Index: test/Sema/warn-shadow.c
===
--- test/Sema/warn-shadow.c
+++ test/Sema/warn-shadow.c
@@ -59,3 +59,8 @@
 void test8() {
   int bob; // expected-warning {{declaration shadows a variable in the global scope}}
 }
+
+enum PR24718_1{pr24718}; // expected-note {{previous declaration is here}}
+void PR24718(void) {
+  enum PR24718_2{pr24718}; // expected-warning {{declaration shadows a variable in the global scope}}
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -16290,8 +16290,10 @@
 
   // Verify that there isn't already something declared with this name in this
   // scope.
-  NamedDecl *PrevDecl = LookupSingleName(S, Id, IdLoc, LookupOrdinaryName,
- ForVisibleRedeclaration);
+  LookupResult R(*this, Id, IdLoc, LookupOrdinaryName, ForVisibleRedeclaration);
+  LookupName(R, S);
+  NamedDecl *PrevDecl = R.getAsSingle();
+
   if (PrevDecl && PrevDecl->isTemplateParameter()) {
 // Maybe we will complain about the shadowed template parameter.
 DiagnoseTemplateParameterShadow(IdLoc, PrevDecl);
@@ -16314,6 +16316,9 @@
 return nullptr;
 
   if (PrevDecl) {
+// Check for other kinds of shadowing not already handled.
+CheckShadow(New, PrevDecl, R);
+
 // When in C++, we may get a TagDecl with the same name; in this case the
 // enum constant will 'hide' the tag.
 assert((getLangOpts().CPlusPlus || !isa(PrevDecl)) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

[David CARLIER via Phabricator via cfe-commits]

devnexen added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:275
+os << "sizeof(" << DstName << ")";
+  else
+os << "sizeof()";

MaskRay wrote:
> Why can't this `else if` case be folded into the `strlcpy` case? There are 
> lots of duplication.
> 
> `strlcpy` does not check `DstName.empty()` but this one does. Is there any 
> cases I am missing?
strlcpy does but agreed with your first statement, this handling case for both 
are more different than my initial plan defined them.


https://reviews.llvm.org/D49722



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

[PATCH] D52401: Remove redundant null pointer check in operator delete

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: EricWF, mclow.lists.
Herald added subscribers: libcxx-commits, cfe-commits, ldionne, christof.

If ptr is a null pointer, no action shall occur.


Repository:
  rCXX libc++

https://reviews.llvm.org/D52401

Files:
  src/new.cpp


Index: src/new.cpp
===
--- src/new.cpp
+++ src/new.cpp
@@ -135,8 +135,7 @@
 void
 operator delete(void* ptr) _NOEXCEPT
 {
-if (ptr)
-::free(ptr);
+::free(ptr);
 }
 
 _LIBCPP_WEAK


Index: src/new.cpp
===
--- src/new.cpp
+++ src/new.cpp
@@ -135,8 +135,7 @@
 void
 operator delete(void* ptr) _NOEXCEPT
 {
-if (ptr)
-::free(ptr);
+::free(ptr);
 }
 
 _LIBCPP_WEAK
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52401: Remove redundant null pointer check in operator delete

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 166625.
MaskRay edited the summary of this revision.
MaskRay added a comment.

.


Repository:
  rCXX libc++

https://reviews.llvm.org/D52401

Files:
  src/new.cpp


Index: src/new.cpp
===
--- src/new.cpp
+++ src/new.cpp
@@ -135,8 +135,7 @@
 void
 operator delete(void* ptr) _NOEXCEPT
 {
-if (ptr)
-::free(ptr);
+::free(ptr);
 }
 
 _LIBCPP_WEAK


Index: src/new.cpp
===
--- src/new.cpp
+++ src/new.cpp
@@ -135,8 +135,7 @@
 void
 operator delete(void* ptr) _NOEXCEPT
 {
-if (ptr)
-::free(ptr);
+::free(ptr);
 }
 
 _LIBCPP_WEAK
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

[David CARLIER via Phabricator via cfe-commits]

devnexen updated this revision to Diff 166626.

https://reviews.llvm.org/D49722

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
@@ -7,6 +7,7 @@
 char  *strncat(char *, const char *, size_t);
 size_t strlen (const char *s);
 size_t strlcpy(char *, const char *, size_t);
+size_t strlcat(char *, const char *, size_t);
 
 void testStrncat(const char *src) {
   char dest[10];
@@ -33,3 +34,21 @@
   strlcpy(dest + 5, src, 5);
   strlcpy(dest + 5, src, 10); // expected-warning {{The third argument is larger than the size of the input buffer.}}
 }
+
+void testStrlcat(const char *src) {
+  char dest[10];
+  size_t badlen = 20;
+  size_t ulen;
+  strlcpy(dest, "a", sizeof("a") - 1);
+  strlcat(dest, "", (sizeof("") - 1) - sizeof(dest) - 1);
+  strlcpy(dest, "012345678", sizeof(dest));
+  strlcat(dest, "910", sizeof(dest));
+  strlcpy(dest, "0123456789", sizeof(dest));
+  strlcpy(dest, "0123456789", sizeof(dest));
+  strlcat(dest, "0123456789", badlen / 2);
+  strlcat(dest, "0123456789", badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
+  strlcat(dest, "0123456789", badlen - strlen(dest) - 1);
+  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}}
+}
Index: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -90,7 +90,16 @@
   ///   strlcpy(dst, "abcd", 4);
   ///   strlcpy(dst + 3, "abcd", 2);
   ///   strlcpy(dst, "abcd", cpy);
-  bool containsBadStrlcpyPattern(const CallExpr *CE);
+  /// Identify erroneous patterns in the last argument to strlcat - the number
+  /// of bytes to copy.
+  /// The bad pattern checked is when the last argument is basically
+  /// pointing to the destination buffer size or argument larger or
+  /// equal to.
+  ///   char dst[2];
+  ///   strlcat(dst, src2, sizeof(dst));
+  ///   strlcat(dst, src2, 2);
+  ///   strlcat(dst, src2, 10);
+  bool containsBadStrlcpyStrlcatPattern(const CallExpr *CE);
 
 public:
   WalkAST(const CheckerBase *Checker, BugReporter &BR, AnalysisDeclContext *AC)
@@ -142,15 +151,19 @@
   return false;
 }
 
-bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) {
+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);
 
   const auto *DstArgDecl = dyn_cast(DstArg->IgnoreParenImpCasts());
   const auto *LenArgDecl = dyn_cast(LenArg->IgnoreParenLValueCasts());
   uint64_t DstOff = 0;
+  if (isSizeof(LenArg, DstArg))
+return false;
   // - size_t dstlen = sizeof(dst)
   if (LenArgDecl) {
 const auto *LenArgVal = dyn_cast(LenArgDecl->getDecl());
@@ -181,8 +194,14 @@
   if (const auto *Buffer = dyn_cast(DstArgDecl->getType())) {
 ASTContext &C = BR.getContext();
 uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
-if ((BufferLen - DstOff) < ILRawVal)
-  return true;
+auto RemainingBufferLen = BufferLen - DstOff;
+if (Append) {
+  if (RemainingBufferLen <= ILRawVal)
+return true;
+} else {
+  if (RemainingBufferLen < ILRawVal)
+return true;
+}
   }
 }
   }
@@ -219,8 +238,9 @@
  "C String API", os.str(), Loc,
  LenArg->getSourceRange());
 }
-  } else if (CheckerContext::isCLibraryFunction(FD, "strlcpy")) {
-if (containsBadStrlcpyPattern(CE)) {
+  } else if (CheckerContext::isCLibraryFunction(FD, "strlcpy") ||
+ CheckerContext::isCLibraryFunction(FD, "strlcat")) {
+if (containsBadStrlcpyStrlcatPattern(CE)) {
   const Expr *DstArg = CE->getArg(0);
   const Expr *LenArg = CE->getArg(2);
   PathDiagnosticLocation Loc =
@@ -230,13 +250,27 @@
 
   SmallString<256> S;
   llvm::raw_svector_ostream os(S);
-  os << "The third argument is larger than the size of the input buffer. ";
-  if (!DstName.empty())
-os << "Replace with the value 'sizeof(" << DstName << ")` or lower";
-
-  BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument",
- "C String API", os.str(), Loc,
- LenArg->getSourceRange());
+  if (CheckerContext::isCLibraryFunction(FD, "str