[libunwind] r319299 - [CMake] Use the variable from the right project in install-unwind

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Wed Nov 29 00:20:57 2017
New Revision: 319299

URL: http://llvm.org/viewvc/llvm-project?rev=319299&view=rev
Log:
[CMake] Use the variable from the right project in install-unwind

Modified:
libunwind/trunk/src/CMakeLists.txt

Modified: libunwind/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/CMakeLists.txt?rev=319299&r1=319298&r2=319299&view=diff
==
--- libunwind/trunk/src/CMakeLists.txt (original)
+++ libunwind/trunk/src/CMakeLists.txt Wed Nov 29 00:20:57 2017
@@ -143,5 +143,5 @@ if (NOT CMAKE_CONFIGURATION_TYPES AND LI
 DEPENDS unwind
 COMMAND "${CMAKE_COMMAND}"
 -DCMAKE_INSTALL_COMPONENT=unwind
--P "${LIBCXXABI_BINARY_DIR}/cmake_install.cmake")
+-P "${LIBUNWIND_BINARY_DIR}/cmake_install.cmake")
 endif()


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

[libunwind] r319300 - Support building libunwind as a DLL

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Wed Nov 29 00:21:12 2017
New Revision: 319300

URL: http://llvm.org/viewvc/llvm-project?rev=319300&view=rev
Log:
Support building libunwind as a DLL

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

Modified:
libunwind/trunk/CMakeLists.txt
libunwind/trunk/src/config.h

Modified: libunwind/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=319300&r1=319299&r2=319300&view=diff
==
--- libunwind/trunk/CMakeLists.txt (original)
+++ libunwind/trunk/CMakeLists.txt Wed Nov 29 00:21:12 2017
@@ -308,6 +308,11 @@ if (MSVC)
   add_definitions(-D_CRT_SECURE_NO_WARNINGS)
 endif()
 
+# Disable DLL annotations on Windows for static builds.
+if (WIN32 AND LIBUNWIND_ENABLE_STATIC AND NOT LIBUNWIND_ENABLE_SHARED)
+  add_definitions(-D_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+endif()
+
 
#===
 # Setup Source Code
 
#===

Modified: libunwind/trunk/src/config.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/config.h?rev=319300&r1=319299&r2=319300&view=diff
==
--- libunwind/trunk/src/config.h (original)
+++ libunwind/trunk/src/config.h Wed Nov 29 00:21:12 2017
@@ -50,9 +50,13 @@
   #define _LIBUNWIND_EXPORT
   #define _LIBUNWIND_HIDDEN
 #else
-  // FIXME: these macros are not correct for COFF targets
-  #define _LIBUNWIND_EXPORT __attribute__((visibility("default")))
-  #define _LIBUNWIND_HIDDEN __attribute__((visibility("hidden")))
+  #if !defined(__ELF__) && !defined(__MACH__)
+#define _LIBUNWIND_EXPORT __declspec(dllexport)
+#define _LIBUNWIND_HIDDEN
+  #else
+#define _LIBUNWIND_EXPORT __attribute__((visibility("default")))
+#define _LIBUNWIND_HIDDEN __attribute__((visibility("hidden")))
+  #endif
 #endif
 
 #if (defined(__APPLE__) && defined(__arm__)) || 
defined(__USING_SJLJ_EXCEPTIONS__)


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

[PATCH] D40594: [InstCombine] miscompile of __builtin_fmod

[Dmitry Venikov via Phabricator via cfe-commits]

Quolyk created this revision.

Motivation: https://bugs.llvm.org/show_bug.cgi?id=34870
I'm totally not sure this is correct


https://reviews.llvm.org/D40594

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtins.c


Index: test/CodeGen/builtins.c
===
--- test/CodeGen/builtins.c
+++ test/CodeGen/builtins.c
@@ -253,13 +253,16 @@
   volatile long double resld;
 
   resf = __builtin_fmodf(F,F);
-  // CHECK: frem float
+  // CHECK: call float @fmodf(float %
+  // CHECK-NOT: readnone
 
   resd = __builtin_fmod(D,D);
-  // CHECK: frem double
+  // CHECK: call double @fmod(double %
+  // CHECK-NOT: readnone
 
   resld = __builtin_fmodl(LD,LD);
-  // CHECK: frem x86_fp80
+  // CHECK: call x86_fp80 @fmodl(x86_fp80 %
+  // CHECK-NOT: readnone
 
   resf = __builtin_fabsf(F);
   resd = __builtin_fabs(D);
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -899,14 +899,6 @@
   case Builtin::BI__builtin_fabsl: {
 return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::fabs));
   }
-  case Builtin::BI__builtin_fmod:
-  case Builtin::BI__builtin_fmodf:
-  case Builtin::BI__builtin_fmodl: {
-Value *Arg1 = EmitScalarExpr(E->getArg(0));
-Value *Arg2 = EmitScalarExpr(E->getArg(1));
-Value *Result = Builder.CreateFRem(Arg1, Arg2, "fmod");
-return RValue::get(Result);
-  }
   case Builtin::BI__builtin_copysign:
   case Builtin::BI__builtin_copysignf:
   case Builtin::BI__builtin_copysignl: {


Index: test/CodeGen/builtins.c
===
--- test/CodeGen/builtins.c
+++ test/CodeGen/builtins.c
@@ -253,13 +253,16 @@
   volatile long double resld;
 
   resf = __builtin_fmodf(F,F);
-  // CHECK: frem float
+  // CHECK: call float @fmodf(float %
+  // CHECK-NOT: readnone
 
   resd = __builtin_fmod(D,D);
-  // CHECK: frem double
+  // CHECK: call double @fmod(double %
+  // CHECK-NOT: readnone
 
   resld = __builtin_fmodl(LD,LD);
-  // CHECK: frem x86_fp80
+  // CHECK: call x86_fp80 @fmodl(x86_fp80 %
+  // CHECK-NOT: readnone
 
   resf = __builtin_fabsf(F);
   resd = __builtin_fabs(D);
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -899,14 +899,6 @@
   case Builtin::BI__builtin_fabsl: {
 return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::fabs));
   }
-  case Builtin::BI__builtin_fmod:
-  case Builtin::BI__builtin_fmodf:
-  case Builtin::BI__builtin_fmodl: {
-Value *Arg1 = EmitScalarExpr(E->getArg(0));
-Value *Arg2 = EmitScalarExpr(E->getArg(1));
-Value *Result = Builder.CreateFRem(Arg1, Arg2, "fmod");
-return RValue::get(Result);
-  }
   case Builtin::BI__builtin_copysign:
   case Builtin::BI__builtin_copysignf:
   case Builtin::BI__builtin_copysignl: {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40543: Pass by reference NewQualifiedName in QualifiedRenameRule

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: include/clang/Tooling/Refactoring/Rename/RenamingAction.h:79
   QualifiedRenameRule(const NamedDecl *ND,
-  std::string NewQualifiedName)
+  const std::string &NewQualifiedName)
   : ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}

Quolyk wrote:
> hokein wrote:
> > Passing `std::string` object is fine here -- because we use std::move below 
> > to avoid the extra copy.
> > 
> > Is the warning caught by the clang-tidy check?
> Unfortunatelly, I haven't tested, but I believe it's checkcpp warning.
I think this is a false positive of the checkcpp.

There is a similar clang-tidy "performance-unnecessary-value-param" check, but 
I don't think the check will warn this case.




https://reviews.llvm.org/D40543



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

[PATCH] D40528: add new check to find NSError init invocation

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tidy/objc/AvoidNserrorInitCheck.cpp:31
+  diag(MatchedExpr->getLocStart(),
+   "use errorWithDomain:code:userInfo: to create a new NSError");
+}

not sure what's the best message here.

From apple's document:

> You create an error object either by allocating it and then initializing it 
> with the initWithDomain:code:userInfo: method of NSError or by using the 
> class factory method errorWithDomain:code:userInfo:.

errorWithDomain:code:userInfo: is not the only way to create an NSError.




Comment at: clang-tidy/objc/AvoidNserrorInitCheck.h:24
+/// http://clang.llvm.org/extra/clang-tidy/checks/objc-avoid-nserror-init.html
+class AvoidNserrorInitCheck : public ClangTidyCheck {
+ public:

I'd name it `AvoidNSErrorInitCheck`.



Comment at: docs/clang-tidy/checks/objc-avoid-nserror-init.rst:10
+``errorWithDomain:code:userInfo:`` to create new NSError objects instead
+of ``[NSError alloc] init]``. Otherwise it will lead to a warning message
+during compilation in Xcode.

Wizard wrote:
> hokein wrote:
> > What's the warning message in Xcode? I suspect whether there is a 
> > diagnostic flag in clang already. 
> It was discussed originally here 
> https://buganizer.corp.google.com/issues/62445078 I think
Thanks. Please don't include any internal links next time.

Looks like the error message 
(https://stackoverflow.com/questions/33720042/why-does-nserror-alloc-init-in-xcode-throw-an-error)
 is shown during runtime, instead of compilation. Could you please confirm it, 
and update the document here? 

> [NSError init] called; this results in an invalid NSError instance. It will 
> raise an exception in a future release. Please call 
> errorWithDomain:code:userInfo: or initWithDomain:code:userInfo:. This message 
> shown only once.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40528



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

[PATCH] D40564: [clangd] Simplify common JSON-parsing patterns in Protocol.

[Eric Liu via Phabricator via cfe-commits]

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

lgtm




Comment at: clangd/Protocol.cpp:56
+assert(*this && "Must check this is an object before calling parse()");
+if (const json::Expr *E = O->get(Prop)) {
+  return parse(*E, Out);

nit: no braces around one liners.



Comment at: clangd/Protocol.cpp:101
+  static bool parse(const json::Expr &E, llvm::Optional &Out) {
+if (E.asNull())
+  return true;

Should we set `Out` to `None` in this case?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40564



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

[PATCH] D40060: [clangd] Fuzzy match scorer

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/FuzzyMatch.cpp:118
+0x00, 0x00, 0x00, 0x00, // Control characters
+0xff, 0xff, 0xff, 0xff, // Punctuation
+0x55, 0x55, 0xf5, 0xff, // Numbers->Lower, more Punctuation.

I'm not sure if we care, but maybe we should treat `+`, `-` and other symbols 
that could be in operator names (e.g., `operator +`) differently for C++.
Could also make sense for other languages with overloaded operators.



Comment at: clangd/FuzzyMatch.cpp:244
+  // Forbidden: matching the first pattern character in the middle of a 
segment.
+  if (!P && WordRole[W] == Tail)
+return AwfulScore;

Maybe use `P == 0` instead? It's equivalent, but a bit easier to read if you 
think of `P` as an offset.
Totally subjective, though, it's fine to have it either way.



Comment at: clangd/FuzzyMatch.cpp:245
+  if (!P && WordRole[W] == Tail)
+return AwfulScore;
+  ScoreT S = 0;

Does it mean I will get no matches in the following situation?

`Items = [printf, scanf]`
`Pattern = f`

It may be a bit confusing, since I do have a match, even though is terrible and 
it's ok to put those items very low in the list.
A more real example is:
`Items = [fprintf, fscanf]`
`Pattern = print`

Would `fprintf` match in that case? I think it should.

Another important one:
`Items = [istream, ostream]`
`Pattern = stream`



Comment at: clangd/FuzzyMatch.h:31
+public:
+  // Characters beyond MaxPat are ignored.
+  FuzzyMatcher(llvm::StringRef Pattern);

Maybe move the truncating logic into the clients? The users of this code are 
certainly better suited to report warnings/reject requests that are too large.



Comment at: clangd/FuzzyMatch.h:53
+
+  int PatN, WordN;   // Length of pattern and word.
+  char Pat[MaxPat], Word[MaxWord];   // Raw pattern and word data.

Maybe we could split the data we store into two sections:
1. Pattern-specific data. Initialized on construction, never changed later.
2. Per-match data. Initialized per `match()` call.

Otherwise it is somewhat hard to certify whether everything is being 
initialized properly.


https://reviews.llvm.org/D40060



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

[PATCH] D40310: Restructure how we break tokens.

[Manuel Klimek via Phabricator via cfe-commits]

klimek updated this revision to Diff 124709.
klimek marked 4 inline comments as done.
klimek added a comment.

Address review comments.


https://reviews.llvm.org/D40310

Files:
  lib/Format/BreakableToken.cpp
  lib/Format/BreakableToken.h
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestComments.cpp

Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1096,11 +1096,12 @@
 }
 
 TEST_F(FormatTestComments, SplitsLongLinesInComments) {
+  // FIXME: Do we need to fix up the "  */" at the end?
+  // It doesn't look like any of our current logic triggers this.
   EXPECT_EQ("/* This is a long\n"
 " * comment that\n"
-" * doesn't\n"
-" * fit on one line.\n"
-" */",
+" * doesn't fit on\n"
+" * one line.  */",
 format("/* "
"This is a long "
"comment that "
@@ -2102,6 +2103,85 @@
   EXPECT_EQ("///", format(" ///  ", getLLVMStyleWithColumns(20)));
 }
 
+TEST_F(FormatTestComments, ReflowsCommentsPrecise) {
+  // FIXME: This assumes we do not continue compressing whitespace once we are
+  // in reflow mode. Consider compressing whitespace.
+
+  // Test that we stop reflowing precisely at the column limit.
+  // After reflowing, "// reflows into   foo" does not fit the column limit,
+  // so we compress the whitespace.
+  EXPECT_EQ("// some text that\n"
+"// reflows into foo\n",
+format("// some text that reflows\n"
+   "// into   foo\n",
+   getLLVMStyleWithColumns(20)));
+  // Given one more column, "// reflows into   foo" does fit the limit, so we
+  // do not compress the whitespace.
+  EXPECT_EQ("// some text that\n"
+"// reflows into   foo\n",
+format("// some text that reflows\n"
+   "// into   foo\n",
+   getLLVMStyleWithColumns(21)));
+
+  // Make sure that we correctly account for the space added in the reflow case
+  // when making the reflowing decision.
+  // First, when the next line ends precisely one column over the limit, do not
+  // reflow.
+  EXPECT_EQ("// some text that\n"
+"// reflows\n"
+"// into1234567\n",
+format("// some text that reflows\n"
+   "// into1234567\n",
+   getLLVMStyleWithColumns(21)));
+  // Secondly, when the next line ends later, but the first word in that line
+  // is precisely one column over the limit, do not reflow.
+  EXPECT_EQ("// some text that\n"
+"// reflows\n"
+"// into1234567 f\n",
+format("// some text that reflows\n"
+   "// into1234567 f\n",
+   getLLVMStyleWithColumns(21)));
+}
+
+TEST_F(FormatTestComments, ReflowsCommentsWithExtraWhitespace) {
+  // Baseline.
+  EXPECT_EQ("// some text\n"
+"// that re flows\n",
+format("// some text that\n"
+   "// re flows\n",
+   getLLVMStyleWithColumns(16)));
+  EXPECT_EQ("// some text\n"
+"// that re flows\n",
+format("// some text that\n"
+   "// reflows\n",
+   getLLVMStyleWithColumns(16)));
+  EXPECT_EQ("/* some text\n"
+" * that re flows\n"
+" */\n",
+format("/* some text that\n"
+   "*  re   flows\n"
+   "*/\n",
+   getLLVMStyleWithColumns(16)));
+  // FIXME: We do not reflow if the indent of two subsequent lines differs;
+  // given that this is different behavior from block comments, do we want
+  // to keep this?
+  EXPECT_EQ("// some text\n"
+"// that\n"
+"// re flows\n",
+format("// some text that\n"
+   "// re   flows\n",
+   getLLVMStyleWithColumns(16)));
+  // Space within parts of a line that fit.
+  // FIXME: Use the earliest possible split while reflowing to compress the
+  // whitespace within the line.
+  EXPECT_EQ("// some text that\n"
+"// does re   flow\n"
+"// more  here\n",
+format("// some text that does\n"
+   "// re   flow  more  here\n",
+   getLLVMStyleWithColumns(21)));
+}
+
 TEST_F(FormatTestComments, IgnoresIf0Contents) {
   EXPECT_EQ("#if 0\n"
 "}{)(&*(^%%#%@! fsadj f;ldjs ,:;| <<<>>>][)(][\n"
@@ -2484,6 +2564,7 @@
   " long */\n"
   "  b);",
   format("a = f(a, /* long long */ b);", getLLVMStyleWithColumns(16)));
+
   EXPECT_EQ(
   "a = f(\n"
   "a,\n"
@@ -2888,16 +2969,15 @@
getLLVMStyleWithColumns(20)));
 }
 
-TEST_F(FormatTestComments, NoCrush_Bug34236) {
+TEST_F(For

[PATCH] D40310: Restructure how we break tokens.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:1707
+  RemainingTokenColumns = Token->getRemainingLength(
+  NextLineIndex, TailOffset, ContentStartColumn);
+  Reflow = true;

krasimir wrote:
> When we're reflowing we replace the reflow split with a space, so IMO this 
> should be:
> ```
> RemainingTokenColumns = Token->getRemainingLength(
>   NextLineIndex, TailOffset, ContentStartColumn + 1);
> ```
Actually, ContentStartColumn is just calculated incorrectly above. Added tests, 
and added the +1 above, which makes it +1 for all code below.



Comment at: lib/Format/ContinuationIndenter.cpp:1721
+Token->getSplit(NextLineIndex, TailOffset, ColumnLimit,
+ContentStartColumn, CommentPragmasRegex);
+if (Split.first == StringRef::npos) {

krasimir wrote:
> Looks like `ContentStartColumn` is consistently used as the start column of 
> the reflown content on the next line.
> I suggest to `++ContentStartColumn` at the beginning of the body of this if 
> statement (and adjust below appropriately).
Yep, that's what I also figured out - that also led to removing 
++ContentStartColumn in the reflow case below, which was what made this work at 
all.
Added tests to ReflowsCommentsPrecise, which flow through all of the corner 
cases of the if's here.


https://reviews.llvm.org/D40310



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

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This is how I always perceived this option in the first place, so LGTM.
But maybe its intention is different, so we should wait for @arphaman's 
comments.

Could you also update comments of `CodeCompleteConsumer::includeGlobals` and 
`CodeCompleteOptions::IncludeGlobals` to indicate we ignore all namespace-level 
decls now (it currently only mentions top-level decls, which may be confusing).


https://reviews.llvm.org/D40562



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

[PATCH] D40508: Replace long type names in IR with hashes

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In https://reviews.llvm.org/D40508#938686, @rjmccall wrote:

> In https://reviews.llvm.org/D40508#938675, @sepavloff wrote:
>
> > In https://reviews.llvm.org/D40508#938040, @rjmccall wrote:
> >
> > > My skepticism about the raw_ostream is not about the design of having a 
> > > custom raw_ostream subclass, it's that that subclass could conceivably be 
> > > re-used by some other client.  It feels like it belongs as an internal 
> > > hack in Clang absent some real evidence that someone else would use it.
> >
> >
> > This class can be helpful in various cases where string identifier must 
> > persistently identify an object and memory consumption must be low. It may 
> > be:
> >
> > - If we introduce an option that allows a user to specify how many symbols 
> > of full type name are kept in abbreviated name, then `llvm-link` may see 
> > types named as `foo` and `2544896211ff669ed44dccd265f4c9163b340190`, 
> > if they come from modules compiled with different options. To find out that 
> > these are the same type, it must have access to the same machinery.
>
>
> The LLVM linking model does not actually depend on struct type names 
> matching.  My understanding is that, at best, it uses that as a heuristic for 
> deciding whether to make an effort to unify two types, but it's not something 
> that's ultimately supposed to impact IR semantics.


It is mainly true with an exception, when `llvm-link` resolves opaque types it 
relies on type name only. And this behavior creates the issue that 
https://reviews.llvm.org/D40567 tries to resolve.

> If we needed IR type names to match reliably, we would use a mangled name, 
> not a pretty-print.

There is no requirement for IR type name to be an identifier, so pretty-print 
fits the need of type identification.


https://reviews.llvm.org/D40508



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

[PATCH] D40310: Restructure how we break tokens.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:1749
+  }
+  if (!Reflow) {
+// If we didn't reflow into the next line, the only space to consider 
is

nit: Maybe change this to `if (Reflow)` and switch the if-else bodies.



Comment at: lib/Format/ContinuationIndenter.cpp:1777
+  assert(Penalty >= NewBreakPenalty);
+  Penalty -= NewBreakPenalty;
+}

Shouldn't we be resetting `NewBreakBefore` to `false` here?


https://reviews.llvm.org/D40310



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

[PATCH] D40564: [clangd] Simplify common JSON-parsing patterns in Protocol.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319309: [clangd] Simplify common JSON-parsing patterns in 
Protocol. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D40564?vs=124585&id=124713#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40564

Files:
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test

Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -51,7 +51,7 @@
   static URI fromUri(llvm::StringRef uri);
   static URI fromFile(llvm::StringRef file);
 
-  static URI parse(llvm::StringRef U) { return fromUri(U); }
+  static llvm::Optional parse(const json::Expr &U);
   static json::Expr unparse(const URI &U);
 
   friend bool operator==(const URI &LHS, const URI &RHS) {
Index: clang-tools-extra/trunk/clangd/Protocol.cpp
===
--- clang-tools-extra/trunk/clangd/Protocol.cpp
+++ clang-tools-extra/trunk/clangd/Protocol.cpp
@@ -24,6 +24,149 @@
 using namespace clang;
 using namespace clang::clangd;
 
+namespace {
+// Helper for mapping JSON objects onto our protocol structs. Intended use:
+// Optional parse(json::Expr E) {
+//   ObjectParser O(E);
+//   if (!O || !O.parse("mandatory_field", Result.MandatoryField))
+// return None;
+//   O.parse("optional_field", Result.OptionalField);
+//   return Result;
+// }
+// FIXME: the static methods here should probably become the public parse()
+// extension point. Overloading free functions allows us to uniformly handle
+// enums, vectors, etc.
+class ObjectParser {
+public:
+  ObjectParser(const json::Expr &E) : O(E.asObject()) {}
+
+  // True if the expression is an object.
+  operator bool() { return O; }
+
+  template  bool parse(const char *Prop, T &Out) {
+assert(*this && "Must check this is an object before calling parse()");
+if (const json::Expr *E = O->get(Prop))
+  return parse(*E, Out);
+return false;
+  }
+
+  // Optional requires special handling, because missing keys are OK.
+  template  bool parse(const char *Prop, llvm::Optional &Out) {
+assert(*this && "Must check this is an object before calling parse()");
+if (const json::Expr *E = O->get(Prop))
+  return parse(*E, Out);
+Out = None;
+return true;
+  }
+
+private:
+  // Primitives.
+  static bool parse(const json::Expr &E, std::string &Out) {
+if (auto S = E.asString()) {
+  Out = *S;
+  return true;
+}
+return false;
+  }
+
+  static bool parse(const json::Expr &E, int &Out) {
+if (auto S = E.asInteger()) {
+  Out = *S;
+  return true;
+}
+return false;
+  }
+
+  static bool parse(const json::Expr &E, bool &Out) {
+if (auto S = E.asBoolean()) {
+  Out = *S;
+  return true;
+}
+return false;
+  }
+
+  // Types with a parse() function.
+  template  static bool parse(const json::Expr &E, T &Out) {
+if (auto Parsed = std::remove_reference::type::parse(E)) {
+  Out = std::move(*Parsed);
+  return true;
+}
+return false;
+  }
+
+  // Nullable values as Optional.
+  template 
+  static bool parse(const json::Expr &E, llvm::Optional &Out) {
+if (E.asNull()) {
+  Out = None;
+  return true;
+}
+T Result;
+if (!parse(E, Result))
+  return false;
+Out = std::move(Result);
+return true;
+  }
+
+  // Array values with std::vector type.
+  template 
+  static bool parse(const json::Expr &E, std::vector &Out) {
+if (auto *A = E.asArray()) {
+  Out.clear();
+  Out.resize(A->size());
+  for (size_t I = 0; I < A->size(); ++I)
+if (!parse((*A)[I], Out[I]))
+  return false;
+  return true;
+}
+return false;
+  }
+
+  // Object values with std::map
+  template 
+  static bool parse(const json::Expr &E, std::map &Out) {
+if (auto *O = E.asObject()) {
+  for (const auto &KV : *O)
+if (!parse(KV.second, Out[StringRef(KV.first)]))
+  return false;
+  return true;
+}
+return false;
+  }
+
+  // Special cased enums, which can't have T::parse() functions.
+  // FIXME: make everything free functions so there's no special casing.
+  static bool parse(const json::Expr &E, TraceLevel &Out) {
+if (auto S = E.asString()) {
+  if (*S == "off") {
+Out = TraceLevel::Off;
+return true;
+  } else if (*S == "messages") {
+Out = TraceLevel::Messages;
+return true;
+  } else if (*S == "verbose") {
+Out = TraceLevel::Verbose;
+return true;
+  }
+}
+return false;
+  }
+
+  static bool parse(const json::Expr &E, FileChangeType &Out) {
+if (auto T = E.asInteger()) {
+  if (*T < static_cast(FileChangeType::Created) ||

[clang-tools-extra] r319309 - [clangd] Simplify common JSON-parsing patterns in Protocol.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Nov 29 03:36:46 2017
New Revision: 319309

URL: http://llvm.org/viewvc/llvm-project?rev=319309&view=rev
Log:
[clangd] Simplify common JSON-parsing patterns in Protocol.

Summary:
This makes the parse() functions about as short as they can be given the
current signature, and moves all array-traversal etc code to a
central location.

We keep the ability to distinguish between optional and required fields:
and we don't propagate parse errors for optional fields.

I've made most fields required per the LSP spec - the looseness we had
here was mostly a historical accident I think.

Reviewers: ioeric

Subscribers: klimek, cfe-commits, ilya-biryukov

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

Modified:
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=319309&r1=319308&r2=319309&view=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Wed Nov 29 03:36:46 2017
@@ -24,6 +24,149 @@
 using namespace clang;
 using namespace clang::clangd;
 
+namespace {
+// Helper for mapping JSON objects onto our protocol structs. Intended use:
+// Optional parse(json::Expr E) {
+//   ObjectParser O(E);
+//   if (!O || !O.parse("mandatory_field", Result.MandatoryField))
+// return None;
+//   O.parse("optional_field", Result.OptionalField);
+//   return Result;
+// }
+// FIXME: the static methods here should probably become the public parse()
+// extension point. Overloading free functions allows us to uniformly handle
+// enums, vectors, etc.
+class ObjectParser {
+public:
+  ObjectParser(const json::Expr &E) : O(E.asObject()) {}
+
+  // True if the expression is an object.
+  operator bool() { return O; }
+
+  template  bool parse(const char *Prop, T &Out) {
+assert(*this && "Must check this is an object before calling parse()");
+if (const json::Expr *E = O->get(Prop))
+  return parse(*E, Out);
+return false;
+  }
+
+  // Optional requires special handling, because missing keys are OK.
+  template  bool parse(const char *Prop, llvm::Optional &Out) {
+assert(*this && "Must check this is an object before calling parse()");
+if (const json::Expr *E = O->get(Prop))
+  return parse(*E, Out);
+Out = None;
+return true;
+  }
+
+private:
+  // Primitives.
+  static bool parse(const json::Expr &E, std::string &Out) {
+if (auto S = E.asString()) {
+  Out = *S;
+  return true;
+}
+return false;
+  }
+
+  static bool parse(const json::Expr &E, int &Out) {
+if (auto S = E.asInteger()) {
+  Out = *S;
+  return true;
+}
+return false;
+  }
+
+  static bool parse(const json::Expr &E, bool &Out) {
+if (auto S = E.asBoolean()) {
+  Out = *S;
+  return true;
+}
+return false;
+  }
+
+  // Types with a parse() function.
+  template  static bool parse(const json::Expr &E, T &Out) {
+if (auto Parsed = std::remove_reference::type::parse(E)) {
+  Out = std::move(*Parsed);
+  return true;
+}
+return false;
+  }
+
+  // Nullable values as Optional.
+  template 
+  static bool parse(const json::Expr &E, llvm::Optional &Out) {
+if (E.asNull()) {
+  Out = None;
+  return true;
+}
+T Result;
+if (!parse(E, Result))
+  return false;
+Out = std::move(Result);
+return true;
+  }
+
+  // Array values with std::vector type.
+  template 
+  static bool parse(const json::Expr &E, std::vector &Out) {
+if (auto *A = E.asArray()) {
+  Out.clear();
+  Out.resize(A->size());
+  for (size_t I = 0; I < A->size(); ++I)
+if (!parse((*A)[I], Out[I]))
+  return false;
+  return true;
+}
+return false;
+  }
+
+  // Object values with std::map
+  template 
+  static bool parse(const json::Expr &E, std::map &Out) {
+if (auto *O = E.asObject()) {
+  for (const auto &KV : *O)
+if (!parse(KV.second, Out[StringRef(KV.first)]))
+  return false;
+  return true;
+}
+return false;
+  }
+
+  // Special cased enums, which can't have T::parse() functions.
+  // FIXME: make everything free functions so there's no special casing.
+  static bool parse(const json::Expr &E, TraceLevel &Out) {
+if (auto S = E.asString()) {
+  if (*S == "off") {
+Out = TraceLevel::Off;
+return true;
+  } else if (*S == "messages") {
+Out = TraceLevel::Messages;
+return true;
+  } else if (*S == "verbose") {
+Out = TraceLevel::Verbose;
+return true;
+  }
+}
+return false;
+  }
+
+  static bool parse(const json::Expr &E, FileChangeType &Out) {
+if (auto T = E.asInteger()) {
+ 

[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.

[Peter Siket via Phabricator via cfe-commits]

MontyKutyi added a comment.

Ping.


https://reviews.llvm.org/D34030



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

[PATCH] D40596: [clangd] New conventions for JSON-marshalling functions, centralize machinery

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: cfe-commits, klimek.

- JSON<->Obj interface is now ADL functions, so they play nicely with enums
- recursive vector/map parsing and ObjectMapper moved to JSONExpr and tested
- renamed (un)parse to (de)serialize, since text -> JSON is called parse
- Protocol.cpp gets a bit shorter

Sorry for the giant patch, it's prety mechanical though


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40596

Files:
  clangd/ClangdLSPServer.cpp
  clangd/JSONExpr.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  test/clangd/trace.test
  unittests/clangd/JSONExprTests.cpp

Index: unittests/clangd/JSONExprTests.cpp
===
--- unittests/clangd/JSONExprTests.cpp
+++ unittests/clangd/JSONExprTests.cpp
@@ -229,6 +229,64 @@
   }
 }
 
+// Sample struct with typical JSON-mapping rules.
+struct CustomStruct {
+  CustomStruct() : B(false) {}
+  CustomStruct(std::string S, llvm::Optional I, bool B)
+  : S(S), I(I), B(B) {}
+  std::string S;
+  llvm::Optional I;
+  bool B;
+};
+inline bool operator==(const CustomStruct &L, const CustomStruct &R) {
+  return L.S == R.S && L.I == R.I && L.B == R.B;
+}
+inline std::ostream &operator<<(std::ostream &OS, const CustomStruct &S) {
+  return OS << "(" << S.S << ", " << (S.I ? std::to_string(*S.I) : "None")
+<< ", " << S.B << ")";
+}
+bool deserialize(const json::Expr &E, CustomStruct &R) {
+  ObjectMapper O(E);
+  if (!O || !O.map("str", R.S) || !O.map("int", R.I))
+return false;
+  O.map("bool", R.B);
+  return true;
+}
+
+TEST(JSONTest, Deserialize) {
+  std::map> R;
+  CustomStruct ExpectedStruct = {"foo", 42, true};
+  std::map> Expected;
+  Expr J = obj{{"foo", ary{
+   obj{
+   {"str", "foo"},
+   {"int", 42},
+   {"bool", true},
+   {"unknown", "ignored"},
+   },
+   obj{{"str", "bar"}},
+   obj{
+   {"str", "baz"},
+   {"bool", "string"}, // OK, deserialize ignores.
+   },
+   }}};
+  Expected["foo"] = {
+  CustomStruct("foo", 42, true),
+  CustomStruct("bar", llvm::None, false),
+  CustomStruct("baz", llvm::None, false),
+  };
+  ASSERT_TRUE(deserialize(J, R));
+  EXPECT_EQ(R, Expected);
+
+  CustomStruct V;
+  EXPECT_FALSE(deserialize(nullptr, V)) << "Not an object " << V;
+  EXPECT_FALSE(deserialize(obj{}, V)) << "Missing required field " << V;
+  EXPECT_FALSE(deserialize(obj{{"str", 1}}, V)) << "Wrong type " << V;
+  // Optional must parse as the correct type if present.
+  EXPECT_FALSE(deserialize(obj{{"str", 1}, {"int", "string"}}, V))
+  << "Wrong type for Optional " << V;
+}
+
 } // namespace
 } // namespace json
 } // namespace clangd
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -11,7 +11,7 @@
 #  CHECK: {"displayTimeUnit":"ns","traceEvents":[
 # Start opening the doc.
 #  CHECK: "name": "textDocument/didOpen"
-#  CHECK: "ph": "E"
+#  CHECK: "ph": "B"
 # Start building the preamble.
 #  CHECK: "name": "Preamble"
 #  CHECK: },
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -21,7 +21,7 @@
 // Helper for attaching ProtocolCallbacks methods to a JSONRPCDispatcher.
 // Invoke like: Registerer("foo", &ProtocolCallbacks::onFoo)
 // onFoo should be: void onFoo(Ctx &C, FooParams &Params)
-// FooParams should have a static factory method: parse(const json::Expr&).
+// FooParams should have a JSON deserialize function.
 struct HandlerRegisterer {
   template 
   void operator()(StringRef Method,
@@ -31,11 +31,9 @@
 auto *Callbacks = this->Callbacks;
 Dispatcher.registerHandler(
 Method, [=](RequestContext C, const json::Expr &RawParams) {
-  if (auto P = [&] {
-trace::Span Tracer("Parse");
-return std::decay::type::parse(RawParams);
-  }()) {
-(Callbacks->*Handler)(std::move(C), *P);
+  typename std::remove_reference::type P;
+  if (deserialize(RawParams, P)) {
+(Callbacks->*Handler)(std::move(C), P);
   } else {
 Out->log("Failed to decode " + Method + " request.\n");
   }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -13,8 +13,8 @@
 // This is not meant to be a complete implementation, new interfaces are added
 // when they're needed.
 //
-// Each struct has a parse and unparse function, that converts back and fo

[PATCH] D39114: [XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 124717.
dberris added a comment.

- [XRay][compiler-rt][Darwin] Use dynamic initialisation as an alternative


https://reviews.llvm.org/D39114

Files:
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/weak_symbols.txt
  compiler-rt/lib/xray/xray_fdr_logging.cc
  compiler-rt/lib/xray/xray_flags.inc
  compiler-rt/lib/xray/xray_init.cc
  compiler-rt/lib/xray/xray_trampoline_x86_64.S

Index: compiler-rt/lib/xray/xray_trampoline_x86_64.S
===
--- compiler-rt/lib/xray/xray_trampoline_x86_64.S
+++ compiler-rt/lib/xray/xray_trampoline_x86_64.S
@@ -14,10 +14,13 @@
 //===--===//
 
 #include "../builtins/assembly.h"
+#include "../sanitizer_common/sanitizer_asm.h"
+
+
 
 .macro SAVE_REGISTERS
 	subq $192, %rsp
-	.cfi_def_cfa_offset 200
+	CFI_DEF_CFA_OFFSET(200)
 	// At this point, the stack pointer should be aligned to an 8-byte boundary,
 	// because any call instructions that come after this will add another 8
 	// bytes and therefore align it to 16-bytes.
@@ -57,7 +60,7 @@
 	movq	8(%rsp), %r8
 	movq	0(%rsp), %r9
 	addq	$192, %rsp
-	.cfi_def_cfa_offset 8
+	CFI_DEF_CFA_OFFSET(8)
 .endm
 
 .macro ALIGNED_CALL_RAX
@@ -75,21 +78,25 @@
 .endm
 
 	.text
+#if !defined(__APPLE__)
+	.section .text
+#else
+	.section __TEXT,__text
+#endif
 	.file "xray_trampoline_x86.S"
 
 //===--===//
 
-	.globl __xray_FunctionEntry
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionEntry)
 	.align 16, 0x90
-	.type __xray_FunctionEntry,@function
-
-__xray_FunctionEntry:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionEntry)
+ASM_TSAN_SYMBOL(__xray_FunctionEntry):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
 	// This load has to be atomic, it's concurrent with __xray_patch().
 	// On x86/amd64, a simple (type-aligned) MOV instruction is enough.
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq	%rax, %rax
 	je	.Ltmp0
 
@@ -101,28 +108,27 @@
 .Ltmp0:
 	RESTORE_REGISTERS
 	retq
-.Ltmp1:
-	.size __xray_FunctionEntry, .Ltmp1-__xray_FunctionEntry
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionEntry)
+	CFI_ENDPROC
 
 //===--===//
 
-	.globl __xray_FunctionExit
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionExit)
 	.align 16, 0x90
-	.type __xray_FunctionExit,@function
-__xray_FunctionExit:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionExit)
+ASM_TSAN_SYMBOL(__xray_FunctionExit):
+	CFI_STARTPROC
 	// Save the important registers first. Since we're assuming that this
 	// function is only jumped into, we only preserve the registers for
 	// returning.
 	subq	$56, %rsp
-	.cfi_def_cfa_offset 64
+	CFI_DEF_CFA_OFFSET(64)
 	movq  %rbp, 48(%rsp)
 	movupd	%xmm0, 32(%rsp)
 	movupd	%xmm1, 16(%rsp)
 	movq	%rax, 8(%rsp)
 	movq	%rdx, 0(%rsp)
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq %rax,%rax
 	je	.Ltmp2
 
@@ -138,22 +144,21 @@
 	movq	8(%rsp), %rax
 	movq	0(%rsp), %rdx
 	addq	$56, %rsp
-	.cfi_def_cfa_offset 8
+	CFI_DEF_CFA_OFFSET(8)
 	retq
-.Ltmp3:
-	.size __xray_FunctionExit, .Ltmp3-__xray_FunctionExit
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionExit)
+	CFI_ENDPROC
 
 //===--===//
 
-	.global __xray_FunctionTailExit
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionTailExit)
 	.align 16, 0x90
-	.type __xray_FunctionTailExit,@function
-__xray_FunctionTailExit:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionTailExit)
+ASM_TSAN_SYMBOL(__xray_FunctionTailExit):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq %rax,%rax
 	je	.Ltmp4
 
@@ -165,26 +170,25 @@
 .Ltmp4:
 	RESTORE_REGISTERS
 	retq
-.Ltmp5:
-	.size __xray_FunctionTailExit, .Ltmp5-__xray_FunctionTailExit
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionTailExit)
+	CFI_ENDPROC
 
 //===--===//
 
-	.globl __xray_ArgLoggerEntry
+	.globl ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry)
 	.align 16, 0x90
-	.type __xray_ArgLoggerEntry,@function
-__xray_ArgLoggerEntry:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_ArgLoggerEntry)
+ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
 	// Again, these function pointer loads must be atomic; MOV is fine.
-	movq	_ZN6__xray13XRayArgLoggerE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray13XRayArgLoggerE)(%rip), %rax
 	testq	%rax, %rax
 	jne	.Larg1entryLog
 
 	// If [arg1 logging handler] not set, defer to no-arg logging.
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq	%ra

[PATCH] D40310: Restructure how we break tokens.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:1749
+  }
+  if (!Reflow) {
+// If we didn't reflow into the next line, the only space to consider 
is

krasimir wrote:
> nit: Maybe change this to `if (Reflow)` and switch the if-else bodies.
I had that first, but found that harder to follow when re-reading the code - if 
you feel strongly, I'm also happy to turn it around again :)



Comment at: lib/Format/ContinuationIndenter.cpp:1777
+  assert(Penalty >= NewBreakPenalty);
+  Penalty -= NewBreakPenalty;
+}

krasimir wrote:
> Shouldn't we be resetting `NewBreakBefore` to `false` here?
NewBreakBefore is always reset at the start of the loop, so resetting it here 
wouldn't matter.


https://reviews.llvm.org/D40310



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

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+  str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will also be garbage";

dcoughlin wrote:
> "Unary operator" is probably not something we should expect users to know 
> about. How about just "The expression is an uninitialized value. The computed 
> value will also be garbage."
> 
Yep, i did not like my wording either :)



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1051
+  ExplodedNodeSet Dst3;
+  evalStore(Dst3, U, U, *I, state, loc, V2_untested);
+  Bldr.addNodes(Dst3);

xazax.hun wrote:
> This will trigger checkBind when unknown value is pre/post incremented. I 
> wonder if this is the desired behavior. Couldn't you achieve the same on the 
> checker side by having a checkPreStmt hook?
> 
> @NoQ, @dcoughlin what do you think about triggering checkBind here? In fact, 
> there is a bind during pre/post increment/decrement. But do we want these 
> events to be visible for checkers with undefined values?
> This will trigger checkBind when unknown value is pre/post incremented. I 
> wonder if this is the desired behavior. Couldn't you achieve the same on the 
> checker side by having a checkPreStmt hook?
The current approach is what was suggested in the 
https://bugs.llvm.org/show_bug.cgi?id=35419#c6, unless i have misread it of 
course.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1046
 if (V2_untested.isUnknownOrUndef()) {
   Bldr.generateNode(U, *I, state->BindExpr(U, LCtx, V2_untested));
+

dcoughlin wrote:
> Instead of generating a node node here, you'll want to generate a new state:
> ```
> state = state->BindExpr(U, LCtx, V2_untested);
> ```
> and use that state in the call to evalStore().
> 
> Otherwise, you'll end up exploring from both the new generated node and from 
> *I.
> 
> Here is a test that demonstrates why this is a problem. You should add it to 
> your tests.
> 
> ```
> void foo() {
>   int x;
> 
>   x++; // expected-warning {{The expression of the unary operator is an 
> uninitialized value. The computed value will also be garbage}}
> 
>   clang_analyzer_warnIfReached(); // no-warning
> 
> ```
> The undefined assignment checker generates what we call "fatal" errors. These 
> errors are so bad that it decides not to explore further on that path to 
> report additional errors. This means that the analyzer should treat any code 
> that is dominated by such an error as not reachable. You'll need to add the 
> `debug.ExprInspection` checker to your test RUN line and a prototype for 
> clang_analyzer_warnIfReached() for this test to to work.
> 
> Instead of generating a node node here, you'll want to generate a new state:
> ```
> state = state->BindExpr(U, LCtx, V2_untested);
> ```
> and use that state in the call to evalStore().
Hm, so the only change needed here is
```diff
- Bldr.generateNode(U, *I, state->BindExpr(U, LCtx, V2_untested));
+ state = state->BindExpr(U, LCtx, V2_untested);
```
?



Comment at: test/Analysis/malloc-plist.c:13
 int *p = malloc(12);
-(*p)++;
+(*p)++; // expected-warning {{The expression of the unary operator is 
an uninitialized value. The computed value will also be garbage}}
+*p = 0; // no-warning

dcoughlin wrote:
> Once you change the core modeling to not generate an extra node, you'll want 
> to initialize `*p` to `0` or something to preserve the intent of this test. 
> For this test it is important that the increment not stop further exploration 
> of the path.
Hmm, *this* test did not break after i changed 
`ExprEngine::VisitIncrementDecrementOperator()`.
Did i change it wrong?



Comment at: test/Analysis/malloc-plist.c:111
 p = (int*)malloc(12);
-(*p)++;
+(*p)++; // expected-warning {{The expression of the unary operator is an 
uninitialized value. The computed value will also be garbage}}
+*p = 0; // no-warning

dcoughlin wrote:
> Same point applies here.
Same here.


Repository:
  rC Clang

https://reviews.llvm.org/D40463



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

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Sema/CodeCompleteConsumer.h:284
 
+  llvm::Optional ScopeSpecifier;
+

Maybe add a brief comment for consistency with other decls?



Comment at: lib/Sema/SemaCodeComplete.cpp:4609
 
+  if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);

Why do we alter this code path?



Comment at: lib/Sema/SemaCodeComplete.cpp:4611
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CC.setCXXScopeSpecifier(SS);
+HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);

Do we really want to set invalid scope specifiers here?


https://reviews.llvm.org/D40563



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

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:4609
 
+  if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);

ilya-biryukov wrote:
> Why do we alter this code path?
Maybe we should add a test or provide examples why the current code does not 
work for us?


https://reviews.llvm.org/D40563



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

[PATCH] D39455: [CodeGen] Add initial support for union members in TBAA

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev updated this revision to Diff 124726.
kosarev added a comment.

- Added tests with nested unions.
- Rebased.


https://reviews.llvm.org/D39455

Files:
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/CodeGenTBAA.cpp
  lib/CodeGen/CodeGenTBAA.h
  test/CodeGen/tbaa-union.cpp
  test/CodeGen/union-tbaa1.c

Index: test/CodeGen/union-tbaa1.c
===
--- test/CodeGen/union-tbaa1.c
+++ test/CodeGen/union-tbaa1.c
@@ -15,30 +15,32 @@
 // But no tbaa for the two stores:
 // CHECK: %uw[[UW1:[0-9]*]] = getelementptr
 // CHECK: store{{.*}}%uw[[UW1]]
-// CHECK: tbaa ![[OCPATH:[0-9]+]]
+// CHECK: tbaa [[TAG_vect32_union_member:![0-9]+]]
 // There will be a load after the store, and it will use tbaa. Make sure
 // the check-not above doesn't find it:
 // CHECK: load
   Tmp[*Index][0].uw = Arr[*Index][0] * Num;
 // CHECK: %uw[[UW2:[0-9]*]] = getelementptr
 // CHECK: store{{.*}}%uw[[UW2]]
-// CHECK: tbaa ![[OCPATH]]
+// CHECK: tbaa [[TAG_vect32_union_member]]
   Tmp[*Index][1].uw = Arr[*Index][1] * Num;
 // Same here, don't generate tbaa for the loads:
 // CHECK: %uh[[UH1:[0-9]*]] = bitcast %union.vect32
 // CHECK: %arrayidx[[AX1:[0-9]*]] = getelementptr{{.*}}%uh[[UH1]]
 // CHECK: load i16, i16* %arrayidx[[AX1]]
-// CHECK: tbaa ![[OCPATH]]
+// CHECK: tbaa [[TAG_vect32_union_member]]
 // CHECK: store
   Vec[0] = Tmp[*Index][0].uh[1];
 // CHECK: %uh[[UH2:[0-9]*]] = bitcast %union.vect32
 // CHECK: %arrayidx[[AX2:[0-9]*]] = getelementptr{{.*}}%uh[[UH2]]
 // CHECK: load i16, i16* %arrayidx[[AX2]]
-// CHECK: tbaa ![[OCPATH]]
+// CHECK: tbaa [[TAG_vect32_union_member]]
 // CHECK: store
   Vec[1] = Tmp[*Index][1].uh[1];
   bar(Tmp);
 }
 
-// CHECK-DAG: ![[CHAR:[0-9]+]] = !{!"omnipotent char"
-// CHECK-DAG: ![[OCPATH]] = !{![[CHAR]], ![[CHAR]], i64 0}
+// CHECK-DAG: [[TAG_vect32_union_member]] = !{[[TYPE_vect32:!.*]], [[TYPE_union_member:!.*]], i64 0}
+// CHECK-DAG: [[TYPE_vect32]] = !{!"", [[TYPE_union_member]], i64 0}
+// CHECK-DAG: [[TYPE_union_member]] = !{!"union member", [[TYPE_char:!.*]], i64 0}
+// CHECK-DAG: [[TYPE_char]] = !{!"omnipotent char", {{.*}}}
Index: test/CodeGen/tbaa-union.cpp
===
--- test/CodeGen/tbaa-union.cpp
+++ test/CodeGen/tbaa-union.cpp
@@ -0,0 +1,100 @@
+// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s -emit-llvm -o - | FileCheck %s
+//
+// Check that we generate correct TBAA information for accesses to union
+// members.
+
+struct X {
+  int a, b;
+  int arr[3];
+  int c, d;
+};
+
+union U {
+  int i;
+  X x;
+  int j;
+};
+
+struct S {
+  U u, v;
+};
+
+union N {
+  int i;
+  S s;
+  int j;
+};
+
+struct R {
+  N n, m;
+};
+
+int f1(U *p) {
+// CHECK-LABEL: _Z2f1P1U
+// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_U_j:!.*]]
+  return p->j;
+}
+
+int f2(S *p) {
+// CHECK-LABEL: _Z2f2P1S
+// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_u_i:!.*]]
+  return p->u.i;
+}
+
+int f3(S *p) {
+// CHECK-LABEL: _Z2f3P1S
+// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_v_j:!.*]]
+  return p->v.j;
+}
+
+int f4(S *p) {
+// CHECK-LABEL: _Z2f4P1S
+// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_u_x_b:!.*]]
+  return p->u.x.b;
+}
+
+int f5(S *p) {
+// CHECK-LABEL: _Z2f5P1S
+// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_v_x_b:!.*]]
+  return p->v.x.b;
+}
+
+int f6(S *p) {
+// CHECK-LABEL: _Z2f6P1S
+// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_u_x_arr:!.*]]
+  return p->u.x.arr[1];
+}
+
+int f7(S *p) {
+// CHECK-LABEL: _Z2f7P1S
+// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_v_x_arr:!.*]]
+  return p->v.x.arr[1];
+}
+
+int f8(N *p) {
+// CHECK-LABEL: _Z2f8P1N
+// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_N_s_v_x_c:!.*]]
+  return p->s.v.x.c;
+}
+
+int f9(R *p) {
+// CHECK-LABEL: _Z2f9P1R
+// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_R_m_s_v_x_c:!.*]]
+  return p->m.s.v.x.c;
+}
+
+// CHECK-DAG: [[TAG_U_j]] = !{[[TYPE_U:!.*]], [[TYPE_union_member:!.*]], i64 0}
+// CHECK-DAG: [[TAG_S_u_i]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 0}
+// CHECK-DAG: [[TAG_S_u_x_b]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 0}
+// CHECK-DAG: [[TAG_S_u_x_arr]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 0}
+// CHECK-DAG: [[TAG_S_v_j]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 28}
+// CHECK-DAG: [[TAG_S_v_x_b]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 28}
+// CHECK-DAG: [[TAG_S_v_x_arr]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 28}
+// CHECK-DAG: [[TAG_N_s_v_x_c]] = !{[[TYPE_N:!.*]], [[TYPE_union_member]], i64 0}
+// CHECK-DAG: [[TAG_R_m_s_v_x_c]] = !{[[TYPE_R:!.*]], [[TYPE_union_member]], i64 56}
+// CHECK-DAG: [[TYPE_U]] = !{!"_ZTS1U", [[TYPE_union_member]], i64 0}
+// CHECK-DAG: [[TYPE_S]] = !{!"_ZTS1S", [[TYPE_U]], i64 0, [[TYPE_U]], i64 28}
+// CHECK-DAG: [[TYPE_N]] = !{!"_ZTS1N", [[TYPE_union_member]], i64 0}
+// CHECK-DAG: [[TYPE_R]] = !{!"_ZTS1R", [[TYPE_N]], i64 0, [[TYPE_N]], i64 56}
+// CHECK-DAG: [[TYPE_union_member]] = !{!"union member", [[

[PATCH] D39455: [CodeGen] Add initial support for union members in TBAA

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev added a comment.

Thanks Hal. Will commit it tomorrow.


https://reviews.llvm.org/D39455



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

[PATCH] D40601: [XRay][clang] Introduce -fxray-always-emit-customevents

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.

The -fxray-always-emit-customevents flag instructs clang to always emit
the LLVM IR for calls to the `__xray_customevent(...)` built-in
function. The default behaviour currently respects whether the function
has an `[[clang::xray_never_instrument]]` attribute, and thus not lower
the appropriate IR code for the custom event built-in.

This change allows users calling through to the
`__xray_customevent(...)` built-in to always see those calls lowered to
the corresponding LLVM IR to lay down instrumentation points for these
custom event calls.


https://reviews.llvm.org/D40601

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-always-emit-customevent.cpp

Index: clang/test/CodeGen/xray-always-emit-customevent.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-always-emit-customevent.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fxray-instrument -fxray-always-emit-customevents -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+
+// CHECK-LABEL: @_Z15neverInstrumentv
+[[clang::xray_never_instrument]] void neverInstrument() {
+  static constexpr char kPhase[] = "never";
+  __xray_customevent(kPhase, 5);
+  // CHECK: call void @llvm.xray.customevent(i8*{{.*}}, i32 5)
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -785,6 +785,8 @@
   Opts.InstrumentFunctionEntryBare =
   Args.hasArg(OPT_finstrument_function_entry_bare);
   Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
+  Opts.XRayAlwaysEmitCustomEvents =
+  Args.hasArg(OPT_fxray_always_emit_customevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
@@ -2503,6 +2505,11 @@
   Opts.XRayInstrument =
   Args.hasFlag(OPT_fxray_instrument, OPT_fnoxray_instrument, false);
 
+  // -fxray-always-emit-customevents
+  Opts.XRayAlwaysEmitCustomEvents =
+  Args.hasFlag(OPT_fxray_always_emit_customevents,
+   OPT_fnoxray_always_emit_customevents, false);
+
   // -fxray-{always,never}-instrument= filenames.
   Opts.XRayAlwaysInstrumentFiles =
   Args.getAllArgValues(OPT_fxray_always_instrument);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -29,6 +29,8 @@
 "-fxray-instruction-threshold=";
 constexpr char XRayAlwaysInstrumentOption[] = "-fxray-always-instrument=";
 constexpr char XRayNeverInstrumentOption[] = "-fxray-never-instrument=";
+constexpr char XRayAlwaysEmitCustomEventsOption[] =
+"-fxray-always-emit-customevents";
 } // namespace
 
 XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
@@ -63,6 +65,14 @@
 D.Diag(clang::diag::err_drv_invalid_value) << A->getAsString(Args) << S;
 }
 
+// By default, the back-end will not emit the lowering for XRay customevent
+// calls if the function is not instrumented. In the future we will change
+// this default to be the reverse, but in the meantime we're going to
+// introduce the new functionality behind a flag.
+if (Args.hasFlag(options::OPT_fxray_always_emit_customevents,
+ options::OPT_fnoxray_always_emit_customevents, false))
+  XRayAlwaysEmitCustomEvents = true;
+
 // Validate the always/never attribute files. We also make sure that they
 // are treated as actual dependencies.
 for (const auto &Filename :
@@ -91,6 +101,10 @@
 return;
 
   CmdArgs.push_back(XRayInstrumentOption);
+
+  if (XRayAlwaysEmitCustomEvents)
+CmdArgs.push_back(XRayAlwaysEmitCustomEventsOption);
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -1771,6 +1771,10 @@
   /// instrumented with XRay nop sleds.
   bool ShouldXRayInstrumentFunction() const;
 
+  /// AlwaysEmitXRayCustomEvents - Return true if we must unconditionally emit
+  /// XRay custom event handling calls.
+  bool AlwaysEmitXRayCustomEvents() const;
+
   /// Encode an address into a form suitable for use in a function prologue.
   llvm::Constant 

[PATCH] D39279: Stringizing raw string literals containing newline

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added a comment.

I am sorry I wasn't really clear. What I meant was to do something like this 
(pseudo-code, dealing only with newlines):

  if( Str.size() == 0)
return;
  
  // Calculate all the extra space needed first.
  
  typename T::size_type extra_space = 0;
  bool previous_char_was_endline = false;
  
  for(const auto ch : Str) {
if( ch == '\n' || ch == '\r' ) {
  if( !previous_char_was_endline )
++extra_space;
  previous_char_was_endline = true;
} else {
  previous_char_was_endline = false;
}
  }
  
  if (extra_space == 0)
return; 
  
  // Resize the string.
  
  const typename T::size_type original_size = Str.size();
  Str.resize(original_size + extra_space);
  
  // Iterate backwards and move characters as needed.
  
  bool is_in_block_of_endlines = false;
  
  for(typename T::size_type i = original_size - 1; i > 0; --i) {
if( Str[i] == '\n' || Str[i] == '\r' ) {
  if (!is_in_block_of_endlines) {
Str[i + extra_space - 1] = '\\';
Str[i + extra_space] = 'n';
--extra_space;
if(extra_space == 0)
  return; // early exit - no more characters need to be moved
  }
  is_in_block_of_endlines = true;
} else {
  Str[i + extra_space] = Str[i];
  is_in_block_of_endlines = false;
}
  }

This is just a suggestion, it is a bit more complicated but should be 
O(lenght_of_string) whereas your solution is a bit more straightforward but is 
more like O(length_of_string * number_of_endlines_in_string). I leave it up to 
you what is better here. If you decide to go this way, please assume my 
pseudocode is buggy and don't rely on it.


https://reviews.llvm.org/D39279



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

[PATCH] D40310: Restructure how we break tokens.

[Krasimir Georgiev via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D40310



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

[PATCH] D39114: [XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin

[Dean Michael Berris via Phabricator via cfe-commits]

dberris closed this revision.
dberris added a comment.

Already commited; closing this revision out now.


https://reviews.llvm.org/D39114



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:22
+AST_MATCHER(CXXRecordDecl, hasDefinition) {
+  if (!Node.hasDefinition())
+return false;

`return Node.hasDefinition();`



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:32
+  StringRef Name = Node->getIdentifier()->getName();
+  MultipleInheritanceCheck::InterfaceMap->insert(
+std::make_pair(Name, isInterface));

What's the reason to qualify `InterfaceMap` and other members of this class?



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.h:35-38
+  void addNodeToInterfaceMap(const CXXRecordDecl *Node, bool isInterface);
+  bool getInterfaceStatus(const CXXRecordDecl *Node, bool *isInterface);
+  bool isCurrentClassInterface(const CXXRecordDecl *Node);
+  bool isInterface(const CXXRecordDecl *Node);

Do all these methods need to be public?



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.h:44
+  // where N is the number of classes.
+  llvm::StringMap *InterfaceMap;
+};

What's the reason to use a pointer and dynamically allocate the map? Just make 
it a value and clear it instead of deleting.



Comment at: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst:46
+
+See the features disallowed in Fuchsia at 
https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md

This is not about the check, rather about the underlying style guide. The 
document linked here doesn't explain why certain features are disallowed. I'd 
suggest putting some effort in expanding the document to include reasoning for 
each rule (e.g. see 
https://google.github.io/styleguide/cppguide.html#Multiple_Inheritance for a 
related rule in the Google C++ style guide).


https://reviews.llvm.org/D40580



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

[PATCH] D40594: [InstCombine] miscompile of __builtin_fmod

[Sanjay Patel via Phabricator via cfe-commits]

spatel added reviewers: efriedma, hfinkel.
spatel added a comment.

I don't know the history of the frem instruction in IR, and the description in 
http://llvm.org/docs/LangRef.html#frem-instruction is vague.

But based on the existing code, I think this is working as intended. If the 
instruction has the same semantics as the math library function 'fmod', then we 
do want to convert this to an IR instruction in clang.

Note that when I filed PR34870, I assumed the bug was in the IR optimizer in 
the instcombine pass, but if the IR instruction does not match the semantics of 
the math library function, then I'd be wrong. :)


https://reviews.llvm.org/D40594



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

[PATCH] D40588: [OpenMP] Diagnose undeclared variables on declare target clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev requested changes to this revision.
ABataev added a comment.
This revision now requires changes to proceed.

I'm unable to reproduce a crash, the test works correctly even without you patch


https://reviews.llvm.org/D40588



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

r319314 - Restructure how we break tokens.

[Manuel Klimek via cfe-commits]

Author: klimek
Date: Wed Nov 29 06:29:43 2017
New Revision: 319314

URL: http://llvm.org/viewvc/llvm-project?rev=319314&view=rev
Log:
Restructure how we break tokens.

This fixes some bugs in the reflowing logic and splits out the concerns
of reflowing from BreakableToken.

Things to do after this patch:
- Refactor the breakProtrudingToken function possibly into a class, so we
  can split it up into methods that operate on the common state.
- Optimize whitespace compression when reflowing by using the next possible
  split point instead of the latest possible split point.
- Retry different strategies for reflowing (strictly staying below the
  column limit vs. allowing excess characters if possible).

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

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/lib/Format/BreakableToken.h
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=319314&r1=319313&r2=319314&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Wed Nov 29 06:29:43 2017
@@ -67,6 +67,8 @@ static BreakableToken::Split getCommentS
  unsigned ColumnLimit,
  unsigned TabWidth,
  encoding::Encoding Encoding) {
+  DEBUG(llvm::dbgs() << "Comment split: \"" << Text << ", " << ColumnLimit
+ << "\", Content start: " << ContentStartColumn << "\n");
   if (ColumnLimit <= ContentStartColumn + 1)
 return BreakableToken::Split(StringRef::npos, 0);
 
@@ -171,7 +173,7 @@ bool switchesFormatting(const FormatToke
 }
 
 unsigned
-BreakableToken::getLineLengthAfterCompression(unsigned RemainingTokenColumns,
+BreakableToken::getLengthAfterCompression(unsigned RemainingTokenColumns,
   Split Split) const {
   // Example: consider the content
   // lala  lala
@@ -181,50 +183,58 @@ BreakableToken::getLineLengthAfterCompre
   // We compute the number of columns when the split is compressed into a 
single
   // space, like:
   // lala lala
+  //
+  // FIXME: Correctly measure the length of whitespace in Split.second so it
+  // works with tabs.
   return RemainingTokenColumns + 1 - Split.second;
 }
 
-unsigned BreakableSingleLineToken::getLineCount() const { return 1; }
+unsigned BreakableStringLiteral::getLineCount() const { return 1; }
 
-unsigned BreakableSingleLineToken::getLineLengthAfterSplit(
-unsigned LineIndex, unsigned TailOffset,
-StringRef::size_type Length) const {
-  return StartColumn + Prefix.size() + Postfix.size() +
- encoding::columnWidthWithTabs(Line.substr(TailOffset, Length),
-   StartColumn + Prefix.size(),
-   Style.TabWidth, Encoding);
+unsigned BreakableStringLiteral::getRangeLength(unsigned LineIndex,
+unsigned Offset,
+StringRef::size_type Length,
+unsigned StartColumn) const {
+  assert(false &&
+ "Getting the length of a part of the string literal indicates that "
+ "the code tries to reflow it.");
 }
 
-BreakableSingleLineToken::BreakableSingleLineToken(
+unsigned
+BreakableStringLiteral::getRemainingLength(unsigned LineIndex, unsigned Offset,
+   unsigned StartColumn) const {
+  return UnbreakableTailLength + Postfix.size() +
+ encoding::columnWidthWithTabs(Line.substr(Offset, StringRef::npos),
+   StartColumn, Style.TabWidth, Encoding);
+}
+
+unsigned BreakableStringLiteral::getContentStartColumn(unsigned LineIndex,
+   bool Break) const {
+  return StartColumn + Prefix.size();
+}
+
+BreakableStringLiteral::BreakableStringLiteral(
 const FormatToken &Tok, unsigned StartColumn, StringRef Prefix,
 StringRef Postfix, bool InPPDirective, encoding::Encoding Encoding,
 const FormatStyle &Style)
 : BreakableToken(Tok, InPPDirective, Encoding, Style),
-  StartColumn(StartColumn), Prefix(Prefix), Postfix(Postfix) {
+  StartColumn(StartColumn), Prefix(Prefix), Postfix(Postfix),
+  UnbreakableTailLength(Tok.UnbreakableTailLength) {
   assert(Tok.TokenText.startswith(Prefix) && Tok.TokenText.endswith(Postfix));
   Line = Tok.TokenText.substr(
   Prefix.size(), Tok.TokenText.size() - Prefix.size() - Postfix.size());
 }
 
-BreakableStringLiteral::BreakableStringLiteral(
-const FormatToken &Tok, unsigned StartC

[PATCH] D40310: Restructure how we break tokens.

[Manuel Klimek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319314: Restructure how we break tokens. (authored by 
klimek).

Repository:
  rL LLVM

https://reviews.llvm.org/D40310

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/lib/Format/BreakableToken.h
  cfe/trunk/lib/Format/ContinuationIndenter.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp
  cfe/trunk/unittests/Format/FormatTestComments.cpp

Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -67,6 +67,8 @@
  unsigned ColumnLimit,
  unsigned TabWidth,
  encoding::Encoding Encoding) {
+  DEBUG(llvm::dbgs() << "Comment split: \"" << Text << ", " << ColumnLimit
+ << "\", Content start: " << ContentStartColumn << "\n");
   if (ColumnLimit <= ContentStartColumn + 1)
 return BreakableToken::Split(StringRef::npos, 0);
 
@@ -171,7 +173,7 @@
 }
 
 unsigned
-BreakableToken::getLineLengthAfterCompression(unsigned RemainingTokenColumns,
+BreakableToken::getLengthAfterCompression(unsigned RemainingTokenColumns,
   Split Split) const {
   // Example: consider the content
   // lala  lala
@@ -181,50 +183,58 @@
   // We compute the number of columns when the split is compressed into a single
   // space, like:
   // lala lala
+  //
+  // FIXME: Correctly measure the length of whitespace in Split.second so it
+  // works with tabs.
   return RemainingTokenColumns + 1 - Split.second;
 }
 
-unsigned BreakableSingleLineToken::getLineCount() const { return 1; }
+unsigned BreakableStringLiteral::getLineCount() const { return 1; }
 
-unsigned BreakableSingleLineToken::getLineLengthAfterSplit(
-unsigned LineIndex, unsigned TailOffset,
-StringRef::size_type Length) const {
-  return StartColumn + Prefix.size() + Postfix.size() +
- encoding::columnWidthWithTabs(Line.substr(TailOffset, Length),
-   StartColumn + Prefix.size(),
-   Style.TabWidth, Encoding);
+unsigned BreakableStringLiteral::getRangeLength(unsigned LineIndex,
+unsigned Offset,
+StringRef::size_type Length,
+unsigned StartColumn) const {
+  assert(false &&
+ "Getting the length of a part of the string literal indicates that "
+ "the code tries to reflow it.");
 }
 
-BreakableSingleLineToken::BreakableSingleLineToken(
+unsigned
+BreakableStringLiteral::getRemainingLength(unsigned LineIndex, unsigned Offset,
+   unsigned StartColumn) const {
+  return UnbreakableTailLength + Postfix.size() +
+ encoding::columnWidthWithTabs(Line.substr(Offset, StringRef::npos),
+   StartColumn, Style.TabWidth, Encoding);
+}
+
+unsigned BreakableStringLiteral::getContentStartColumn(unsigned LineIndex,
+   bool Break) const {
+  return StartColumn + Prefix.size();
+}
+
+BreakableStringLiteral::BreakableStringLiteral(
 const FormatToken &Tok, unsigned StartColumn, StringRef Prefix,
 StringRef Postfix, bool InPPDirective, encoding::Encoding Encoding,
 const FormatStyle &Style)
 : BreakableToken(Tok, InPPDirective, Encoding, Style),
-  StartColumn(StartColumn), Prefix(Prefix), Postfix(Postfix) {
+  StartColumn(StartColumn), Prefix(Prefix), Postfix(Postfix),
+  UnbreakableTailLength(Tok.UnbreakableTailLength) {
   assert(Tok.TokenText.startswith(Prefix) && Tok.TokenText.endswith(Postfix));
   Line = Tok.TokenText.substr(
   Prefix.size(), Tok.TokenText.size() - Prefix.size() - Postfix.size());
 }
 
-BreakableStringLiteral::BreakableStringLiteral(
-const FormatToken &Tok, unsigned StartColumn, StringRef Prefix,
-StringRef Postfix, bool InPPDirective, encoding::Encoding Encoding,
-const FormatStyle &Style)
-: BreakableSingleLineToken(Tok, StartColumn, Prefix, Postfix, InPPDirective,
-   Encoding, Style) {}
-
-BreakableToken::Split
-BreakableStringLiteral::getSplit(unsigned LineIndex, unsigned TailOffset,
- unsigned ColumnLimit,
- llvm::Regex &CommentPragmasRegex) const {
-  return getStringSplit(Line.substr(TailOffset),
-StartColumn + Prefix.size() + Postfix.size(),
-ColumnLimit, Style.TabWidth, Encoding);
+BreakableToken::Split BreakableStringLiteral::getSplit(
+unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+unsigned ContentStartColumn, llvm::Regex &CommentPra

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 124739.
ioeric marked 4 inline comments as done.
ioeric added a comment.

- Address review comments.


https://reviews.llvm.org/D40563

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  lib/Sema/SemaCodeComplete.cpp

Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4603,9 +4603,19 @@
 
 void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
bool EnteringContext) {
-  if (!SS.getScopeRep() || !CodeCompleter)
+  if (SS.isEmpty() || !CodeCompleter)
 return;
 
+  // We want to keep the scope specifier even if it's invalid (e.g. the scope
+  // "a::b::" is not corresponding to any context/namespace in the AST), since
+  // it can be useful for global code completion which have information about
+  // contexts/symbols that are not in the AST.
+  if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CC.setCXXScopeSpecifier(SS);
+HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
+return;
+  }
   // Always pretend to enter a context to ensure that a dependent type
   // resolves to a dependent record.
   DeclContext *Ctx = computeDeclContext(SS, /*EnteringContext=*/true);
@@ -4621,7 +4631,7 @@
 CodeCompleter->getCodeCompletionTUInfo(),
 CodeCompletionContext::CCC_Name);
   Results.EnterNewScope();
-  
+
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   NestedNameSpecifier *NNS = SS.getScopeRep();
@@ -4635,16 +4645,18 @@
   // qualified-id completions.
   if (!EnteringContext)
 MaybeAddOverrideCalls(*this, Ctx, Results);
-  Results.ExitScope();  
-  
+  Results.ExitScope();
+
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
  /*IncludeGlobalScope=*/true,
  /*IncludeDependentBases=*/true);
 
-  HandleCodeCompleteResults(this, CodeCompleter, 
-Results.getCompletionContext(),
-Results.data(),Results.size());
+  auto CC = Results.getCompletionContext();
+  CC.setCXXScopeSpecifier(SS);
+
+  HandleCodeCompleteResults(this, CodeCompleter, CC, Results.data(),
+Results.size());
 }
 
 void Sema::CodeCompleteUsing(Scope *S) {
Index: include/clang/Sema/CodeCompleteConsumer.h
===
--- include/clang/Sema/CodeCompleteConsumer.h
+++ include/clang/Sema/CodeCompleteConsumer.h
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Type.h"
 #include "clang/Sema/CodeCompleteOptions.h"
+#include "clang/Sema/DeclSpec.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
@@ -280,6 +281,10 @@
   /// \brief The identifiers for Objective-C selector parts.
   ArrayRef SelIdents;
 
+  /// \brief The scope specifier that comes before the completion token e.g.
+  /// "a::b::"
+  llvm::Optional ScopeSpecifier;
+
 public:
   /// \brief Construct a new code-completion context of the given kind.
   CodeCompletionContext(enum Kind Kind) : Kind(Kind), SelIdents(None) { }
@@ -315,8 +320,20 @@
   /// \brief Determines whether we want C++ constructors as results within this
   /// context.
   bool wantConstructorResults() const;
-};
 
+  /// \brief Sets the scope specifier that comes before the completion token.
+  /// This is expected to be set in code completions on qualfied specifiers
+  /// (e.g. "a::b::").
+  void setCXXScopeSpecifier(CXXScopeSpec SS) {
+this->ScopeSpecifier = std::move(SS);
+  }
+
+  llvm::Optional getCXXScopeSpecifier() {
+if (ScopeSpecifier)
+  return ScopeSpecifier.getPointer();
+return llvm::None;
+  }
+};
 
 /// \brief A "string" used to describe how code completion can
 /// be performed for an entity.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:4609
 
+  if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > Why do we alter this code path?
> Maybe we should add a test or provide examples why the current code does not 
> work for us?
There is no existing unit test for CodeCompletion, and there is no much to test 
here either. I added a comment with example for this.



Comment at: lib/Sema/SemaCodeComplete.cpp:4611
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CC.setCXXScopeSpecifier(SS);
+HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);

ilya-biryukov wrote:
> Do we really want to set invalid scope specifiers here?
Yes, see the comment I added.


https://reviews.llvm.org/D40563



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

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 124740.
ioeric added a comment.

- Clarify comment for includeGlobals()


https://reviews.llvm.org/D40562

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaLookup.cpp
  test/CodeCompletion/ignore-global-decls.cpp


Index: test/CodeCompletion/ignore-global-decls.cpp
===
--- /dev/null
+++ test/CodeCompletion/ignore-global-decls.cpp
@@ -0,0 +1,20 @@
+namespace ns {
+  struct bar {
+  };
+
+  struct baz {
+  };
+
+  int func(int a, bar b, baz c);
+}
+
+void test() {
+  ns::
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:7 %s -o - | 
FileCheck %s --check-prefix=CHECK-1
+// CHECK-1-DAG: COMPLETION: bar : bar
+// CHECK-1-DAG: COMPLETION: baz : baz
+// CHECK-1-DAG: COMPLETION: func : [#int#]func(<#int a#>, <#bar b#>, <#baz c#>)
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:7 
-no-code-completion-globals %s -o - | FileCheck %s -allow-empty 
--check-prefix=CHECK-EMPTY
+// CHECK-EMPTY: {{^}}{{$}}
+}
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3785,8 +3785,12 @@
   LookupResult Result(*this, DeclarationName(), SourceLocation(), Kind);
   Result.setAllowHidden(Consumer.includeHiddenDecls());
   VisibleDeclsRecord Visited;
-  if (!IncludeGlobalScope)
+  if (!IncludeGlobalScope) {
 Visited.visitedContext(Context.getTranslationUnitDecl());
+// Declarations in namespace are also considered global.
+if (Ctx->isNamespace() || Ctx->isTranslationUnit())
+  Visited.visitedContext(Ctx);
+  }
   ShadowContextRAII Shadow(Visited);
   ::LookupVisibleDecls(Ctx, Result, /*QualifiedNameLookup=*/true,
/*InBaseClass=*/false, Consumer, Visited,
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4639,7 +4639,7 @@
   
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
- /*IncludeGlobalScope=*/true,
+ CodeCompleter->includeGlobals(),
  /*IncludeDependentBases=*/true);
 
   HandleCodeCompleteResults(this, CodeCompleter, 
Index: include/clang/Sema/CodeCompleteConsumer.h
===
--- include/clang/Sema/CodeCompleteConsumer.h
+++ include/clang/Sema/CodeCompleteConsumer.h
@@ -901,10 +901,9 @@
 return CodeCompleteOpts.IncludeCodePatterns;
   }
 
-  /// \brief Whether to include global (top-level) declaration results.
-  bool includeGlobals() const {
-return CodeCompleteOpts.IncludeGlobals;
-  }
+  /// \brief Whether to include global (top-level) declaration results. These
+  /// includes declarations in namespaces or translation units.
+  bool includeGlobals() const { return CodeCompleteOpts.IncludeGlobals; }
 
   /// \brief Whether to include brief documentation comments within the set of
   /// code completions returned.


Index: test/CodeCompletion/ignore-global-decls.cpp
===
--- /dev/null
+++ test/CodeCompletion/ignore-global-decls.cpp
@@ -0,0 +1,20 @@
+namespace ns {
+  struct bar {
+  };
+
+  struct baz {
+  };
+
+  int func(int a, bar b, baz c);
+}
+
+void test() {
+  ns::
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:7 %s -o - | FileCheck %s --check-prefix=CHECK-1
+// CHECK-1-DAG: COMPLETION: bar : bar
+// CHECK-1-DAG: COMPLETION: baz : baz
+// CHECK-1-DAG: COMPLETION: func : [#int#]func(<#int a#>, <#bar b#>, <#baz c#>)
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:7 -no-code-completion-globals %s -o - | FileCheck %s -allow-empty --check-prefix=CHECK-EMPTY
+// CHECK-EMPTY: {{^}}{{$}}
+}
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3785,8 +3785,12 @@
   LookupResult Result(*this, DeclarationName(), SourceLocation(), Kind);
   Result.setAllowHidden(Consumer.includeHiddenDecls());
   VisibleDeclsRecord Visited;
-  if (!IncludeGlobalScope)
+  if (!IncludeGlobalScope) {
 Visited.visitedContext(Context.getTranslationUnitDecl());
+// Declarations in namespace are also considered global.
+if (Ctx->isNamespace() || Ctx->isTranslationUnit())
+  Visited.visitedContext(Ctx);
+  }
   ShadowContextRAII Shadow(Visited);
   ::LookupVisibleDecls(Ctx, Result, /*QualifiedNameLookup=*/true,
/*InBaseClass=*/false, Consumer, Visited,
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4639,7 +4639,7 @@
   
   CodeCompletionDeclConsumer Consumer(Results,

[PATCH] D39812: [Driver, CodeGen] pass through and apply -fassociative-math

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

Ping * 2.


https://reviews.llvm.org/D39812



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

[PATCH] D40108: [clang-tidy] Adding Fuchsia checkers to clang-tidy

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

A couple of late comments.




Comment at: clang-tidy/fuchsia/DefaultArgumentsCheck.cpp:39
+  return;
+} else if (DefaultArgRange.getBegin().isMacroID()) {
+  diag(D->getLocStart(),

No `else` after `return`, please. 
http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return



Comment at: clang-tidy/fuchsia/DefaultArgumentsCheck.cpp:56
+  "declaring a parameter with a default argument is disallowed")
+  << D
+  << FixItHint::CreateRemoval(RemovalRange);

This argument is not used in the message. Does it have any effect at all?



Comment at: docs/clang-tidy/index.rst:64
 ``clang-analyzer-``Clang Static Analyzer checks.
+``fuchsia-``   Checks related to Fuchsia coding conventions.
 ``google-``Checks related to Google coding conventions.

How about adding a link to the coding style document? (We should do this for 
other entries here, but that's unrelated to your patch.)


https://reviews.llvm.org/D40108



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

[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location

[Strahinja Petrovic via Phabricator via cfe-commits]

spetrovic added a comment.

I tried to compile some important libraries for X86 and MIPS64 within Chromium 
with clang/llvm. I have compared results between LLVM trunk, and LLVM trunk 
with my patch. There is code size improvement on many libraries, here are some 
results:

- X86

libframe~1.5%
librendering~1.2%
liblayout_2 ~0.7%
libpaint_1  ~0.65%
libglslang  ~0.5%
libediting_3~0.5%
libcore_generated   ~0.5%
libanimation_1  ~0.5%

- Mips64

libglslang  ~3.2%
libframe~1.5%
liblayout_0 ~1.2%
libGLESv2   ~1%
librendering~0.7%
liblayout_1 ~0.5%
libcss_9~0.5%

Those are just some of libraries where we have code size improvement with this 
patch, I have also tried to compile LLVM with LLVM (with my patch) for x86 and 
MIPS64, and there is also code size improvement on both architectures. For 
example within clang executable for MIPS64 ~350 unaligned loads is removed. So, 
what do you think about it ?


https://reviews.llvm.org/D39053



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

[PATCH] D40453: Add the nvidia-cuda-toolkit Debian package path to search path

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC319317: Add the nvidia-cuda-toolkit  Debian package path to 
search path (authored by sylvestre).

Repository:
  rC Clang

https://reviews.llvm.org/D40453

Files:
  lib/Driver/ToolChains/Cuda.cpp


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -75,6 +75,11 @@
 CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda");
 for (const char *Ver : Versions)
   CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda-" + Ver);
+
+if (Distro(D.getVFS).IsDebian())
+  // Special case for Debian to have nvidia-cuda-toolkit work
+  // out of the box. More info on http://bugs.debian.org/882505
+  CudaPathCandidates.push_back(D.SysRoot + "/usr/lib/cuda");
   }
 
   for (const auto &CudaPath : CudaPathCandidates) {


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -75,6 +75,11 @@
 CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda");
 for (const char *Ver : Versions)
   CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda-" + Ver);
+
+if (Distro(D.getVFS).IsDebian())
+  // Special case for Debian to have nvidia-cuda-toolkit work
+  // out of the box. More info on http://bugs.debian.org/882505
+  CudaPathCandidates.push_back(D.SysRoot + "/usr/lib/cuda");
   }
 
   for (const auto &CudaPath : CudaPathCandidates) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r319317 - Add the nvidia-cuda-toolkit Debian package path to search path

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Wed Nov 29 07:03:28 2017
New Revision: 319317

URL: http://llvm.org/viewvc/llvm-project?rev=319317&view=rev
Log:
Add the nvidia-cuda-toolkit  Debian package path to search path

Summary:
Reported here:
http://bugs.debian.org/882505

Patch by Andreas Beckmann


Reviewers: Hahnfeld, tra

Reviewed By: tra

Subscribers: jlebar, cfe-commits

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Cuda.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Cuda.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Cuda.cpp?rev=319317&r1=319316&r2=319317&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Cuda.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Cuda.cpp Wed Nov 29 07:03:28 2017
@@ -75,6 +75,11 @@ CudaInstallationDetector::CudaInstallati
 CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda");
 for (const char *Ver : Versions)
   CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda-" + Ver);
+
+if (Distro(D.getVFS).IsDebian())
+  // Special case for Debian to have nvidia-cuda-toolkit work
+  // out of the box. More info on http://bugs.debian.org/882505
+  CudaPathCandidates.push_back(D.SysRoot + "/usr/lib/cuda");
   }
 
   for (const auto &CudaPath : CudaPathCandidates) {


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

[PATCH] D40594: [InstCombine] miscompile of __builtin_fmod

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

Side note: I think there is a different bug here in clang because from what I 
can tell, we convert the builtin or libcall to 'frem' even when errno could be 
set by the call. https://reviews.llvm.org/D40044 doesn't address this case 
because frem is an LLVM instruction rather than an LLVM intrinsic.


https://reviews.llvm.org/D40594



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

[PATCH] D40548: [clangd] Prototyping index support and naive index-based global code completion. WIP

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 124745.
ioeric added a comment.
Herald added a subscriber: klimek.

Merged with origin/master


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40548

Files:
  clangd/ASTIndex.cpp
  clangd/ASTIndex.h
  clangd/CMakeLists.txt
  clangd/ClangdIndex.cpp
  clangd/ClangdIndex.h
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/ClangdUnitStore.cpp
  clangd/ClangdUnitStore.h
  clangd/SymbolIndex.h
  clangd/tool/ClangdMain.cpp

Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -172,8 +172,7 @@
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory,
-CCOpts, ResourceDirRef,
-CompileCommandsDirPath);
+CCOpts, ResourceDirRef, CompileCommandsDirPath, {});
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
   return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErrorCode;
Index: clangd/SymbolIndex.h
===
--- /dev/null
+++ clangd/SymbolIndex.h
@@ -0,0 +1,57 @@
+//===--- CompletionIndex.h - Index for code completion ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPLETIONINDEX_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPLETIONINDEX_H
+
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+
+struct CompletionRequest {
+  std::string Query;
+  std::vector FixedPrefixes;
+};
+
+struct ScoreSignals {
+  float fuzzy_score;
+};
+
+struct CompletionSymbol {
+  ScoreSignals Signals;
+
+  std::string UID;
+  std::string QualifiedName;
+};
+
+struct CompletionResult {
+  //std::vector Symbol;
+  std::vector Symbols;
+  bool all_matched;
+};
+
+class SymbolIndex {
+public:
+  virtual ~SymbolIndex() = default;
+
+  virtual llvm::Expected
+  complete(const CompletionRequest &Req) const = 0;
+
+  virtual llvm::Expected
+  getSymbolInfo(llvm::StringRef UID) const = 0;
+
+  virtual llvm::Expected>
+  getAllOccurrences(llvm::StringRef UID) const = 0;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPLETIONINDEX_H
Index: clangd/ClangdUnitStore.h
===
--- clangd/ClangdUnitStore.h
+++ clangd/ClangdUnitStore.h
@@ -12,6 +12,7 @@
 
 #include 
 
+#include "ASTIndex.h"
 #include "ClangdUnit.h"
 #include "GlobalCompilationDatabase.h"
 #include "Path.h"
@@ -25,11 +26,14 @@
 /// Thread-safe mapping from FileNames to CppFile.
 class CppFileCollection {
 public:
+  explicit CppFileCollection(ASTIndexSourcer *IndexSourcer)
+  : IndexSourcer(IndexSourcer) {}
+
   std::shared_ptr
   getOrCreateFile(PathRef File, PathRef ResourceDir,
   GlobalCompilationDatabase &CDB, bool StorePreamblesInMemory,
   std::shared_ptr PCHs,
-  clangd::Logger &Logger) {
+  clangd::Logger &Logger, ASTIndexSourcer *IndexSourcer) {
 std::lock_guard Lock(Mutex);
 
 auto It = OpenedFiles.find(File);
@@ -39,7 +43,8 @@
   It = OpenedFiles
.try_emplace(File, CppFile::Create(File, std::move(Command),
   StorePreamblesInMemory,
-  std::move(PCHs), Logger))
+  std::move(PCHs), Logger,
+  IndexSourcer))
.first;
 }
 return It->second;
@@ -86,6 +91,7 @@
 
   std::mutex Mutex;
   llvm::StringMap> OpenedFiles;
+  ASTIndexSourcer *IndexSourcer;
 };
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdUnitStore.cpp
===
--- clangd/ClangdUnitStore.cpp
+++ clangd/ClangdUnitStore.cpp
@@ -23,6 +23,7 @@
 
   std::shared_ptr Result = It->second;
   OpenedFiles.erase(It);
+  IndexSourcer->remove(File);
   return Result;
 }
 
@@ -42,14 +43,15 @@
 It = OpenedFiles
  .try_emplace(File, CppFile::Create(File, std::move(NewCommand),
 StorePreamblesInMemory,
-std::move(PCHs), Logger))
+std::move(PCHs), Logger,
+IndexSourcer))
  

r319318 - Fix 'control reaches end of non-void' warning by using llvm_unreachable.

[Manuel Klimek via cfe-commits]

Author: klimek
Date: Wed Nov 29 07:09:12 2017
New Revision: 319318

URL: http://llvm.org/viewvc/llvm-project?rev=319318&view=rev
Log:
Fix 'control reaches end of non-void' warning by using llvm_unreachable.

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=319318&r1=319317&r2=319318&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Wed Nov 29 07:09:12 2017
@@ -195,9 +195,8 @@ unsigned BreakableStringLiteral::getRang
 unsigned Offset,
 StringRef::size_type Length,
 unsigned StartColumn) const {
-  assert(false &&
- "Getting the length of a part of the string literal indicates that "
- "the code tries to reflow it.");
+  llvm_unreachable("Getting the length of a part of the string literal "
+   "indicates that the code tries to reflow it.");
 }
 
 unsigned


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

[PATCH] D40527: [libclang] Record parsing invocation to a temporary file when requested by client

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added inline comments.



Comment at: tools/libclang/CIndexer.cpp:131
+  if (!File.empty())
+llvm::sys::fs::remove(File);
+}

Just a thought - since we are not propagating errors from constructor we are 
not really sure we were able to create the file in the first place (e. g. 
return from ctor at line 103). Should we still try to delete it?


Repository:
  rC Clang

https://reviews.llvm.org/D40527



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

r319319 - Follow up of r319317, add the missing header file

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Wed Nov 29 07:11:53 2017
New Revision: 319319

URL: http://llvm.org/viewvc/llvm-project?rev=319319&view=rev
Log:
Follow up of r319317, add the missing header file

Modified:
cfe/trunk/lib/Driver/ToolChains/Cuda.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Cuda.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Cuda.cpp?rev=319319&r1=319318&r2=319319&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Cuda.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Cuda.cpp Wed Nov 29 07:11:53 2017
@@ -13,6 +13,7 @@
 #include "clang/Basic/Cuda.h"
 #include "clang/Config/config.h"
 #include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Driver/Distro.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"


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

r319320 - [OPENMP] General improvement of handling of `teams distribute`

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Nov 29 07:14:16 2017
New Revision: 319320

URL: http://llvm.org/viewvc/llvm-project?rev=319320&view=rev
Log:
[OPENMP] General improvement of handling of `teams distribute`
directive, NFC.

Some general improvements in support of `teams distribute` directive.

Modified:
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=319320&r1=319319&r2=319320&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Wed Nov 29 07:14:16 2017
@@ -3893,7 +3893,7 @@ void CodeGenFunction::EmitOMPTeamsDistri
 CodeGenDistribute);
 CGF.EmitOMPReductionClauseFinal(S, /*ReductionKind=*/OMPD_teams);
   };
-  emitCommonOMPTeamsDirective(*this, S, OMPD_teams, CodeGen);
+  emitCommonOMPTeamsDirective(*this, S, OMPD_distribute, CodeGen);
   emitPostUpdateForReductionClause(*this, S,
[](CodeGenFunction &) { return nullptr; });
 }

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=319320&r1=319319&r2=319320&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed Nov 29 07:14:16 2017
@@ -7057,14 +7057,24 @@ StmtResult Sema::ActOnOpenMPTeamsDistrib
   // The point of exit cannot be a branch out of the structured block.
   // longjmp() and throw() must not violate the entry/exit criteria.
   CS->getCapturedDecl()->setNothrow();
+  for (int ThisCaptureLevel = getOpenMPCaptureLevels(OMPD_teams_distribute);
+   ThisCaptureLevel > 1; --ThisCaptureLevel) {
+CS = cast(CS->getCapturedStmt());
+// 1.2.2 OpenMP Language Terminology
+// Structured block - An executable statement with a single entry at the
+// top and a single exit at the bottom.
+// The point of exit cannot be a branch out of the structured block.
+// longjmp() and throw() must not violate the entry/exit criteria.
+CS->getCapturedDecl()->setNothrow();
+  }
 
   OMPLoopDirective::HelperExprs B;
   // In presence of clause 'collapse' with number of loops, it will
   // define the nested loops number.
   unsigned NestedLoopCount =
   CheckOpenMPLoop(OMPD_teams_distribute, getCollapseNumberExpr(Clauses),
-  nullptr /*ordered not a clause on distribute*/, AStmt,
-  *this, *DSAStack, VarsWithImplicitDSA, B);
+  nullptr /*ordered not a clause on distribute*/, CS, 
*this,
+  *DSAStack, VarsWithImplicitDSA, B);
   if (NestedLoopCount == 0)
 return StmtError();
 


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

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+  str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will also be garbage";

lebedev.ri wrote:
> dcoughlin wrote:
> > "Unary operator" is probably not something we should expect users to know 
> > about. How about just "The expression is an uninitialized value. The 
> > computed value will also be garbage."
> > 
> Yep, i did not like my wording either :)
A value being undefined does nt mean uninitialized. E.g.: the result of a bad 
shift operation might be UndefVal as well.
Aren't these diagnostics misleading in those cases? Or we do not care about 
those?


Repository:
  rC Clang

https://reviews.llvm.org/D40463



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

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

[Manuel Klimek via Phabricator via cfe-commits]

klimek created this revision.

When we break a long line like:
Column limit: 21

  |
  // foo foo foo foo foo foo foo foo foo foo foo foo

The local decision when to allow protruding vs. breaking can lead to this
outcome (2 excess characters, 2 breaks):

  // foo foo foo foo foo
  // foo foo foo foo foo
  // foo foo

While strictly staying within the column limit leads to this strictly better
outcome (fully below the column limit, 2 breaks):

  // foo foo foo foo
  // foo foo foo foo
  // foo foo foo foo

To get an optimal solution, we would need to consider all combinations of excess
characters vs. breaking for all lines, but that would lead to a significant
increase in the search space of the algorithm for little gain.

Instead, we blindly try both approches and·select the one that leads to the
overall lower penalty.


Repository:
  rC Clang

https://reviews.llvm.org/D40605

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9934,8 +9934,8 @@
   Style.PenaltyExcessCharacter = 90;
   verifyFormat("int a; // the comment", Style);
   EXPECT_EQ("int a; // the comment\n"
-"   // aa",
-format("int a; // the comment aa", Style));
+"   // aaa",
+format("int a; // the comment aaa", Style));
   EXPECT_EQ("int a; /* first line\n"
 "* second line\n"
 "* third line\n"
@@ -9963,14 +9963,14 @@
Style));
 
   EXPECT_EQ("// foo bar baz bazfoo\n"
-"// foo bar\n",
+"// foo bar foo bar\n",
 format("// foo bar baz bazfoo\n"
-   "// foobar\n",
+   "// foo bar foo   bar\n",
Style));
   EXPECT_EQ("// foo bar baz bazfoo\n"
-"// foo bar\n",
+"// foo bar foo bar\n",
 format("// foo bar baz  bazfoo\n"
-   "// foobar\n",
+   "// foobar foo bar\n",
Style));
 
   // FIXME: Optimally, we'd keep bazfoo on the first line and reflow bar to the
@@ -9996,6 +9996,15 @@
"// foo bar baz  bazfoo bar\n"
"// foo   bar\n",
Style));
+
+  // Make sure we do not keep protruding characters if strict mode reflow is
+  // cheaper than keeping protruding characters.
+  Style.ColumnLimit = 21;
+  EXPECT_EQ("// foo foo foo foo\n"
+"// foo foo foo foo\n"
+"// foo foo foo foo\n",
+format("// foo foo foo foo foo foo foo foo foo foo foo foo\n",
+   Style));
 }
 
 #define EXPECT_ALL_STYLES_EQUAL(Styles)\
Index: lib/Format/ContinuationIndenter.h
===
--- lib/Format/ContinuationIndenter.h
+++ lib/Format/ContinuationIndenter.h
@@ -123,14 +123,25 @@
   /// \brief If the current token sticks out over the end of the line, break
   /// it if possible.
   ///
-  /// \returns An extra penalty if a token was broken, otherwise 0.
+  /// \returns A pair (penalty, exceeded), where penalty is the extra penalty
+  /// when tokens are broken or lines exceed the column limit, and exceeded
+  /// indicates whether the algorithm purposefully left lines exceeding the
+  /// column limit.
   ///
   /// The returned penalty will cover the cost of the additional line breaks
   /// and column limit violation in all lines except for the last one. The
   /// penalty for the column limit violation in the last line (and in single
   /// line tokens) is handled in \c addNextStateToQueue.
-  unsigned breakProtrudingToken(const FormatToken &Current, LineState &State,
-bool AllowBreak, bool DryRun);
+  ///
+  /// \p Strict indicates whether reflowing is allowed to leave characters
+  /// protruding the column limit; if true, lines will be split strictly within
+  /// the column limit where possible; if false, words are allowed to protrude
+  /// over the column limit as long as the penalty is less than the penalty
+  /// of a break.
+  std::pair breakProtrudingToken(const FormatToken &Current,
+ LineState &State,
+ bool AllowBreak, bool DryRun,
+ bool Strict);
 
   /// \brief Returns the \c BreakableToken starting at \p Current, or nullptr
   /// if the current token cannot be broken.
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1390,7 +1390,36 @@

[PATCH] D40527: [libclang] Record parsing invocation to a temporary file when requested by client

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added inline comments.



Comment at: tools/libclang/CIndexer.cpp:108
+  // Write out the information about the invocation to it.
+  auto WriteStringKey = [&](StringRef Key, StringRef Value) {
+OS << R"(")" << Key << R"(":")";

Nit: Maybe capturing &OS is enough?


Repository:
  rC Clang

https://reviews.llvm.org/D40527



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

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Zoltán Gera via Phabricator via cfe-commits]

gerazo added a comment.

Thanks George for the review. I will start working on the code right away. I've 
tried to answer the simpler cases.




Comment at: tools/scan-build-py/libscanbuild/analyze.py:44
 
+CTU_FUNCTION_MAP_FILENAME = 'externalFnMap.txt'
+CTU_TEMP_FNMAP_FOLDER = 'tmpExternalFnMaps'

george.karpenkov wrote:
> What would happen when multiple analyzer runs are launched concurrently in 
> the same directory? Would they race on this file?
Yes and no. The 1st, collect part of CTU creates this file by aggregating all 
data from the build system, while the 2nd part which does the analysis itself 
only reads it. Multiple analysis can use this file simultaneously without 
problem. However, multiple collect phases launched on a system does not make 
sense. In this case, the later one would write over the previous results with 
the same data.



Comment at: tools/scan-build-py/libscanbuild/analyze.py:64
 if need_analyzer(args.build):
-run_analyzer_parallel(args)
+run_analyzer_with_ctu(args)
 else:

george.karpenkov wrote:
> `run_analyzer_with_ctu` is an unfortunate function name, since it may or may 
> not launch CTU depending on the passed arguments. Could we just call it 
> `run_analyzer`?
We have an other run_analyzer method but still, it is a good idead, I will make 
something better up.



Comment at: tools/scan-build-py/libscanbuild/analyze.py:103
 
+def prefix_with(constant, pieces):
+""" From a sequence create another sequence where every second element

george.karpenkov wrote:
> Actually I would prefer a separate NFC PR just moving this function. This PR 
> is already too complicated as it is.
Yes. The only reason was for the move to make it testable. However, we need 
more tests as you wrote down below.



Comment at: tools/scan-build-py/libscanbuild/analyze.py:120
+  func_map_cmd=args.func_map_cmd)
+if hasattr(args, 'ctu_phases') and hasattr(args.ctu_phases, 'dir')
+else CtuConfig(collect=False, analyze=False, dir='', func_map_cmd=''))

george.karpenkov wrote:
> Extensive `hasattr` usage is often a codesmell, and it hinders understanding.
> Firstly, shouldn't  `args.ctu_phases.dir` be always available, judging from 
> the code in `arguments.py`?
> Secondly, in what cases `args.ctu_phases` is not available when we are 
> already going down the CTU code path? Shouldn't we throw an error at that 
> point instead of creating a default configuration? 
> (default-configurations-instead-of-crashing is also another way to introduce 
> very subtle and hard-to-catch error modes)
It definitely needs more comments, so thanks, I will put them here.
There are two separate possibilities here for this code part:
1. The user does not want CTU at all. In this case, no CTU phases are switched 
on (collect and analyze), so no CTU code will run. This is why dir has no 
importance in this case.
2. CTU capabilities were not even detected, so the help and available arguments 
about CTU are also missing. In this case we create a dummy config telling that 
everything is switched off.

The reason for using hasattr was that not having CTU capabilities is not an 
exceptional condition, rather a matter of configuration. I felt that handling 
this with an exception would be misleading.



Comment at: tools/scan-build-py/libscanbuild/analyze.py:128
+A function map contains the id of a function (mangled name) and the
+originating source (the corresponding AST file) name."""
+

george.karpenkov wrote:
> Could you also specify what `func_map_lines` is (file? list? of strings?) in 
> the docstring? There's specific Sphinx syntax for that. Otherwise it is hard 
> to follow.
Thanks, I'll do it.



Comment at: tools/scan-build-py/libscanbuild/analyze.py:144
+if len(ast_files) == 1:
+mangled_ast_pairs.append((mangled_name, ast_files.pop()))
+

george.karpenkov wrote:
> Firstly, no need to modify the set in order to get the first element, just 
> use `next(iter(ast_files))`.
> Secondly, when exactly do conflicting names happen? Is it a bug? Shouldn't 
> the tool log such cases?
Nice catch, thanks.
For your second question: Unfortunately, it is not a bug, rather a misfeature 
which we can't handle yet. There can be tricky build-systems where a single 
file with different configurations is built multiple times, so the same 
function signature will be present in multiple link units. The other option is 
when two separate compile units have an exact same function signature, but they 
are never linked together, so it is not a problem in the build itself. In this 
case, the cross compilation unit functionality cannot tell exactly which 
compilation unit to turn to for the function, because there are multiple valid 
choices. In this ca

r319269 - Reland "Fix vtable not receiving hidden visibility when using push(visibility)"

[Jake Ehrlich via cfe-commits]

Author: jakehehrlich
Date: Tue Nov 28 16:54:20 2017
New Revision: 319269

URL: http://llvm.org/viewvc/llvm-project?rev=319269&view=rev
Log:
Reland "Fix vtable not receiving hidden visibility when using push(visibility)"

I had to reland this change in order to make the test work on windows

This change should resolve https://bugs.llvm.org/show_bug.cgi?id=35022

https://reviews.llvm.org/D39627

Added:
cfe/trunk/test/CodeGen/push-hidden-visibility-subclass.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGVTT.cpp
cfe/trunk/lib/CodeGen/CGVTables.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=319269&r1=319268&r2=319269&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Tue Nov 28 16:54:20 2017
@@ -240,7 +240,7 @@ llvm::Constant *CodeGenModule::getOrCrea
   getModule(), LTy, Ty.isConstant(getContext()), Linkage, Init, Name,
   nullptr, llvm::GlobalVariable::NotThreadLocal, TargetAS);
   GV->setAlignment(getContext().getDeclAlign(&D).getQuantity());
-  setGlobalVisibility(GV, &D);
+  setGlobalVisibility(GV, &D, ForDefinition);
 
   if (supportsCOMDAT() && GV->isWeakForLinker())
 GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));

Modified: cfe/trunk/lib/CodeGen/CGVTT.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTT.cpp?rev=319269&r1=319268&r2=319269&view=diff
==
--- cfe/trunk/lib/CodeGen/CGVTT.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTT.cpp Tue Nov 28 16:54:20 2017
@@ -100,7 +100,7 @@ CodeGenVTables::EmitVTTDefinition(llvm::
 VTT->setComdat(CGM.getModule().getOrInsertComdat(VTT->getName()));
 
   // Set the right visibility.
-  CGM.setGlobalVisibility(VTT, RD);
+  CGM.setGlobalVisibility(VTT, RD, ForDefinition);
 }
 
 llvm::GlobalVariable *CodeGenVTables::GetAddrOfVTT(const CXXRecordDecl *RD) {

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=319269&r1=319268&r2=319269&view=diff
==
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Tue Nov 28 16:54:20 2017
@@ -51,7 +51,7 @@ llvm::Constant *CodeGenModule::GetAddrOf
 
 static void setThunkVisibility(CodeGenModule &CGM, const CXXMethodDecl *MD,
const ThunkInfo &Thunk, llvm::Function *Fn) {
-  CGM.setGlobalVisibility(Fn, MD);
+  CGM.setGlobalVisibility(Fn, MD, ForDefinition);
 }
 
 static void setThunkProperties(CodeGenModule &CGM, const ThunkInfo &Thunk,
@@ -730,7 +730,7 @@ CodeGenVTables::GenerateConstructionVTab
   // Create the variable that will hold the construction vtable.
   llvm::GlobalVariable *VTable =
 CGM.CreateOrReplaceCXXRuntimeVariable(Name, VTType, Linkage);
-  CGM.setGlobalVisibility(VTable, RD);
+  CGM.setGlobalVisibility(VTable, RD, ForDefinition);
 
   // V-tables are always unnamed_addr.
   VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=319269&r1=319268&r2=319269&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Nov 28 16:54:20 2017
@@ -669,7 +669,8 @@ llvm::ConstantInt *CodeGenModule::getSiz
 }
 
 void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
-const NamedDecl *D) const {
+const NamedDecl *D,
+ForDefinition_t IsForDefinition) const 
{
   // Internal definitions always have default visibility.
   if (GV->hasLocalLinkage()) {
 GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
@@ -678,7 +679,8 @@ void CodeGenModule::setGlobalVisibility(
 
   // Set visibility for definitions.
   LinkageInfo LV = D->getLinkageAndVisibility();
-  if (LV.isVisibilityExplicit() || !GV->hasAvailableExternallyLinkage())
+  if (LV.isVisibilityExplicit() ||
+  (IsForDefinition && !GV->hasAvailableExternallyLinkage()))
 GV->setVisibility(GetLLVMVisibility(LV.getVisibility()));
 }
 
@@ -1059,7 +1061,7 @@ void CodeGenModule::SetLLVMFunctionAttri
 void CodeGenModule::SetCommonAttributes(const Decl *D,
 llvm::GlobalValue *GV) {
   if (const auto *ND = dyn_cast_or_null(D))
-setGlobalVisibility(GV, ND);
+setGlobalVisibility(GV, ND, ForDefinition);
   else
 GV->setVisib

r319322 - Fix function call to fix build

[Ismail Donmez via cfe-commits]

Author: ismail
Date: Wed Nov 29 07:18:02 2017
New Revision: 319322

URL: http://llvm.org/viewvc/llvm-project?rev=319322&view=rev
Log:
Fix function call to fix build

../tools/clang/lib/Driver/ToolChains/Cuda.cpp:80:18: error: reference to 
non-static member function must be called; did you mean to call it with no 
arguments?
if (Distro(D.getVFS).IsDebian())
   ~~^~
   ()


Modified:
cfe/trunk/lib/Driver/ToolChains/Cuda.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Cuda.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Cuda.cpp?rev=319322&r1=319321&r2=319322&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Cuda.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Cuda.cpp Wed Nov 29 07:18:02 2017
@@ -77,7 +77,7 @@ CudaInstallationDetector::CudaInstallati
 for (const char *Ver : Versions)
   CudaPathCandidates.push_back(D.SysRoot + "/usr/local/cuda-" + Ver);
 
-if (Distro(D.getVFS).IsDebian())
+if (Distro(D.getVFS()).IsDebian())
   // Special case for Debian to have nvidia-cuda-toolkit work
   // out of the box. More info on http://bugs.debian.org/882505
   CudaPathCandidates.push_back(D.SysRoot + "/usr/lib/cuda");


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

Re: [llvm-dev] LLVM buildmaster is back to work now but two builders remain OFF

[Greg Bedwell via cfe-commits]

> Two slaves remain off for now as they produce a lot of warnings and their
log files are way too big.

I think https://reviews.llvm.org/D40603 will fix this issue.

-Greg


On 28 November 2017 at 22:13, Galina Kistanova via llvm-dev <
llvm-...@lists.llvm.org> wrote:

> Hello everyone,
>
> LLVM buildmaster is back to work.
>
> Two slaves remain off for now as they produce a lot of warnings and their
> log files are way too big.
>
> The next builds have full logs available. Hope this helps to fix this
> issue:
>
> http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/8716
> http://lab.llvm.org:8011/builders/clang-with-thin-lto-windows/builds/2645
>
> Thanks
>
> Galina
>
> ___
> LLVM Developers mailing list
> llvm-...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7922-7925
+def err_type_tag_out_of_range : Error<
+  "type tag index %0 is greater than the number of arguments specified">;
+def err_arg_tag_out_of_range : Error<
+  "argument tag index %0 is greater than the number of arguments specified">;

These should be combined into a single diagnostic: `%select{type|argument}0 tag 
index %1 is greater than the number of arguments specified`



Comment at: include/clang/Sema/Sema.h:10458
   void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs);
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc);

`const Expr *` (space before the `*`). Same below in the definition.



Comment at: lib/Sema/SemaChecking.cpp:12345
   const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
+
   bool FoundWrongKind;

Spurious newline?



Comment at: lib/Sema/SemaChecking.cpp:12366
   const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
+
   if (IsPointerAttr) {

Spurious newline?



Comment at: test/Sema/error-type-safety.cpp:3-4
+
+static const int test_void
+  __attribute__((type_tag_for_datatype(test, void))) = 0;
+

Is this declaration necessary?



Comment at: test/Sema/error-type-safety.cpp:16
+  test_invalid_arg_index(0); // expected-error {{argument tag index 99 is 
greater than the number of arguments specified}}
+}

Can you also add a test for a boundary case so we can be sure there are no 
off-by-one errors introduced later?


https://reviews.llvm.org/D40574



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

r319323 - [Driver] Turns out the GNU assembler does support falkor/saphira.

[Chad Rosier via cfe-commits]

Author: mcrosier
Date: Wed Nov 29 08:42:44 2017
New Revision: 319323

URL: http://llvm.org/viewvc/llvm-project?rev=319323&view=rev
Log:
[Driver] Turns out the GNU assembler does support falkor/saphira.

Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/test/Driver/as-mcpu.c

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=319323&r1=319322&r2=319323&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Wed Nov 29 08:42:44 2017
@@ -51,9 +51,7 @@ static void normalizeCPUNamesForAssemble
 StringRef CPUArg(A->getValue());
 if (CPUArg.equals_lower("krait"))
   CmdArgs.push_back("-mcpu=cortex-a15");
-else if(CPUArg.equals_lower("kryo") ||
-CPUArg.equals_lower("falkor") ||
-CPUArg.equals_lower("saphira"))
+else if(CPUArg.equals_lower("kryo"))
   CmdArgs.push_back("-mcpu=cortex-a57");
 else
   Args.AddLastArg(CmdArgs, options::OPT_mcpu_EQ);

Modified: cfe/trunk/test/Driver/as-mcpu.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/as-mcpu.c?rev=319323&r1=319322&r2=319323&view=diff
==
--- cfe/trunk/test/Driver/as-mcpu.c (original)
+++ cfe/trunk/test/Driver/as-mcpu.c Wed Nov 29 08:42:44 2017
@@ -8,10 +8,4 @@
 // RUN: %clang -target arm-linux -mcpu=kryo -### -c %s -v -fno-integrated-as 
2>&1 | FileCheck -check-prefix=CHECK-CORTEX-A57 %s
 // RUN: %clang -target aarch64-linux -mcpu=kryo -### -c %s -v 
-fno-integrated-as 2>&1 | FileCheck -check-prefix=CHECK-CORTEX-A57 %s
 
-// RUN: %clang -target arm-linux -mcpu=falkor -### -c %s -v -fno-integrated-as 
2>&1 | FileCheck -check-prefix=CHECK-CORTEX-A57 %s
-// RUN: %clang -target aarch64-linux -mcpu=falkor -### -c %s -v 
-fno-integrated-as 2>&1 | FileCheck -check-prefix=CHECK-CORTEX-A57 %s
-
-// RUN: %clang -target arm-linux -mcpu=saphira -### -c %s -v 
-fno-integrated-as 2>&1 | FileCheck -check-prefix=CHECK-CORTEX-A57 %s
-// RUN: %clang -target aarch64-linux -mcpu=saphira -### -c %s -v 
-fno-integrated-as 2>&1 | FileCheck -check-prefix=CHECK-CORTEX-A57 %s
-
 // CHECK-CORTEX-A57: as{{(.exe)?}}" "{{.*}}-mcpu=cortex-a57


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

[PATCH] D40476: Switch kryo to use -mcpu=cortex-a57 when invoking the assembler

[Chad Rosier via Phabricator via cfe-commits]

mcrosier added a comment.

In https://reviews.llvm.org/D40476#936700, @mcrosier wrote:

> In https://reviews.llvm.org/D40476#936372, @pirama wrote:
>
> > Thanks for the review.  Now let's just hope the windows bots stay happy :)
>
>
> Actually, I just checked and it looks like falkor and saphira were both added 
> as of a few weeks ago.  I'll revert this part of the patch shortly.


Done so in r319323.


Repository:
  rL LLVM

https://reviews.llvm.org/D40476



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:58
+  // Interfaces should have exclusively pure methods.
+  for (auto method : Node->methods()) {
+if (method->isUserProvided() && !method->isPure()) {

Eugene.Zelenko wrote:
> const auto?
Should be `const auto *`; also `method` does not meet our naming convention 
rules. Actually, the whole thing can be replaced by `return 
llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) { return 
M->isUserProvided() && !M->isPure(); });`



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:21
+
+AST_MATCHER(CXXRecordDecl, hasDefinition) {
+  if (!Node.hasDefinition())

This is another example of an AST matcher that should simply be made public in 
ASTMatchers.h (as a separate commit). Feel free to add me as a reviewer to that 
patch (it should be pretty trivial).



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:29-30
+// previously.
+void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
+bool isInterface) {
+  StringRef Name = Node->getIdentifier()->getName();

Formatting is off here -- be sure to run the patch through clang-format.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:40
+bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
+bool *isInterface) {
+  StringRef Name = Node->getIdentifier()->getName();

Rather than passing a pointer, why not pass a reference?



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:49
+
+// TODO(smklein): Make an exception for 'Dispatcher' -- consider it an
+// interface, even though it isn't.

We don't put names into our TODO or FIXME comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:54
+  // Interfaces should have no fields.
+  if (!Node->field_empty()) {
+return false;

Can elide braces, here and elsewhere.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:102
+// concrete classes
+int NumConcrete = 0;
+for (const auto &I : D->bases()) {

Might as well make this `unsigned` rather than `int`.


https://reviews.llvm.org/D40580



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

[clang-tools-extra] r319325 - [clang-tidy] make readability-simplify-bool-expr completely ignore macros

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Nov 29 09:16:09 2017
New Revision: 319325

URL: http://llvm.org/viewvc/llvm-project?rev=319325&view=rev
Log:
[clang-tidy] make readability-simplify-bool-expr completely ignore macros

Modified:
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp?rev=319325&r1=319324&r2=319325&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
Wed Nov 29 09:16:09 2017
@@ -62,10 +62,7 @@ const char SimplifyConditionalReturnDiag
 const CXXBoolLiteralExpr *getBoolLiteral(const MatchFinder::MatchResult 
&Result,
  StringRef Id) {
   const auto *Literal = Result.Nodes.getNodeAs(Id);
-  return (Literal &&
-  Result.SourceManager->isMacroBodyExpansion(Literal->getLocStart()))
- ? nullptr
- : Literal;
+  return (Literal && Literal->getLocStart().isMacroID()) ? nullptr : Literal;
 }
 
 internal::Matcher returnsBool(bool Value, StringRef Id = "ignored") {

Modified: 
clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp?rev=319325&r1=319324&r2=319325&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp 
Wed Nov 29 09:16:09 2017
@@ -281,6 +281,8 @@ static constexpr bool truthy() {
 }
 
 #define HAS_XYZ_FEATURE true
+#define M1(what) M2(true, what)
+#define M2(condition, what) if (condition) what
 
 void macros_and_constexprs(int i = 0) {
   bool b = (i == 1);
@@ -295,9 +297,15 @@ void macros_and_constexprs(int i = 0) {
 // inline the macro first.
 i = 3;
   }
+  if (HAS_XYZ_FEATURE) {
+i = 5;
+  }
   i = 4;
+  M1(i = 7);
 }
 
+#undef HAS_XYZ_FEATURE
+
 bool conditional_return_statements(int i) {
   if (i == 0) return true; else return false;
 }


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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/AST/ASTContext.cpp:2213
+Field->isBitField()
+? Field->getBitWidthValue(Context)
+: Context.toBits(Context.getTypeSizeInChars(Field->getType()));

erichkeane wrote:
> rsmith wrote:
> > erichkeane wrote:
> > > erichkeane wrote:
> > > > rsmith wrote:
> > > > > This isn't quite right; you want min(bitfield length, bit size of 
> > > > > type) here. Eg, a `bool b : 8` or `int n : 1000` bitfield has padding 
> > > > > bits.
> > > > I guess I'm missing something with this comment... why would a bitfield 
> > > > with bool b: 8 have padding?
> > > > 
> > > > Additionally, isn't int n : 1000 illegal, since 1000 > 32 (# bits in an 
> > > > int)?
> > > > Both clang and GCC reject the 2nd case.  Curiously, GCC rejects bool b: 
> > > > 9, but Clang seems to allow 'bool' to have ANY size.  Is that an error 
> > > > itself?
> > > > 
> > > > Finally: Should we consider ANY bool to be not a unique object 
> > > > representation?  It has 1 bit of data, but 8 bits of storage.  GCC's 
> > > > implementation of this trait accepts them, but I'm second guessing that 
> > > > at the moment...
> > > I did a bit more looking into this... In the 'TYPE b: X' case, 'X' is 
> > > always the size of the field, so :
> > > 
> > > struct Bitfield {
> > >   bool b: 16;
> > > };
> > > static_assert(sizeof(Bitfield) == 2);
> > > 
> > > The rest of my padding detection works correctly.
> > Sounds like you've found more GCC bugs. In C++ (unlike in C), there is no 
> > restriction on the maximum length of a bit-field. Specifically, 
> > [class.bit]p1 says: "The value of the integral constant expression may be 
> > larger than the number of bits in the object
> > representation (6.7) of the bit-field’s type; in such cases the extra bits 
> > are padding bits (6.7)."
> > 
> > For non-`bool` bit-fields, or `bool` bit-fields with more than 8 bits, the 
> > extra bits are padding, and the type does not have unique object 
> > representations. You'll need to check for this.
> > 
> > For `bool` bit-fields of length 1, we obviously have unique object 
> > representations for that one bit.
> > 
> > The interesting case is `bool` bit-fields with length between 2 and 8 
> > inclusive. It would seem reasonable to assume they have unique 
> > representations, as we do for plain `bool` values, but this is really an 
> > ABI question (as, actually, is the plain `bool` case). The x86-64 psABI is 
> > not very clear on the bit-field question, but it does say "Booleans, when 
> > stored in a memory object, are stored as single byte objects the value of 
> > which is always 0 (false) or 1 (true).", so for at least x86-64, `bool`s of 
> > up to 8 bits should be considered to have unique object representations. 
> > The PPC64 psABI appears to say the same thing, but is also somewhat unclear 
> > about the bit-field case.
> Umm... so holy crap.  We actually reserve the object spece for the whole 
> thing, but ONLY the first sizeof(field) fields are actually stored to!  
> Everything else is truncated!
> 
> I think the correct solution is to do:
>// too large of a bitfield size causes truncation, and thus 'padding' bits.
>if (FieldSizeInBits > Context.getTypeSizeInBits(Field->getType()) return 
> false;
I suspect that the 'bool' 2-8 condition is a case where we can let the sleeping 
dogs lie here :)  Incoming patch has the other condition, where the bitfield 
size is greater than the type size.



Comment at: lib/AST/ASTContext.cpp:2277
+
+  return false;
+}

rsmith wrote:
> More cases to handle here:
> 
>  * vectors (careful about, eg, vector of 3 foo)
>  * `_Complex int` and friends
>  * `_Atomic T`
>  * Obj-C block pointers
>  * Obj-C object pointers
>  * and perhaps OpenCL's various builtin types (pipe, sampler_t, event_t, 
> clk_event_t, queue_t, reserve_id_t)
> 
> It's fine to leave these for another change, but we shouldn't forget about 
> them. (There're also Obj-C class types and the Obj-C selector type, but I 
> think it makes sense for those to return `false` here.)
I'm not sure what the correct answer is in any of those cases, since i'm not 
particularly familiar with any of them.  I'll put a 'fixme' with the contents 
of your comment in the code, and revisit it (or hope someone more knowledgable 
can).


https://reviews.llvm.org/D39347



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 124765.

https://reviews.llvm.org/D39347

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  lib/AST/ASTContext.cpp
  lib/AST/CXXABI.h
  lib/AST/ItaniumCXXABI.cpp
  lib/AST/MicrosoftCXXABI.cpp
  lib/AST/Type.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/type-traits.cpp

Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2201,150 +2201,6 @@
   return false;
 }
 
-bool QualType::unionHasUniqueObjectRepresentations(
-const ASTContext &Context) const {
-  assert((*this)->isUnionType() && "must be union type");
-  CharUnits UnionSize = Context.getTypeSizeInChars(*this);
-  const RecordDecl *Union = getTypePtr()->getAs()->getDecl();
-
-  for (const auto *Field : Union->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-if (FieldSize != UnionSize)
-  return false;
-  }
-  return true;
-}
-
-static bool isStructEmpty(QualType Ty) {
-  assert(Ty.getTypePtr()->isStructureOrClassType() &&
- "Must be struct or class");
-  const RecordDecl *RD = Ty.getTypePtr()->getAs()->getDecl();
-
-  if (!RD->field_empty())
-return false;
-
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-return ClassDecl->isEmpty();
-  }
-
-  return true;
-}
-
-bool QualType::structHasUniqueObjectRepresentations(
-const ASTContext &Context) const {
-  assert((*this)->isStructureOrClassType() && "Must be struct or class");
-  const RecordDecl *RD = getTypePtr()->getAs()->getDecl();
-
-  if (isStructEmpty(*this))
-return false;
-
-  // Check base types.
-  CharUnits BaseSize{};
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-for (const auto Base : ClassDecl->bases()) {
-  if (Base.isVirtual())
-return false;
-
-  // Empty bases are permitted, otherwise ensure base has unique
-  // representation. Also, Empty Base Optimization means that an
-  // Empty base takes up 0 size.
-  if (!isStructEmpty(Base.getType())) {
-if (!Base.getType().structHasUniqueObjectRepresentations(Context))
-  return false;
-BaseSize += Context.getTypeSizeInChars(Base.getType());
-  }
-}
-  }
-
-  CharUnits StructSize = Context.getTypeSizeInChars(*this);
-
-  // This struct obviously has bases that keep it from being 'empty', so
-  // checking fields is no longer required.  Ensure that the struct size
-  // is the sum of the bases.
-  if (RD->field_empty())
-return StructSize == BaseSize;
-
-  CharUnits CurOffset =
-  Context.toCharUnitsFromBits(Context.getFieldOffset(*RD->field_begin()));
-
-  // If the first field isn't at the sum of the size of the bases, there
-  // is padding somewhere.
-  if (BaseSize != CurOffset)
-return false;
-
-  for (const auto *Field : RD->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-CharUnits FieldOffset =
-Context.toCharUnitsFromBits(Context.getFieldOffset(Field));
-// Has padding between fields.
-if (FieldOffset != CurOffset)
-  return false;
-CurOffset += FieldSize;
-  }
-  // Check for tail padding.
-  return CurOffset == StructSize;
-}
-
-bool QualType::hasUniqueObjectRepresentations(const ASTContext &Context) const {
-  // C++17 [meta.unary.prop]:
-  //   The predicate condition for a template specialization
-  //   has_unique_object_representations shall be
-  //   satisfied if and only if:
-  // (9.1) - T is trivially copyable, and
-  // (9.2) - any two objects of type T with the same value have the same
-  // object representation, where two objects
-  //   of array or non-union class type are considered to have the same value
-  //   if their respective sequences of
-  //   direct subobjects have the same values, and two objects of union type
-  //   are considered to have the same
-  //   value if they have the same active member and the corresponding members
-  //   have the same value.
-  //   The set of scalar types for which this condition holds is
-  //   implementation-defined. [ Note: If a type has padding
-  //   bits, the condition does not hold; otherwise, the condition holds true
-  //   for unsigned integral types. -- end note ]
-  if (isNull())
-return false;
-
-  // Arrays are unique only if their element type is unique.
-  if ((*this)->isArrayType())
-return Context.getBaseElementType(*this).hasUniqueObjectRepresentations(
-Context);
-
-  // (9.1) - T is trivially copyable, and
-  if (!isTriviallyCopyableType(Context))
-return false;
-
-  // Functions are not unique.
-  if ((*this)->isFunctionType())
-return false;
-
-  // All integrals and enums are unique!
-  if ((*this)->isIntegralOrEnumerationType())
-return true;
-
-  // All pointe

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/ClangdUnit.cpp:1173
+H.range = L.range;
+Ref = getDataBufferFromSourceRange(AST, SR, L);
+  }

malaperle wrote:
> malaperle wrote:
> > I get the same crash as I mentioned before if I hover on the class in 
> > "isa(D)", from Clangdunit.cpp (disclaimer: I'm testing with 
> > older source so it might not be there anymore.)
> > 
> > I have still yet to investigate this.
> I think it has to do with macro expansions, the plot thickens..
ObjCMethodDecl is forward-declared in astfwd.h as part of a macro expansion. We 
have to either use the expansion loc or spelling loc to get the appropriate 
file entry. Probably expansion loc makes more sense but I'll test a bit.


https://reviews.llvm.org/D35894



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 124769.
erichkeane added a comment.

Woops, missed an 'svn add' and lost the member ptr test.  Back now!


https://reviews.llvm.org/D39347

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  lib/AST/ASTContext.cpp
  lib/AST/CXXABI.h
  lib/AST/ItaniumCXXABI.cpp
  lib/AST/MicrosoftCXXABI.cpp
  lib/AST/Type.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/has_unique_object_reps_member_ptr.cpp
  test/SemaCXX/type-traits.cpp

Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2201,150 +2201,6 @@
   return false;
 }
 
-bool QualType::unionHasUniqueObjectRepresentations(
-const ASTContext &Context) const {
-  assert((*this)->isUnionType() && "must be union type");
-  CharUnits UnionSize = Context.getTypeSizeInChars(*this);
-  const RecordDecl *Union = getTypePtr()->getAs()->getDecl();
-
-  for (const auto *Field : Union->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-if (FieldSize != UnionSize)
-  return false;
-  }
-  return true;
-}
-
-static bool isStructEmpty(QualType Ty) {
-  assert(Ty.getTypePtr()->isStructureOrClassType() &&
- "Must be struct or class");
-  const RecordDecl *RD = Ty.getTypePtr()->getAs()->getDecl();
-
-  if (!RD->field_empty())
-return false;
-
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-return ClassDecl->isEmpty();
-  }
-
-  return true;
-}
-
-bool QualType::structHasUniqueObjectRepresentations(
-const ASTContext &Context) const {
-  assert((*this)->isStructureOrClassType() && "Must be struct or class");
-  const RecordDecl *RD = getTypePtr()->getAs()->getDecl();
-
-  if (isStructEmpty(*this))
-return false;
-
-  // Check base types.
-  CharUnits BaseSize{};
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-for (const auto Base : ClassDecl->bases()) {
-  if (Base.isVirtual())
-return false;
-
-  // Empty bases are permitted, otherwise ensure base has unique
-  // representation. Also, Empty Base Optimization means that an
-  // Empty base takes up 0 size.
-  if (!isStructEmpty(Base.getType())) {
-if (!Base.getType().structHasUniqueObjectRepresentations(Context))
-  return false;
-BaseSize += Context.getTypeSizeInChars(Base.getType());
-  }
-}
-  }
-
-  CharUnits StructSize = Context.getTypeSizeInChars(*this);
-
-  // This struct obviously has bases that keep it from being 'empty', so
-  // checking fields is no longer required.  Ensure that the struct size
-  // is the sum of the bases.
-  if (RD->field_empty())
-return StructSize == BaseSize;
-
-  CharUnits CurOffset =
-  Context.toCharUnitsFromBits(Context.getFieldOffset(*RD->field_begin()));
-
-  // If the first field isn't at the sum of the size of the bases, there
-  // is padding somewhere.
-  if (BaseSize != CurOffset)
-return false;
-
-  for (const auto *Field : RD->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-CharUnits FieldOffset =
-Context.toCharUnitsFromBits(Context.getFieldOffset(Field));
-// Has padding between fields.
-if (FieldOffset != CurOffset)
-  return false;
-CurOffset += FieldSize;
-  }
-  // Check for tail padding.
-  return CurOffset == StructSize;
-}
-
-bool QualType::hasUniqueObjectRepresentations(const ASTContext &Context) const {
-  // C++17 [meta.unary.prop]:
-  //   The predicate condition for a template specialization
-  //   has_unique_object_representations shall be
-  //   satisfied if and only if:
-  // (9.1) - T is trivially copyable, and
-  // (9.2) - any two objects of type T with the same value have the same
-  // object representation, where two objects
-  //   of array or non-union class type are considered to have the same value
-  //   if their respective sequences of
-  //   direct subobjects have the same values, and two objects of union type
-  //   are considered to have the same
-  //   value if they have the same active member and the corresponding members
-  //   have the same value.
-  //   The set of scalar types for which this condition holds is
-  //   implementation-defined. [ Note: If a type has padding
-  //   bits, the condition does not hold; otherwise, the condition holds true
-  //   for unsigned integral types. -- end note ]
-  if (isNull())
-return false;
-
-  // Arrays are unique only if their element type is unique.
-  if ((*this)->isArrayType())
-return Context.getBaseElementType(*this).hasUniqueObjectRepresentations(
-Context);
-
-  // (9.1) - T is trivially copyable, and
-  if (!isTriviallyCopyableType(Context))
-return false;
-
-  // Functions are not unique.
-  if ((*this)->isFunctionTyp

[PATCH] D40611: Add has definition AST matcher

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
Herald added a subscriber: klimek.

Adds AST matcher for the definition of a CXXRecordDecl.


https://reviews.llvm.org/D40611

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2008,5 +2008,26 @@
   cxxNewExpr(hasArraySize(integerLiteral(equals(10));
 }
 
+TEST(HasDefinition, MatchesStructDefinition) {
+  EXPECT_TRUE(matches("struct x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("struct x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
+TEST(HasDefinition, MatchesClassDefinition) {
+  EXPECT_TRUE(matches("class x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("class x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
+TEST(HasDefinition, MatchesUnionDefinition) {
+  EXPECT_TRUE(matches("union x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("union x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -250,6 +250,7 @@
   REGISTER_MATCHER(hasDeclContext);
   REGISTER_MATCHER(hasDeducedType);
   REGISTER_MATCHER(hasDefaultArgument);
+  REGISTER_MATCHER(hasDefinition);
   REGISTER_MATCHER(hasDescendant);
   REGISTER_MATCHER(hasDestinationType);
   REGISTER_MATCHER(hasDynamicExceptionSpec);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -5853,6 +5853,17 @@
 InnerMatcher.matches(*Node.getArraySize(), Finder, Builder);
 }
 
+/// \brief Matches a declaration that is defined.
+///
+/// Example matches x (matcher = cxxRecordDecl(hasDefinition()))
+/// \code
+/// class x {};
+/// class y;
+/// \endcode
+AST_MATCHER(CXXRecordDecl, hasDefinition) {
+  return Node.hasDefinition();
+}
+
 } // namespace ast_matchers
 } // namespace clang
 
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -2307,6 +2307,15 @@
 
 
 
+MatcherCXXRecordDecl>hasDefinition
+Matches a declaration 
that is defined.
+
+Example matches x (matcher = cxxRecordDecl(hasDefinition()))
+class x {};
+class y;
+
+
+
 MatcherCXXRecordDecl>isDerivedFromstd::string 
BaseName
 Overloaded method as 
shortcut for isDerivedFrom(hasName(...)).
 


Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2008,5 +2008,26 @@
   cxxNewExpr(hasArraySize(integerLiteral(equals(10));
 }
 
+TEST(HasDefinition, MatchesStructDefinition) {
+  EXPECT_TRUE(matches("struct x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("struct x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
+TEST(HasDefinition, MatchesClassDefinition) {
+  EXPECT_TRUE(matches("class x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("class x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
+TEST(HasDefinition, MatchesUnionDefinition) {
+  EXPECT_TRUE(matches("union x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("union x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -250,6 +250,7 @@
   REGISTER_MATCHER(hasDeclContext);
   REGISTER_MATCHER(hasDeducedType);
   REGISTER_MATCHER(hasDefaultArgument);
+  REGISTER_MATCHER(hasDefinition);
   REGISTER_MATCHER(hasDescendant);
   REGISTER_MATCHER(hasDestinationType);
   REGISTER_MATCHER(hasDynamicExceptionSpec);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMat

[PATCH] D39376: [PowerPC] Add implementation for -msave-toc-indirect option - clang portion

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.



Comment at: include/clang/Driver/Options.td:1907
 def mvsx : Flag<["-"], "mvsx">, Group;
+def msave_toc_indirect : Flag<["-"], "msave-toc-indirect">, 
Group;
 def mno_vsx : Flag<["-"], "mno-vsx">, Group;

hfinkel wrote:
> You also need mno_save_toc_indirect here too.
nit: The new option splits the mvsx and mno_vsx pair. I think we should keep 
the related options together. 


https://reviews.llvm.org/D39376



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

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1046
 if (V2_untested.isUnknownOrUndef()) {
   Bldr.generateNode(U, *I, state->BindExpr(U, LCtx, V2_untested));
+

lebedev.ri wrote:
> dcoughlin wrote:
> > Instead of generating a node node here, you'll want to generate a new state:
> > ```
> > state = state->BindExpr(U, LCtx, V2_untested);
> > ```
> > and use that state in the call to evalStore().
> > 
> > Otherwise, you'll end up exploring from both the new generated node and 
> > from *I.
> > 
> > Here is a test that demonstrates why this is a problem. You should add it 
> > to your tests.
> > 
> > ```
> > void foo() {
> >   int x;
> > 
> >   x++; // expected-warning {{The expression of the unary operator is an 
> > uninitialized value. The computed value will also be garbage}}
> > 
> >   clang_analyzer_warnIfReached(); // no-warning
> > 
> > ```
> > The undefined assignment checker generates what we call "fatal" errors. 
> > These errors are so bad that it decides not to explore further on that path 
> > to report additional errors. This means that the analyzer should treat any 
> > code that is dominated by such an error as not reachable. You'll need to 
> > add the `debug.ExprInspection` checker to your test RUN line and a 
> > prototype for clang_analyzer_warnIfReached() for this test to to work.
> > 
> > Instead of generating a node node here, you'll want to generate a new state:
> > ```
> > state = state->BindExpr(U, LCtx, V2_untested);
> > ```
> > and use that state in the call to evalStore().
> Hm, so the only change needed here is
> ```diff
> - Bldr.generateNode(U, *I, state->BindExpr(U, LCtx, V2_untested));
> + state = state->BindExpr(U, LCtx, V2_untested);
> ```
> ?
Yes, that's right. What you have here looks good to me.



Comment at: test/Analysis/malloc-plist.c:13
 int *p = malloc(12);
-(*p)++;
+(*p)++; // expected-warning {{The expression of the unary operator is 
an uninitialized value. The computed value will also be garbage}}
+*p = 0; // no-warning

lebedev.ri wrote:
> dcoughlin wrote:
> > Once you change the core modeling to not generate an extra node, you'll 
> > want to initialize `*p` to `0` or something to preserve the intent of this 
> > test. For this test it is important that the increment not stop further 
> > exploration of the path.
> Hmm, *this* test did not break after i changed 
> `ExprEngine::VisitIncrementDecrementOperator()`.
> Did i change it wrong?
Aah, my mistake. I though this test was passing -verify (which verifies 
expected-warning{{}} contents). Instead, it is only checking the plists, so the 
missing expected leak would only be caught by the plist change.

I suggest changing the test to:
```
...
if (in > 5) {
int *p = malloc(12);
*p = 0;
(*p)++;
}
...
```

So that the test doesn't generate a fatal error for the access to uninitialized 
memory and instead continues to check for the path of the leak.



Comment at: test/Analysis/malloc-plist.c:112
+(*p)++; // expected-warning {{The expression is an uninitialized value. 
The computed value will also be garbage}}
+*p = 0; // no-warning
+(*p)++; // no-warning

Here you should stick the `*p = 0;` before the post increment so the rest of 
the code is exercised.


Repository:
  rC Clang

https://reviews.llvm.org/D40463



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

Re: r319297 - Toolchain: Normalize dwarf, sjlj and seh eh

[Reid Kleckner via cfe-commits]

I see a bunch of link errors that suggest we're using the wrong EH
personality in MSVC mode now:

FAILED: win_clang_nacl_win64/swiftshader/libEGL.dll
win_clang_nacl_win64/swiftshader/libEGL.dll.lib
win_clang_nacl_win64/swiftshader/libEGL.dll.pdb
C:/b/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe
../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x64
False link.exe /nologo
/IMPLIB:win_clang_nacl_win64/swiftshader/libEGL.dll.lib /DLL
/OUT:win_clang_nacl_win64/swiftshader/libEGL.dll
/PDB:win_clang_nacl_win64/swiftshader/libEGL.dll.pdb
@win_clang_nacl_win64/swiftshader/libEGL.dll.rsp
Config.obj : error LNK2001: unresolved external symbol _Unwind_Resume
Display.obj : error LNK2001: unresolved external symbol _Unwind_Resume
Surface.obj : error LNK2001: unresolved external symbol _Unwind_Resume
Config.obj : error LNK2001: unresolved external symbol
__gxx_personality_seh0
Display.obj : error LNK2001: unresolved external symbol
__gxx_personality_seh0
Surface.obj : error LNK2001: unresolved external symbol
__gxx_personality_seh0

I'll dig in a bit to see if there's an easy fix.

On Tue, Nov 28, 2017 at 11:25 PM, Martell Malone via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: martell
> Date: Tue Nov 28 23:25:12 2017
> New Revision: 319297
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319297&view=rev
> Log:
> Toolchain: Normalize dwarf, sjlj and seh eh
>
> This is a re-apply of r319294.
>
> adds -fseh-exceptions and -fdwarf-exceptions flags
>
> clang will check if the user has specified an exception model flag,
> in the absense of specifying the exception model clang will then check
> the driver default and append the model flag for that target to cc1
>
> -fno-exceptions has a higher priority then specifying the model
>
> move __SEH__ macro definitions out of Targets into InitPreprocessor
> behind the -fseh-exceptions flag
>
> move __ARM_DWARF_EH__ macrodefinitions out of verious targets and into
> InitPreprocessor behind the -fdwarf-exceptions flag and arm|thumb check
>
> remove unused USESEHExceptions from the MinGW Driver
>
> fold USESjLjExceptions into a new GetExceptionModel function that
> gives the toolchain classes more flexibility with eh models
>
> Reviewers: rnk, mstorsjo
>
> Differential Revision: https://reviews.llvm.org/D39673
>
> Added:
> cfe/trunk/test/CodeGenCXX/mingw-w64-exceptions.c
> Modified:
> cfe/trunk/docs/ClangCommandLineReference.rst
> cfe/trunk/include/clang/Basic/LangOptions.def
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/include/clang/Driver/ToolChain.h
> cfe/trunk/lib/Basic/Targets/ARM.cpp
> cfe/trunk/lib/Basic/Targets/OSTargets.cpp
> cfe/trunk/lib/Basic/Targets/OSTargets.h
> cfe/trunk/lib/Basic/Targets/X86.h
> cfe/trunk/lib/CodeGen/BackendUtil.cpp
> cfe/trunk/lib/CodeGen/CGException.cpp
> cfe/trunk/lib/Driver/ToolChain.cpp
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
> cfe/trunk/lib/Driver/ToolChains/Darwin.h
> cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
> cfe/trunk/lib/Driver/ToolChains/FreeBSD.h
> cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
> cfe/trunk/lib/Driver/ToolChains/MinGW.h
> cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
> cfe/trunk/lib/Driver/ToolChains/NetBSD.h
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> cfe/trunk/lib/Frontend/InitPreprocessor.cpp
> cfe/trunk/test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
> cfe/trunk/test/Preprocessor/arm-target-features.c
> cfe/trunk/test/Preprocessor/init.c
>
> Modified: cfe/trunk/docs/ClangCommandLineReference.rst
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/
> ClangCommandLineReference.rst?rev=319297&r1=319296&r2=319297&view=diff
> 
> ==
> --- cfe/trunk/docs/ClangCommandLineReference.rst (original)
> +++ cfe/trunk/docs/ClangCommandLineReference.rst Tue Nov 28 23:25:12 2017
> @@ -1706,10 +1706,18 @@ Which overload candidates to show when o
>
>  Enable C++14 sized global deallocation functions
>
> +.. option:: -fdwarf-exceptions
> +
> +Use DWARF style exceptions
> +
>  .. option:: -fsjlj-exceptions
>
>  Use SjLj style exceptions
>
> +.. option:: -fseh-exceptions
> +
> +Use SEH style exceptions
> +
>  .. option:: -fslp-vectorize, -fno-slp-vectorize, -ftree-slp-vectorize
>
>  Enable the superword-level parallelism vectorization passes
>
> Modified: cfe/trunk/include/clang/Basic/LangOptions.def
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/LangOptions.def?rev=319297&r1=319296&r2=319297&view=diff
> 
> ==
> --- cfe/trunk/include/clang/Basic/LangOptions.def (original)
> +++ cfe/trunk/include/clang/Basic/LangOptions.def Tue Nov 28 23:25:12 2017
> @@ -124,7 +124,9 @@ LANGOPT(ZVector   , 1, 0, "Syste
>  LANGOPT(Exceptions, 1, 0, "exception handl

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124772.
juliehockett marked 11 inline comments as done.
juliehockett added a comment.

Moved AST matcher to ASTMatchers.h (see D40611 
), addressing comments.


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s fuchsia-multiple-inheritance %t
+
+class Base_A {
+public:
+  virtual int foo() { return 0; }
+};
+
+class Base_B {
+public:
+  virtual int bar() { return 0; }
+};
+
+class Base_A_child : public Base_A {
+public:
+  virtual int baz() { return 0; }
+};
+
+class Interface_A {
+public:
+  virtual int foo() = 0;
+};
+
+class Interface_B {
+public:
+  virtual int bar() = 0;
+};
+
+class Interface_C {
+public:
+  virtual int blat() = 0;
+};
+
+class Interface_A_with_member {
+public:
+  virtual int foo() = 0;
+  int val = 0;
+};
+
+class Interface_with_A_Parent : public Base_A {
+public:
+  virtual int baz() = 0;
+};
+
+// Inherits from multiple concrete classes.
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};
+class Bad_Child1 : public Base_A, Base_B {};
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+class Bad_Child2 : public Base_A, Interface_A_with_member {
+  virtual int foo() override { return 0; }
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+  virtual int baz() override { return 0; }
+};
+
+// Easy cases of single inheritance
+class Simple_Child1 : public Base_A {};
+class Simple_Child2 : public Interface_A {
+  virtual int foo() override { return 0; }
+};
+
+// Valid uses of multiple inheritance
+class Good_Child1 : public Interface_A, Interface_B {
+  virtual int foo() override { return 0; }
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child2 : public Base_A, Interface_B {
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child3 : public Base_A_child, Interface_C, Interface_B {
+  virtual int bar() override { return 0; }
+  virtual int blat() override { return 0; }
+};
+
+int main(void) {
+  Bad_Child1 a;
+  Bad_Child2 b;
+  Bad_Child3 c;
+  Simple_Child1 d;
+  Simple_Child2 e;
+  Good_Child1 f;
+  Good_Child2 g;
+  Good_Child3 h;
+  return 0;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -69,6 +69,7 @@
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
fuchsia-default-arguments
+   fuchsia-multiple-inheritance
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
Index: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - fuchsia-multiple-inheritance
+
+fuchsia-multiple-inheritance
+
+
+Warns if a class inherits from multiple classes that are not pure virtual.
+
+For example, declaring a class that inherits from multiple concrete classes is
+disallowed:
+
+.. code-block:: c++
+
+  class Base_A {
+  public:
+virtual int foo() { return 0; }
+  };
+
+  class Base_B {
+  public:
+virtual int bar() { return 0; }
+  };
+
+  // Warning
+  class Bad_Child1 : public Base_A, Base_B {};
+
+A class that inherits from a pure virtual is allowed:
+
+.. code-block:: c++
+
+  class Interface_A {
+  public:
+virtual int foo() = 0;
+  };
+
+  class Interface_B {
+  public:
+virtual int bar() = 0;
+  };
+
+  // No warning
+  class Good_Child1 : public Interface_A, Interface_B {
+virtual int foo() override { return 0; }
+virtual int bar() override { return 0; }
+  };
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -1

[PATCH] D40566: Check if MemberExpr arguments are type-dependent.

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Hi, thanks for working on this!
Why don't we just set the TypeDependent bit to false when building the 
underlying MemberExpr here? Based on the section of the standard you quoted 
that node shouldn't be considered type-dependent, but it is by clang. I think 
this patch is just hiding that underlying problem by fishing out the real 
type-dependence after constructing the MemberExpr when really that information 
should be included in the MemberExpr to begin with.
Thanks,
Erik


https://reviews.llvm.org/D40566



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

r319332 - [OPENMP] Do not allow `linear` clauses on non-simd distribute

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Nov 29 10:20:04 2017
New Revision: 319332

URL: http://llvm.org/viewvc/llvm-project?rev=319332&view=rev
Log:
[OPENMP] Do not allow `linear` clauses on non-simd distribute
directives.

`linear` clause is not allowed on non-simd distribute-based directives.

Removed:
cfe/trunk/test/OpenMP/distribute_parallel_for_linear_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_linear_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_linear_messages.cpp
Modified:
cfe/trunk/include/clang/Basic/OpenMPKinds.def
cfe/trunk/test/OpenMP/distribute_parallel_for_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_ast_print.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_ast_print.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_messages.cpp

Modified: cfe/trunk/include/clang/Basic/OpenMPKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/OpenMPKinds.def?rev=319332&r1=319331&r2=319332&view=diff
==
--- cfe/trunk/include/clang/Basic/OpenMPKinds.def (original)
+++ cfe/trunk/include/clang/Basic/OpenMPKinds.def Wed Nov 29 10:20:04 2017
@@ -610,7 +610,6 @@ OPENMP_DISTRIBUTE_PARALLEL_FOR_CLAUSE(sh
 OPENMP_DISTRIBUTE_PARALLEL_FOR_CLAUSE(reduction)
 OPENMP_DISTRIBUTE_PARALLEL_FOR_CLAUSE(copyin)
 OPENMP_DISTRIBUTE_PARALLEL_FOR_CLAUSE(schedule)
-OPENMP_DISTRIBUTE_PARALLEL_FOR_CLAUSE(linear)
 
 // Clauses allowed for OpenMP directive 'distribute parallel for simd'
 OPENMP_DISTRIBUTE_PARALLEL_FOR_SIMD_CLAUSE(firstprivate)
@@ -746,7 +745,6 @@ OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLA
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(shared)
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(reduction)
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(schedule)
-OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(linear)
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(num_teams)
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(thread_limit)
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(copyin)
@@ -807,7 +805,6 @@ OPENMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_
 OPENMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(num_threads)
 OPENMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(proc_bind)
 OPENMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(schedule)
-OPENMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(linear)
 
 // Clauses allowed for OpenMP directive
 // 'target teams distribute parallel for simd'.

Removed: cfe/trunk/test/OpenMP/distribute_parallel_for_linear_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_parallel_for_linear_messages.cpp?rev=319331&view=auto
==
--- cfe/trunk/test/OpenMP/distribute_parallel_for_linear_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/distribute_parallel_for_linear_messages.cpp (removed)
@@ -1,338 +0,0 @@
-// RUN: %clang_cc1 -verify -fopenmp %s
-
-namespace X {
-  int x;
-};
-
-struct B {
-  static int ib; // expected-note {{'B::ib' declared here}}
-  static int bfoo() { return 8; }
-};
-
-int bfoo() { return 4; }
-
-int z;
-const int C1 = 1;
-const int C2 = 2;
-void test_linear_colons()
-{
-  int B = 0;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B:bfoo())
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B::ib:B:bfoo()) // expected-error 
{{unexpected ':' in nested name specifier; did you mean '::'}}
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B:ib) // expected-error {{use of 
undeclared identifier 'ib'; did you mean 'B::ib'}}
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(z:B:ib) // expected-error 
{{unexpected ':' in nested name specifier; did you mean '::'?}}
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B:B::bfoo())
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(X::x : ::z)
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B,::z, X::x)
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(::z)
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B::bfoo()) // expected-error 
{{expected variable name}}
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B::ib,B:C1+C2)
-  for (int i = 0; i < 10; ++i) ;
-}
-
-template T test_template(T* arr, N num) {
-  N i;
-  T sum = (T

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/Protocol.h:453
+struct MarkedString {
+  /**
+   * MarkedString can be used to render human readable text. It is either a

The doc should use /// like the others


https://reviews.llvm.org/D35894



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

[PATCH] D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true

[Devin Coughlin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319333: [analyzer] Fix unreachable creating 
PathDiagnosticLocation with widen-loops=true (authored by dcoughlin).

Changed prior to commit:
  https://reviews.llvm.org/D37187?vs=124549&id=124778#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37187

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  cfe/trunk/test/Analysis/loop-widening-notes.cpp


Index: cfe/trunk/test/Analysis/loop-widening-notes.cpp
===
--- cfe/trunk/test/Analysis/loop-widening-notes.cpp
+++ cfe/trunk/test/Analysis/loop-widening-notes.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha -analyzer-max-loop 2 
-analyzer-config widen-loops=true -analyzer-output=text -verify %s
+
+int *p_a;
+int bar();
+int flag_a;
+int test_for_bug_25609() {
+  if (p_a == 0) // expected-note {{Assuming 'p_a' is equal to null}} 
+// expected-note@-1 {{Taking true branch}}
+bar();
+  for (int i = 0;  // expected-note {{Loop condition is true.  Entering loop 
body}}
+   // expected-note@-1 {{Loop condition is false. Execution 
continues on line 16}}
+   ++i,// expected-note {{Value assigned to 'p_a'}} 
+   i < flag_a;
+   ++i) {}
+  
+  *p_a = 25609; // no-crash expected-warning {{Dereference of null pointer 
(loaded from variable 'p_a')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from 
variable 'p_a')}}
+  return *p_a;
+}
+
+int flag_b;
+int while_analyzer_output() {
+  flag_b = 100;
+  int num = 10;
+  while (flag_b-- > 0) { // expected-note {{Loop condition is true.  Entering 
loop body}} 
+ // expected-note@-1 {{Value assigned to 'num'}} 
+ // expected-note@-2 {{Loop condition is false. 
Execution continues on line 30}}
+num = flag_b;
+  }
+  if (num < 0) // expected-note {{Assuming 'num' is >= 0}} 
+   // expected-note@-1 {{Taking false branch}}
+flag_b = 0;
+  else if (num >= 1) // expected-note {{Assuming 'num' is < 1}} 
+ // expected-note@-1 {{Taking false branch}}
+flag_b = 50;
+  else
+flag_b = 100;
+  return flag_b / num; // no-crash expected-warning {{Division by zero}} 
+   // expected-note@-1 {{Division by zero}}
+}
+
+int flag_c;
+int do_while_analyzer_output() {
+  int num = 10;
+  do {   // expected-note {{Loop condition is true. Execution continues on 
line 47}} 
+ // expected-note@-1 {{Loop condition is false.  Exiting loop}}
+num--;
+  } while (flag_c-- > 0); //expected-note {{Value assigned to 'num'}}
+  int local = 0;
+  if (num == 0)   // expected-note {{Assuming 'num' is equal to 0}} 
+  // expected-note@-1 {{Taking true branch}}
+local = 10 / num; // no-crash expected-warning {{Division by zero}}
+  // expected-note@-1 {{Division by zero}}
+  return local;
+}
+
+int flag_d;
+int test_for_loop() {
+  int num = 10;
+  for (int i = 0;// expected-note {{Loop condition is true.  Entering loop 
body}} 
+ // expected-note@-1 {{Loop condition is false. Execution 
continues on line 67}}
+   new int(10),  // expected-note {{Value assigned to 'num'}}
+   i < flag_d;
+   ++i) { 
+++num;
+  }
+  if (num == 0) // expected-note {{Assuming 'num' is equal to 0}} 
+// expected-note@-1 {{Taking true branch}}
+flag_d += 10;
+  return flag_d / num; // no-crash expected-warning {{Division by zero}} 
+   // expected-note@-1 {{Division by zero}}
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -690,6 +690,15 @@
 return getLocationForCaller(CEE->getCalleeContext(),
 CEE->getLocationContext(),
 SMng);
+  } else if (Optional BE = P.getAs()) {
+CFGElement BlockFront = BE->getBlock()->front();
+if (auto StmtElt = BlockFront.getAs()) {
+  return PathDiagnosticLocation(StmtElt->getStmt()->getLocStart(), SMng);
+} else if (auto NewAllocElt = BlockFront.getAs()) {
+  return PathDiagnosticLocation(
+  NewAllocElt->getAllocatorExpr()->getLocStart(), SMng);
+}
+llvm_unreachable("Unexpected CFG element at front of block");
   } else {
 llvm_unreachable("Unexpected ProgramPoint");
   }


Index: cfe/trunk/test/Analysis/loop-widening-notes.cpp
===
--- cfe/trunk/test/Analysis/loop-widening-notes.cpp
+++ cfe/trunk/test/Analysis/loop-widening-notes.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=

r319333 - [analyzer] Fix unreachable creating PathDiagnosticLocation with widen-loops=true

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Nov 29 10:25:37 2017
New Revision: 319333

URL: http://llvm.org/viewvc/llvm-project?rev=319333&view=rev
Log:
[analyzer] Fix unreachable creating PathDiagnosticLocation with widen-loops=true

In the original design of the analyzer, it was assumed that a BlockEntrance
doesn't create a new binding on the Store, but this assumption isn't true when
'widen-loops' is set to true. Fix this by finding an appropriate location
BlockEntrace program points.

Patch by Henry Wong!

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

Added:
cfe/trunk/test/Analysis/loop-widening-notes.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=319333&r1=319332&r2=319333&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Wed Nov 29 10:25:37 
2017
@@ -690,6 +690,15 @@ PathDiagnosticLocation::create(const Pro
 return getLocationForCaller(CEE->getCalleeContext(),
 CEE->getLocationContext(),
 SMng);
+  } else if (Optional BE = P.getAs()) {
+CFGElement BlockFront = BE->getBlock()->front();
+if (auto StmtElt = BlockFront.getAs()) {
+  return PathDiagnosticLocation(StmtElt->getStmt()->getLocStart(), SMng);
+} else if (auto NewAllocElt = BlockFront.getAs()) {
+  return PathDiagnosticLocation(
+  NewAllocElt->getAllocatorExpr()->getLocStart(), SMng);
+}
+llvm_unreachable("Unexpected CFG element at front of block");
   } else {
 llvm_unreachable("Unexpected ProgramPoint");
   }

Added: cfe/trunk/test/Analysis/loop-widening-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/loop-widening-notes.cpp?rev=319333&view=auto
==
--- cfe/trunk/test/Analysis/loop-widening-notes.cpp (added)
+++ cfe/trunk/test/Analysis/loop-widening-notes.cpp Wed Nov 29 10:25:37 2017
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha -analyzer-max-loop 2 
-analyzer-config widen-loops=true -analyzer-output=text -verify %s
+
+int *p_a;
+int bar();
+int flag_a;
+int test_for_bug_25609() {
+  if (p_a == 0) // expected-note {{Assuming 'p_a' is equal to null}} 
+// expected-note@-1 {{Taking true branch}}
+bar();
+  for (int i = 0;  // expected-note {{Loop condition is true.  Entering loop 
body}}
+   // expected-note@-1 {{Loop condition is false. Execution 
continues on line 16}}
+   ++i,// expected-note {{Value assigned to 'p_a'}} 
+   i < flag_a;
+   ++i) {}
+  
+  *p_a = 25609; // no-crash expected-warning {{Dereference of null pointer 
(loaded from variable 'p_a')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from 
variable 'p_a')}}
+  return *p_a;
+}
+
+int flag_b;
+int while_analyzer_output() {
+  flag_b = 100;
+  int num = 10;
+  while (flag_b-- > 0) { // expected-note {{Loop condition is true.  Entering 
loop body}} 
+ // expected-note@-1 {{Value assigned to 'num'}} 
+ // expected-note@-2 {{Loop condition is false. 
Execution continues on line 30}}
+num = flag_b;
+  }
+  if (num < 0) // expected-note {{Assuming 'num' is >= 0}} 
+   // expected-note@-1 {{Taking false branch}}
+flag_b = 0;
+  else if (num >= 1) // expected-note {{Assuming 'num' is < 1}} 
+ // expected-note@-1 {{Taking false branch}}
+flag_b = 50;
+  else
+flag_b = 100;
+  return flag_b / num; // no-crash expected-warning {{Division by zero}} 
+   // expected-note@-1 {{Division by zero}}
+}
+
+int flag_c;
+int do_while_analyzer_output() {
+  int num = 10;
+  do {   // expected-note {{Loop condition is true. Execution continues on 
line 47}} 
+ // expected-note@-1 {{Loop condition is false.  Exiting loop}}
+num--;
+  } while (flag_c-- > 0); //expected-note {{Value assigned to 'num'}}
+  int local = 0;
+  if (num == 0)   // expected-note {{Assuming 'num' is equal to 0}} 
+  // expected-note@-1 {{Taking true branch}}
+local = 10 / num; // no-crash expected-warning {{Division by zero}}
+  // expected-note@-1 {{Division by zero}}
+  return local;
+}
+
+int flag_d;
+int test_for_loop() {
+  int num = 10;
+  for (int i = 0;// expected-note {{Loop condition is true.  Entering loop 
body}} 
+ // expected-note@-1 {{Loop condition is false. Execution 
continues on line 67}}
+   new int(10),  // expected-note {{Value assigned to 'num'}}
+   i < flag_d;
+   ++i) { 

[PATCH] D39279: Stringizing raw string literals containing newline

[Taewook Oh via Phabricator via cfe-commits]

twoh added a comment.

@jkorous-apple Thanks for the comments! Yeah, I was thinking of 
O(lenght_of_string) approach, but considering the complicatedness of the 
implementation (I guess the real implementation would be a bit more complex 
than your pseudo implementation to handle quote and '\n\r' '\r\n' cases) I 
decided to stay with O(length_of_string * number_of_endlines_in_string) but 
optimizing the number of move operations.

@vsapsai @jkorous-apple, I wonder if you can actually approve the patch or 
suggest other reviewers? Thanks!


https://reviews.llvm.org/D39279



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

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+  str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will also be garbage";

xazax.hun wrote:
> lebedev.ri wrote:
> > dcoughlin wrote:
> > > "Unary operator" is probably not something we should expect users to know 
> > > about. How about just "The expression is an uninitialized value. The 
> > > computed value will also be garbage."
> > > 
> > Yep, i did not like my wording either :)
> A value being undefined does nt mean uninitialized. E.g.: the result of a bad 
> shift operation might be UndefVal as well.
> Aren't these diagnostics misleading in those cases? Or we do not care about 
> those?
I think this is for @dcoughlin to answer.
But shift operation is a binary operator, so this diff should not change that.


Repository:
  rC Clang

https://reviews.llvm.org/D40463



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

Re: r319297 - Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via cfe-commits]

Thanks for letting me know Reid.
I’ll in work and won’t be able to access the repo until lunch time. (~3
hours)
Feel free to revert if it is not trivial.

The easy fix might be to change to == x86_64 from != x86 For is Windows in
the default toolchain. That should restore the old behavior.

I’m curious, is this in clang-cl mode?


On Wed 29 Nov 2017 at 10:12, Reid Kleckner  wrote:

> I see a bunch of link errors that suggest we're using the wrong EH
> personality in MSVC mode now:
>
> FAILED: win_clang_nacl_win64/swiftshader/libEGL.dll
> win_clang_nacl_win64/swiftshader/libEGL.dll.lib
> win_clang_nacl_win64/swiftshader/libEGL.dll.pdb
> C:/b/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe
> ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x64
> False link.exe /nologo
> /IMPLIB:win_clang_nacl_win64/swiftshader/libEGL.dll.lib /DLL
> /OUT:win_clang_nacl_win64/swiftshader/libEGL.dll
> /PDB:win_clang_nacl_win64/swiftshader/libEGL.dll.pdb
> @win_clang_nacl_win64/swiftshader/libEGL.dll.rsp
> Config.obj : error LNK2001: unresolved external symbol _Unwind_Resume
> Display.obj : error LNK2001: unresolved external symbol _Unwind_Resume
> Surface.obj : error LNK2001: unresolved external symbol _Unwind_Resume
> Config.obj : error LNK2001: unresolved external symbol
> __gxx_personality_seh0
> Display.obj : error LNK2001: unresolved external symbol
> __gxx_personality_seh0
> Surface.obj : error LNK2001: unresolved external symbol
> __gxx_personality_seh0
>
> I'll dig in a bit to see if there's an easy fix.
>
> On Tue, Nov 28, 2017 at 11:25 PM, Martell Malone via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: martell
>> Date: Tue Nov 28 23:25:12 2017
>> New Revision: 319297
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=319297&view=rev
>> Log:
>> Toolchain: Normalize dwarf, sjlj and seh eh
>>
>> This is a re-apply of r319294.
>>
>> adds -fseh-exceptions and -fdwarf-exceptions flags
>>
>> clang will check if the user has specified an exception model flag,
>> in the absense of specifying the exception model clang will then check
>> the driver default and append the model flag for that target to cc1
>>
>> -fno-exceptions has a higher priority then specifying the model
>>
>> move __SEH__ macro definitions out of Targets into InitPreprocessor
>> behind the -fseh-exceptions flag
>>
>> move __ARM_DWARF_EH__ macrodefinitions out of verious targets and into
>> InitPreprocessor behind the -fdwarf-exceptions flag and arm|thumb check
>>
>> remove unused USESEHExceptions from the MinGW Driver
>>
>> fold USESjLjExceptions into a new GetExceptionModel function that
>> gives the toolchain classes more flexibility with eh models
>>
>> Reviewers: rnk, mstorsjo
>>
>> Differential Revision: https://reviews.llvm.org/D39673
>>
>> Added:
>> cfe/trunk/test/CodeGenCXX/mingw-w64-exceptions.c
>> Modified:
>> cfe/trunk/docs/ClangCommandLineReference.rst
>> cfe/trunk/include/clang/Basic/LangOptions.def
>> cfe/trunk/include/clang/Driver/Options.td
>> cfe/trunk/include/clang/Driver/ToolChain.h
>> cfe/trunk/lib/Basic/Targets/ARM.cpp
>> cfe/trunk/lib/Basic/Targets/OSTargets.cpp
>> cfe/trunk/lib/Basic/Targets/OSTargets.h
>> cfe/trunk/lib/Basic/Targets/X86.h
>> cfe/trunk/lib/CodeGen/BackendUtil.cpp
>> cfe/trunk/lib/CodeGen/CGException.cpp
>> cfe/trunk/lib/Driver/ToolChain.cpp
>> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
>> cfe/trunk/lib/Driver/ToolChains/Darwin.h
>> cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
>> cfe/trunk/lib/Driver/ToolChains/FreeBSD.h
>> cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
>> cfe/trunk/lib/Driver/ToolChains/MinGW.h
>> cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
>> cfe/trunk/lib/Driver/ToolChains/NetBSD.h
>> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>> cfe/trunk/lib/Frontend/InitPreprocessor.cpp
>> cfe/trunk/test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
>> cfe/trunk/test/Preprocessor/arm-target-features.c
>> cfe/trunk/test/Preprocessor/init.c
>>
>> Modified: cfe/trunk/docs/ClangCommandLineReference.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=319297&r1=319296&r2=319297&view=diff
>>
>> ==
>> --- cfe/trunk/docs/ClangCommandLineReference.rst (original)
>> +++ cfe/trunk/docs/ClangCommandLineReference.rst Tue Nov 28 23:25:12 2017
>> @@ -1706,10 +1706,18 @@ Which overload candidates to show when o
>>
>>  Enable C++14 sized global deallocation functions
>>
>> +.. option:: -fdwarf-exceptions
>> +
>> +Use DWARF style exceptions
>> +
>>  .. option:: -fsjlj-exceptions
>>
>>  Use SjLj style exceptions
>>
>> +.. option:: -fseh-exceptions
>> +
>> +Use SEH style exceptions
>> +
>>  .. option:: -fslp-vectorize, -fno-slp-vectorize, -ftree-slp-vectorize
>>
>>  Enable the superword-level parallelism vectorization passes
>>
>> Modi

r319336 - [clang-cl] Alias /wd4018 to -Wno-sign-compare

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed Nov 29 10:45:03 2017
New Revision: 319336

URL: http://llvm.org/viewvc/llvm-project?rev=319336&view=rev
Log:
[clang-cl] Alias /wd4018 to -Wno-sign-compare

This warning is known to be noisy and projects frequently disable it. In
particular, this should make building isl as bundled in polly with
clang-cl a lot quieter.

Modified:
cfe/trunk/include/clang/Driver/CLCompatOptions.td

Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=319336&r1=319335&r2=319336&view=diff
==
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Wed Nov 29 10:45:03 2017
@@ -154,6 +154,8 @@ def _SLASH_WX_ : CLFlag<"WX-">, HelpText
 def _SLASH_w_flag : CLFlag<"w">, HelpText<"Disable all warnings">, Alias;
 def _SLASH_wd4005 : CLFlag<"wd4005">, Alias,
   AliasArgs<["no-macro-redefined"]>;
+def _SLASH_wd4018 : CLFlag<"wd4018">, Alias,
+  AliasArgs<["no-sign-compare"]>;
 def _SLASH_wd4100 : CLFlag<"wd4100">, Alias,
   AliasArgs<["no-unused-parameter"]>;
 def _SLASH_wd4910 : CLFlag<"wd4910">, Alias,


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

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+  str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will also be garbage";

lebedev.ri wrote:
> xazax.hun wrote:
> > lebedev.ri wrote:
> > > dcoughlin wrote:
> > > > "Unary operator" is probably not something we should expect users to 
> > > > know about. How about just "The expression is an uninitialized value. 
> > > > The computed value will also be garbage."
> > > > 
> > > Yep, i did not like my wording either :)
> > A value being undefined does nt mean uninitialized. E.g.: the result of a 
> > bad shift operation might be UndefVal as well.
> > Aren't these diagnostics misleading in those cases? Or we do not care about 
> > those?
> I think this is for @dcoughlin to answer.
> But shift operation is a binary operator, so this diff should not change that.
I was wondering about examples like:
```
int x = 1 << -1;
++x;
```

In this particular case, it will not issue the misleading error message. The 
shift that results in an undefined value will generate an error node, so we 
will not warn for the pre increment.

But in general I am a bit uncomfortable about the assumption of this check: if 
a value is undefined the reason is that it is being uninitialized. 

Note that, of course this problem is not specific to this patch but a general 
question about the checker. 


Repository:
  rC Clang

https://reviews.llvm.org/D40463



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

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 124783.
mattd added a comment.

- Removed the new lines.
- Added white space between the data type and pointer character.
- Updated the test to check the exact bounds, and over-bounds cases
- Combined the diagnostic messages into one via 'select'




https://reviews.llvm.org/D40574

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaChecking.cpp
  test/Sema/error-type-safety.cpp

Index: test/Sema/error-type-safety.cpp
===
--- test/Sema/error-type-safety.cpp
+++ test/Sema/error-type-safety.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define INT_TAG 42
+
+static const int test_in
+  __attribute__((type_tag_for_datatype(test, int))) = INT_TAG;
+
+// Argument index: 1, Type tag index: 2
+void test_bounds_index(...)
+  __attribute__((argument_with_type_tag(test, 1, 2)));
+
+// Argument index: 3, Type tag index: 1
+void test_bounds_arg_index(...)
+  __attribute__((argument_with_type_tag(test, 3, 1)));
+
+void test_bounds()
+{
+  // Test the boundary edges (ensure no off-by-one) with argument indexing.
+  test_bounds_index(1, INT_TAG);
+
+  test_bounds_index(1); // expected-error {{type tag index 2 is greater than the number of arguments specified}}
+  test_bounds_arg_index(INT_TAG, 1); // expected-error {{argument index 3 is greater than the number of arguments specified}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2754,7 +2754,7 @@
 // Type safety checking.
 if (FDecl) {
   for (const auto *I : FDecl->specific_attrs())
-CheckArgumentWithTypeTag(I, Args.data());
+CheckArgumentWithTypeTag(I, Args, Loc);
 }
   }
 
@@ -12329,10 +12329,18 @@
 }
 
 void Sema::CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs) {
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc) {
   const IdentifierInfo *ArgumentKind = Attr->getArgumentKind();
   bool IsPointerAttr = Attr->getIsPointer();
 
+  // Retrieve the argument representing the 'type_tag'.
+  if (Attr->getTypeTagIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
+<< 0 << Attr->getTypeTagIdx() + 1;
+return;
+  }
   const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
   bool FoundWrongKind;
   TypeTagData TypeInfo;
@@ -12346,6 +12354,13 @@
 return;
   }
 
+  // Retrieve the argument representing the 'arg_idx'.
+  if (Attr->getArgumentIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
+<< 1 << Attr->getArgumentIdx() + 1;
+return;
+  }
   const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
   if (IsPointerAttr) {
 // Skip implicit cast of pointer to `void *' (as a function argument).
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10455,7 +10455,8 @@
   /// \brief Peform checks on a call of a function with argument_with_type_tag
   /// or pointer_with_type_tag attributes.
   void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs);
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc);
 
   /// \brief Check if we are taking the address of a packed field
   /// as this may be a problem if the pointer value is dereferenced.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7919,6 +7919,8 @@
   "'type_tag_for_datatype' attribute requires the initializer to be "
   "an %select{integer|integral}0 constant expression "
   "that can be represented by a 64 bit integer">;
+def err_tag_index_out_of_range : Error<
+  "%select{type tag|argument}0 index %1 is greater than the number of arguments specified">;
 def warn_type_tag_for_datatype_wrong_kind : Warning<
   "this type tag was not designed to be used with this function">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40569: Use default IR alignment for cleanup.dest.slot.

[Jacob Young via Phabricator via cfe-commits]

jacobly added a comment.

Could someone commit this for me? Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D40569



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

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Matt Davis via Phabricator via cfe-commits]

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



Comment at: test/Sema/error-type-safety.cpp:3-4
+
+static const int test_void
+  __attribute__((type_tag_for_datatype(test, void))) = 0;
+

aaron.ballman wrote:
> Is this declaration necessary?
Hi Aaron, thanks for the input.  Yes, defining the tag is necessary in order to 
test the argument index.  If the type tag is not satisfied, the code path 
checking argument tag index will never be reached.  My update will test the 
bounds and is a little more comprehensive.


https://reviews.llvm.org/D40574



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

[PATCH] D40527: [libclang] Record parsing invocation to a temporary file when requested by client

[Alex Lorenz via Phabricator via cfe-commits]

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



Comment at: tools/libclang/CIndexer.cpp:131
+  if (!File.empty())
+llvm::sys::fs::remove(File);
+}

jkorous-apple wrote:
> Just a thought - since we are not propagating errors from constructor we are 
> not really sure we were able to create the file in the first place (e. g. 
> return from ctor at line 103). Should we still try to delete it?
If `createUniqueFile` failed in the constructor (return on line 103) then 
`File` will be empty. Therefore we won't try to remove the file.


Repository:
  rC Clang

https://reviews.llvm.org/D40527



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

[PATCH] D40527: [libclang] Record parsing invocation to a temporary file when requested by client

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 124788.
arphaman added a comment.

- Capture OS only.
- Guard toolchain path accessor with a mutex.


Repository:
  rC Clang

https://reviews.llvm.org/D40527

Files:
  include/clang-c/Index.h
  test/Index/record-parsing-invocation.c
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/CIndexer.cpp
  tools/libclang/CIndexer.h
  tools/libclang/libclang.exports

Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -2,6 +2,7 @@
 clang_CXCursorSet_insert
 clang_CXIndex_getGlobalOptions
 clang_CXIndex_setGlobalOptions
+clang_CXIndex_setInvocationEmissionPathOption
 clang_CXXConstructor_isConvertingConstructor
 clang_CXXConstructor_isCopyConstructor
 clang_CXXConstructor_isDefaultConstructor
Index: tools/libclang/CIndexer.h
===
--- tools/libclang/CIndexer.h
+++ tools/libclang/CIndexer.h
@@ -18,6 +18,7 @@
 #include "clang-c/Index.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Mutex.h"
 #include 
 
 namespace llvm {
@@ -40,6 +41,11 @@
   std::string ResourcesPath;
   std::shared_ptr PCHContainerOps;
 
+  std::string ToolchainPath;
+  llvm::sys::SmartMutex Mutex;
+
+  std::string InvocationEmissionPath;
+
 public:
   CIndexer(std::shared_ptr PCHContainerOps =
std::make_shared())
@@ -71,6 +77,29 @@
 
   /// \brief Get the path of the clang resource files.
   const std::string &getClangResourcesPath();
+
+  StringRef getClangToolchainPath();
+
+  void setInvocationEmissionPath(StringRef Str) {
+InvocationEmissionPath = Str;
+  }
+
+  StringRef getInvocationEmissionPath() const { return InvocationEmissionPath; }
+};
+
+/// Logs information about a particular libclang operation like parsing to
+/// a new file in the invocation emission path.
+class LibclangInvocationReporter {
+public:
+  enum class OperationKind { ParseOperation, CompletionOperation };
+
+  LibclangInvocationReporter(CIndexer &Idx, OperationKind Op,
+ unsigned ParseOptions,
+ llvm::ArrayRef Args);
+  ~LibclangInvocationReporter();
+
+private:
+  std::string File;
 };
 
   /// \brief Return the current size to request for "safety".
Index: tools/libclang/CIndexer.cpp
===
--- tools/libclang/CIndexer.cpp
+++ tools/libclang/CIndexer.cpp
@@ -14,8 +14,10 @@
 #include "CIndexer.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/Version.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Support/MutexGuard.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include 
@@ -76,3 +78,59 @@
   ResourcesPath = LibClangPath.str();
   return ResourcesPath;
 }
+
+StringRef CIndexer::getClangToolchainPath() {
+  // The toolchain path might be requested by multiple threads from
+  // LibclangInvocationReporter.
+  llvm::MutexGuard Guard(Mutex);
+  if (!ToolchainPath.empty())
+return ToolchainPath;
+  StringRef ResourcePath = getClangResourcesPath();
+  ToolchainPath = llvm::sys::path::parent_path(
+  llvm::sys::path::parent_path(llvm::sys::path::parent_path(ResourcePath)));
+  return ToolchainPath;
+}
+
+LibclangInvocationReporter::LibclangInvocationReporter(
+CIndexer &Idx, OperationKind Op, unsigned ParseOptions,
+llvm::ArrayRef Args) {
+  StringRef Path = Idx.getInvocationEmissionPath();
+  if (Path.empty())
+return;
+
+  // Create a temporary file for the invocation log.
+  SmallString<256> TempPath;
+  TempPath = Path;
+  llvm::sys::path::append(TempPath, "libclang-");
+  int FD;
+  if (llvm::sys::fs::createUniqueFile(TempPath, FD, TempPath))
+return;
+  File = std::string(TempPath.begin(), TempPath.end());
+  llvm::raw_fd_ostream OS(FD, /*ShouldClose=*/true);
+
+  // Write out the information about the invocation to it.
+  auto WriteStringKey = [&OS](StringRef Key, StringRef Value) {
+OS << R"(")" << Key << R"(":")";
+OS << Value << '"';
+  };
+  OS << '{';
+  WriteStringKey("toolchain", Idx.getClangToolchainPath());
+  OS << ',';
+  WriteStringKey("libclang.operation",
+ Op == OperationKind::ParseOperation ? "parse" : "complete");
+  OS << ',';
+  OS << R"("libclang.opts":)" << ParseOptions;
+  OS << ',';
+  OS << R"("args":[)";
+  for (const auto &I : llvm::enumerate(Args)) {
+if (I.index())
+  OS << ',';
+OS << '"' << I.value() << '"';
+  }
+  OS << "]}";
+}
+
+LibclangInvocationReporter::~LibclangInvocationReporter() {
+  if (!File.empty())
+llvm::sys::fs::remove(File);
+}
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added inline comments.



Comment at: docs/TaggedAddressSanitizerDesign.rst:22
+*quarantine* to find use-after-free.
+The shadow, the redzones, and the quarantine are the
+major sources of AddressSanitizer's memory overhead.

kcc wrote:
> davidxl wrote:
> > What is the overhead of redzones compared with shadow memory?
> depends.
> shadow is 1/9 of all memory.
> redzones largely depend on the allocation patterns. 
> If most heap allocations are big, the combined redzones are tiny, and vise 
> versa 
> 
It's worth mentioning that typical ASan memory overhead of 2x to 3x, mainly 
from redzones and quarantine.

This new tool will have only the shadow, which is a tiny fraction of the 
original memory footprint, and some overhead from over-aligning everything. The 
latter is expected to be small, especially if N (see below) is reduced to 16 or 
even 8, in which case the shadow grows to 1/16th or 1/8th, respectively. In any 
case, I'd expect less than 25% RAM overhead, hopefully way less.


Repository:
  rC Clang

https://reviews.llvm.org/D40568



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

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[Kostya Serebryany via Phabricator via cfe-commits]

kcc updated this revision to Diff 124797.
kcc added a comment.

rephrase the sources of asan overhead


Repository:
  rC Clang

https://reviews.llvm.org/D40568

Files:
  docs/TaggedAddressSanitizerDesign.rst

Index: docs/TaggedAddressSanitizerDesign.rst
===
--- /dev/null
+++ docs/TaggedAddressSanitizerDesign.rst
@@ -0,0 +1,125 @@
+===
+TaggedAddressSanitizer Design Documentation
+===
+
+This page is a preliminary design document for
+a **hardware-assisted memory safety** (HWAMS) tool,
+similar to :doc:`AddressSanitizer`.
+
+The name **TaggedAddressSanitizer** or TASAN
+(alternative: **HardwareAssistedAddressSanitizer** or HWASAN)
+and the rest of the document,
+are *early draft*, suggestions are welcome.
+
+
+Introduction
+
+
+:doc:`AddressSanitizer`
+tags every 8 bytes of the application memory with a 1 byte tag (using *shadow memory*),
+uses *redzones* to find buffer-overflows and
+*quarantine* to find use-after-free.
+The redzones, the quarantine, and, to a less extent, the shadow, are the
+sources of AddressSanitizer's memory overhead.
+See the `AddressSanitizer paper`_ for details.
+
+AArch64 has the `Address Tagging`_, a hardware feature that allows
+software to use 8 most significant bits of a 64-bit pointer as
+a tag. TaggedAddressSanitizer uses `Address Tagging`_
+to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
+but with smaller memory overhead and slightly different (mostly better)
+accuracy guarantees.
+
+Algorithm
+=
+* Every heap/stack/global memory object is forcibly aligned by `N` bytes
+  (`N` is e.g. 16 or 64)
+* For every such object a random `K`-bit tag `T` is chosen (`K` is e.g. 4 or 8)
+* The pointer to the object is tagged with `T`.
+* The memory for the object is also tagged with `T`
+  (using a `N=>1` shadow memory)
+* Every load and store is instrumented to read the memory tag and compare it
+  with the pointer tag, exception is raised on tag mismatch.
+
+Instrumentation
+===
+
+Memory Accesses
+---
+All memory accesses are prefixed with a call to a run-time function.
+The function encodes the type and the size of access in its name;
+it receives the address as a parameter, e.g. `__hwams_load4(void *ptr)`;
+it loads the memory tag, compares it with the
+pointer tag, and executes `__builtin_trap` (or calls `__hwams_error_load4(void *ptr)`) on mismatch.
+
+It's possible to inline this callback too.
+
+Heap
+
+
+Tagging the heap memory/pointers is done by `malloc`.
+This can be based on any malloc that forces all objects to be N-aligned.
+
+Stack
+-
+
+Special compiler instrumentation is required to align the local variables
+by N, tag the memory and the pointers.
+Stack instrumentation is expected to be a major source of overhead,
+but could be optional.
+TODO: details.
+
+Globals
+---
+
+TODO: details.
+
+Error reporting
+---
+
+Errors are generated by `__builtin_trap` and are handled by a signal handler.
+
+
+Comparison with AddressSanitizer
+
+
+TaggedAddressSanitizer:
+  * Is less portable than :doc:`AddressSanitizer`
+as it relies on hardware `Address Tagging`_ (AArch64).
+Address Tagging can be emulated with compiler instrumentation,
+but it will require the instrumentation to remove the tags before
+any load or store, which is infeasible in any realistic environment
+that contains non-instrumented code.
+  * May have compatibility problems if the target code uses higher
+pointer bits for other purposes.
+  * May require changes in the OS kernels (e.g. Linux seems to dislike
+tagged pointers passed from address space).
+  * Does not require redzones to detect buffer overflows,
+but the buffer overflow detection is probabilistic, with roughly
+`(2**K-1)/(2**K)` probability of catching a bug.
+  * Does not require quarantine to detect heap-use-after-free,
+or stack-use-after-return.
+The detection is similarly probabilistic.
+
+The memory overhead of TaggedAddressSanitizer is expected to be much smaller
+than that of AddressSanitizer:
+`1/N` extra memory for the shadow
+and some overhead due to `N`-aligning all objects.
+
+
+Related Work
+
+* `SPARC ADI`_ implements a similar tool mostly in hardware.
+* `Effective and Efficient Memory Protection Using Dynamic Tainting`_ discusses
+  similar approaches ("lock & key").
+* `Watchdog`_ discussed a heavier, but still somewhat similar
+  "lock & key" approach.
+* *TODO: add more "related work" links. Suggestions are welcome.*
+
+
+.. _Watchdog: http://www.cis.upenn.edu/acg/papers/isca12_watchdog.pdf
+.. _Effective and Efficient Memory Protection Using Dynamic Tainting: https://www.cc.gatech.edu/~orso/papers/clause.doudalis.orso.prvulovic.pdf
+.. _SPARC ADI: https://lazytyped.blogspot.com/2017/09/getting-started-with-ad

[PATCH] D40601: [XRay][clang] Introduce -fxray-always-emit-customevents

[David Blaikie via Phabricator via cfe-commits]

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

Not totally clear to me why this feature is desirable, but assume you've got 
use cases/reasons :)




Comment at: clang/lib/Driver/XRayArgs.cpp:32-33
 constexpr char XRayNeverInstrumentOption[] = "-fxray-never-instrument=";
+constexpr char XRayAlwaysEmitCustomEventsOption[] =
+"-fxray-always-emit-customevents";
 } // namespace

Not clear to me there's a benefit to having these defined as constants versus 
using the literal directly - other parts of the driver use literals directly & 
there's are mostly used just once?


https://reviews.llvm.org/D40601



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

[PATCH] D40618: [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang created this revision.

This fixes the following failure uncovered by https://reviews.llvm.org/D39245:

  Clang :: Index/getcursor-preamble.m


Repository:
  rC Clang

https://reviews.llvm.org/D40618

Files:
  tools/libclang/CIndex.cpp


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [&SM](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [&SM](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [llvm-dev] LLVM buildmaster is back to work now but two builders remain OFF

[Greg Bedwell via cfe-commits]

Hopefully these two should be good to re-enable as of r319330.

-Greg

On 29 November 2017 at 16:23, Greg Bedwell  wrote:

> > Two slaves remain off for now as they produce a lot of warnings and
> their log files are way too big.
>
> I think https://reviews.llvm.org/D40603 will fix this issue.
>
> -Greg
>
>
> On 28 November 2017 at 22:13, Galina Kistanova via llvm-dev <
> llvm-...@lists.llvm.org> wrote:
>
>> Hello everyone,
>>
>> LLVM buildmaster is back to work.
>>
>> Two slaves remain off for now as they produce a lot of warnings and their
>> log files are way too big.
>>
>> The next builds have full logs available. Hope this helps to fix this
>> issue:
>>
>> http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/8716
>> http://lab.llvm.org:8011/builders/clang-with-thin-lto-windows/builds/2645
>>
>> Thanks
>>
>> Galina
>>
>> ___
>> LLVM Developers mailing list
>> llvm-...@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40618: [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added inline comments.



Comment at: tools/libclang/CIndex.cpp:1028
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :

Also removed the assert from here since isBeforeInTranslationUnit already has 
the same assert.


Repository:
  rC Clang

https://reviews.llvm.org/D40618



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

[PATCH] D40508: Replace long type names in IR with hashes

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D40508#938854, @sepavloff wrote:

> In https://reviews.llvm.org/D40508#938686, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D40508#938675, @sepavloff wrote:
> >
> > > In https://reviews.llvm.org/D40508#938040, @rjmccall wrote:
> > >
> > > > My skepticism about the raw_ostream is not about the design of having a 
> > > > custom raw_ostream subclass, it's that that subclass could conceivably 
> > > > be re-used by some other client.  It feels like it belongs as an 
> > > > internal hack in Clang absent some real evidence that someone else 
> > > > would use it.
> > >
> > >
> > > This class can be helpful in various cases where string identifier must 
> > > persistently identify an object and memory consumption must be low. It 
> > > may be:
> > >
> > > - If we introduce an option that allows a user to specify how many 
> > > symbols of full type name are kept in abbreviated name, then `llvm-link` 
> > > may see types named as `foo` and 
> > > `2544896211ff669ed44dccd265f4c9163b340190`, if they come from modules 
> > > compiled with different options. To find out that these are the same 
> > > type, it must have access to the same machinery.
> >
> >
> > The LLVM linking model does not actually depend on struct type names 
> > matching.  My understanding is that, at best, it uses that as a heuristic 
> > for deciding whether to make an effort to unify two types, but it's not 
> > something that's ultimately supposed to impact IR semantics.
>
>
> It is mainly true with an exception, when `llvm-link` resolves opaque types 
> it relies on type name only. And this behavior creates the issue that 
> https://reviews.llvm.org/D40567 tries to resolve.


It is not clear from that report what the actual problem is.  Two incomplete 
types get merged by the linker?  So what?

>> If we needed IR type names to match reliably, we would use a mangled name, 
>> not a pretty-print.
> 
> There is no requirement for IR type name to be an identifier, so pretty-print 
> fits the need of type identification.

Not really; pretty-printing drops a lot of information that is pertinent in a 
stable identifier, like scopes and so on, and makes arbitrary decisions about 
other things, like where to insert spaces, namespace qualifiers, etc.


https://reviews.llvm.org/D40508



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

Re: r319297 - Toolchain: Normalize dwarf, sjlj and seh eh

[Martin Storsjö via cfe-commits]

On Wed, 29 Nov 2017, Martell Malone via cfe-commits wrote:


Thanks for letting me know Reid.
I’ll in work and won’t be able to access the repo until lunch time. (~3
hours)
Feel free to revert if it is not trivial.

The easy fix might be to change to == x86_64 from != x86 For is Windows in
the default toolchain. That should restore the old behavior.


My suggestion would be to just return None for all architectures for the 
default windows (msvc) case. We didn't use to set any defines to indicate 
EH mode there before anyway, so setting it to None should make things 
behave just as before, right?


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

[PATCH] D40594: [InstCombine] miscompile of __builtin_fmod

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

By many years of precedent, the "frem" instruction is supposed to match the C 
fmod(), as opposed to something else like the C99 remainder(); probably worth 
clarifying in LangRef.


https://reviews.llvm.org/D40594



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

[PATCH] D39627: Fix vtable not receiving hidden visibility when using push(visibility)

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

You probably just need to pass a "-triple" flag so we don't use Windows 
mangling or something like that.


Repository:
  rL LLVM

https://reviews.llvm.org/D39627



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

[PATCH] D40618: [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Alex Lorenz via Phabricator via cfe-commits]

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

Thanks, LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D40618



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

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/ClangdUnit.cpp:1135
+RawComment *RC =
+AST.getASTContext().getRawCommentForDeclNoCache(LocationDecl);
+if (RC) {

Not sure why but I don't get the comments when I hover on "push_back" but I do 
get it on many other methods. This can be investigated separately I think. It 
could be a bug in getRawCommentForDeclNoCache


https://reviews.llvm.org/D35894



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

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Could you please add a test?


https://reviews.llvm.org/D40563



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

[PATCH] D40256: [ARM] disable FPU features when using soft floating point.

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D40256



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

[PATCH] D40611: Add has definition AST matcher

[Aaron Ballman via Phabricator via cfe-commits]

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

Aside from a minor commenting nit, LGTM! Do you need me to commit this on your 
behalf? (If so, I can take care of the commenting fix.)




Comment at: include/clang/ASTMatchers/ASTMatchers.h:5856
 
+/// \brief Matches a declaration that is defined.
+///

Might want to mention that this is specific to classes and not other things, 
like functions.


https://reviews.llvm.org/D40611



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

[PATCH] D40618: [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Mandeep Singh Grang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319357: [SourceLocations] Use stronger sort predicate to 
remove non-deterministic… (authored by mgrang).

Changed prior to commit:
  https://reviews.llvm.org/D40618?vs=124798&id=124804#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40618

Files:
  cfe/trunk/tools/libclang/CIndex.cpp


Index: cfe/trunk/tools/libclang/CIndex.cpp
===
--- cfe/trunk/tools/libclang/CIndex.cpp
+++ cfe/trunk/tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [&SM](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.


Index: cfe/trunk/tools/libclang/CIndex.cpp
===
--- cfe/trunk/tools/libclang/CIndex.cpp
+++ cfe/trunk/tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [&SM](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r319357 - [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Mandeep Singh Grang via cfe-commits]

Author: mgrang
Date: Wed Nov 29 12:55:13 2017
New Revision: 319357

URL: http://llvm.org/viewvc/llvm-project?rev=319357&view=rev
Log:
[SourceLocations] Use stronger sort predicate to remove non-deterministic 
ordering

Summary:
This fixes the following failure uncovered by D39245:
  Clang :: Index/getcursor-preamble.m

Reviewers: gbenyei, akyrtzi, bkramer, arphaman

Reviewed By: arphaman

Subscribers: arphaman, cfe-commits

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

Modified:
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/tools/libclang/CIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=319357&r1=319356&r2=319357&view=diff
==
--- cfe/trunk/tools/libclang/CIndex.cpp (original)
+++ cfe/trunk/tools/libclang/CIndex.cpp Wed Nov 29 12:55:13 2017
@@ -1025,8 +1025,9 @@ bool CursorVisitor::VisitObjCContainerDe
 [&SM](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.


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

[PATCH] D40618: [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Mandeep Singh Grang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC319357: [SourceLocations] Use stronger sort predicate to 
remove non-deterministic… (authored by mgrang).

Repository:
  rC Clang

https://reviews.llvm.org/D40618

Files:
  tools/libclang/CIndex.cpp


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [&SM](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [&SM](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits